refactor: rename validation functions and update test cases for clarity and consistency

Author: dido18

internal/orchestrator/app/validator.go

@@ -6,26 +6,27 @@ import (
 	"github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex"
 )
 
-func ValidateAppDescriptor(a AppDescriptor, index *bricksindex.BricksIndex) error {
-	return validatebricks(a, index)
-}
-
-func validatebricks(a AppDescriptor, index *bricksindex.BricksIndex) error {
+// ValidateBricks checks that all bricks referenced in the given AppDescriptor exist in the provided BricksIndex,
+// and that all required variables for each brick are present and valid. It returns an error if any brick is missing,
+// if any variable referenced by the app does not exist in the corresponding brick, or if any required variable is missing.
+// If the index is nil, validation is skipped and nil is returned.
+func ValidateBricks(a AppDescriptor, index *bricksindex.BricksIndex) error {
+	if index == nil {
+		return nil
+	}
 	for _, appBrick := range a.Bricks {
 		indexBrick, found := index.FindBrickByID(appBrick.ID)
 		if !found {
 			return fmt.Errorf("brick %q not found", appBrick.ID)
 		}
 
-		// check the bricks variables inside the app.yaml are valid given a brick definition
 		for appBrickName := range appBrick.Variables {
 			_, exist := indexBrick.GetVariable(appBrickName)
 			if !exist {
 				return fmt.Errorf("variable %q does not exist on brick %q", appBrickName, indexBrick.ID)
 			}
 		}
 
-		// check all required variables has a value
 		for _, indexBrickVariable := range indexBrick.Variables {
 			if indexBrickVariable.IsRequired() {
 				if _, exist := appBrick.Variables[indexBrickVariable.Name]; !exist {

internal/orchestrator/app/validator_test.go

@@ -5,49 +5,62 @@ import (
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.bug.st/f"
 
 	"github.com/arduino/go-paths-helper"
 
 	"github.com/arduino/arduino-app-cli/internal/orchestrator/bricksindex"
 )
 
-func TestValidateAppDescriptor(t *testing.T) {
+func TestValidateBricksOnAppDescriptor(t *testing.T) {
 	bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(paths.New("testdata/validator"))
 	require.Nil(t, err)
 
-	t.Run("valid app descriptor with no bricks", func(t *testing.T) {
-		app, err := ParseDescriptorFile(paths.New("testdata/validator/no-bricks-app.yaml"))
-		require.NoError(t, err)
-		err = ValidateAppDescriptor(app, bricksIndex)
-		assert.NoError(t, err)
-	})
-
-	t.Run("valid app descriptor with empty list of bricks", func(t *testing.T) {
-		app, err := ParseDescriptorFile(paths.New("testdata/validator/empty-bricks-app.yaml"))
-		require.NoError(t, err)
-		err = ValidateAppDescriptor(app, bricksIndex)
-		assert.NoError(t, err)
-	})
-
-	t.Run("invalid app descriptor with missing required variable", func(t *testing.T) {
-		app, err := ParseDescriptorFile(paths.New("testdata/validator/missing-required-app.yaml"))
-		require.NoError(t, err)
-		err = ValidateAppDescriptor(app, bricksIndex)
-		assert.Equal(t, "variable \"ARDUINO_DEVICE_ID\" is required by brick \"arduino:arduino_cloud\"", err.Error())
-	})
-
-	t.Run("invalid app descriptor with a missing required variable among two", func(t *testing.T) {
-		app, err := ParseDescriptorFile(paths.New("testdata/validator/mixed-required-app.yaml"))
-		require.NoError(t, err)
-		err = ValidateAppDescriptor(app, bricksIndex)
-		assert.Equal(t, "variable \"ARDUINO_SECRET\" is required by brick \"arduino:arduino_cloud\"", err.Error())
-	})
-
-	t.Run("invalid app descriptor with not found brick id", func(t *testing.T) {
-		app, err := ParseDescriptorFile(paths.New("testdata/validator/not-found-brick-app.yaml"))
-		require.NoError(t, err)
-		err = ValidateAppDescriptor(app, bricksIndex)
-		assert.Equal(t, "brick \"arduino:not_existing_brick\" not found", err.Error())
-	})
+	testCases := []struct {
+		name        string
+		filename    string
+		expectedErr *string
+	}{
+		{
+			name:        "valid app descriptor with no bricks",
+			filename:    "no-bricks-app.yaml",
+			expectedErr: nil,
+		},
+		{
+			name:        "valid app descriptor with empty list of bricks",
+			filename:    "empty-bricks-app.yaml",
+			expectedErr: nil,
+		},
+		{
+			name:        "invalid app descriptor with missing required variable",
+			filename:    "missing-required-app.yaml",
+			expectedErr: f.Ptr("variable \"ARDUINO_DEVICE_ID\" is required by brick \"arduino:arduino_cloud\""),
+		},
+		{
+			name:        "invalid app descriptor with a missing required variable among two",
+			filename:    "mixed-required-app.yaml",
+			expectedErr: f.Ptr("variable \"ARDUINO_SECRET\" is required by brick \"arduino:arduino_cloud\""),
+		},
+		{
+			name:        "invalid app descriptor with not found brick id",
+			filename:    "not-found-brick-app.yaml",
+			expectedErr: f.Ptr("brick \"arduino:not_existing_brick\" not found"),
+		},
+	}
 
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			app, err := ParseDescriptorFile(paths.New("testdata/validator/" + tc.filename))
+			require.NoError(t, err)
+
+			err = ValidateBricks(app, bricksIndex)
+
+			if tc.expectedErr == nil {
+				assert.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				assert.Equal(t, *tc.expectedErr, err.Error())
+			}
+		})
+	}
 }