From 91e6b1ba42c7d3620456cb66ad7bcce5856d2771 Mon Sep 17 00:00:00 2001 From: dido18 Date: Tue, 25 Nov 2025 17:06:16 +0100 Subject: [PATCH 01/10] WIP: bricks tests --- .../bricksindex/bricks_index_test.go | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) diff --git a/internal/orchestrator/bricksindex/bricks_index_test.go b/internal/orchestrator/bricksindex/bricks_index_test.go index 5b2a1546..713c805b 100644 --- a/internal/orchestrator/bricksindex/bricks_index_test.go +++ b/internal/orchestrator/bricksindex/bricks_index_test.go @@ -16,8 +16,10 @@ package bricksindex import ( + "os" "testing" + "github.com/arduino/go-paths-helper" yaml "github.com/goccy/go-yaml" "github.com/stretchr/testify/require" ) @@ -184,3 +186,155 @@ func TestBricksIndex(t *testing.T) { require.False(t, b.Variables[0].IsRequired()) require.False(t, b.Variables[1].IsRequired()) } + +func TestBricksIndexYAMLFormats(t *testing.T) { + testCases := []struct { + name string + yamlContent string + expectedError string + expectedBricks []Brick + }{ + { + // TODO: the validator of the brick-list must not allow this + name: "missing bricks field does not cuase error", + yamlContent: `other_field: value`, + expectedBricks: nil, + }, + { + name: "bad YAML format - invalid indentation", + yamlContent: `bricks: + - id: arduino:test_brick + name: Test Brick + description: A test brick`, + expectedError: "found character '\t' that cannot start any token", + }, + { + name: "empty bricks", + yamlContent: `bricks: []`, + expectedBricks: []Brick{}, + }, + { + name: "bad YAML format - unclosed quotes", + yamlContent: `bricks: +- id: "arduino:test_brick + name: Test Brick + description: A test brick`, + expectedError: "could not find end character of double-quoted text", + }, + { + name: "bad YAML format - missing colon", + yamlContent: `bricks: +- id arduino:test_brick + name: Test Brick`, + expectedError: "unexpected key name", + }, + { + name: "bad YAML format - invalid syntax", + yamlContent: `bricks: +- id: arduino:test_brick + name: Test Brick + description: A test brick + ports: [7000,`, + expectedError: "sequence end token ']' not found", + }, + { + name: "bad YAML format - tab characters", + yamlContent: "bricks:\n\t- id: arduino:test_brick\n\t name: Test Brick", + expectedError: "found character '\t' that cannot start any token", + }, + { + name: "simple brick", + yamlContent: `bricks: +- id: arduino:simple_brick + name: Test Brick + description: A test brick +`, + expectedBricks: []Brick{ + { + ID: "arduino:simple_brick", + Name: "Test Brick", + Description: "A test brick", + Category: "", + RequiresDisplay: "", + RequireContainer: false, + RequireModel: false, + RequiredDevices: nil, + Variables: nil, + Ports: nil, + ModelName: "", + MountDevicesIntoContainer: false, + }, + }, + }, + { + name: "valid YAML with complex variables", + yamlContent: `bricks: +- id: arduino:complex_brick + name: Complex Brick + description: A complex test brick + category: storage + require_container: true + require_model: true + require_devices: false + mount_devices_into_container: true + model_name: a-complex-model + required_devices: + - camera + ports: + - 7000 + - 8080 + variables: + - name: REQUIRED_VAR + default_value: "" + description: A required variable + - name: OPTIONAL_VAR + default_value: "default_value" + description: An optional variable`, + expectedBricks: []Brick{ + { + ID: "arduino:complex_brick", + Name: "Complex Brick", + Description: "A complex test brick", + Category: "storage", + RequiresDisplay: "", + RequireContainer: true, + RequireModel: true, + RequiredDevices: []string{"camera"}, + MountDevicesIntoContainer: true, + Variables: []BrickVariable{ + { + Name: "REQUIRED_VAR", + DefaultValue: "", + Description: "A required variable", + }, + { + Name: "OPTIONAL_VAR", + DefaultValue: "default_value", + Description: "An optional variable", + }, + }, + Ports: []string{"7000", "8080"}, + ModelName: "a-complex-model", + }, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + tempDir := t.TempDir() + brickIndex := paths.New(tempDir, "bricks-list.yaml") + err := os.WriteFile(brickIndex.String(), []byte(tc.yamlContent), 0600) + require.NoError(t, err) + + index, err := GenerateBricksIndexFromFile(paths.New(tempDir)) + if tc.expectedError != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedError) + } else { + require.NoError(t, err) + require.Equal(t, index.Bricks, tc.expectedBricks, "bricsk mistmatch") + } + }) + } +} From c381c88dfc5320b5d324092cf46e7ccf2d141f67 Mon Sep 17 00:00:00 2001 From: dido18 Date: Tue, 25 Nov 2025 17:24:13 +0100 Subject: [PATCH 02/10] feat(tests): add bricks index test and YAML data for bricks --- .../bricksindex/bricks_index_test.go | 154 +----------------- .../bricksindex/testdata/bricks-list.yaml | 133 +++++++++++++++ 2 files changed, 141 insertions(+), 146 deletions(-) create mode 100644 internal/orchestrator/bricksindex/testdata/bricks-list.yaml diff --git a/internal/orchestrator/bricksindex/bricks_index_test.go b/internal/orchestrator/bricksindex/bricks_index_test.go index 713c805b..8656c125 100644 --- a/internal/orchestrator/bricksindex/bricks_index_test.go +++ b/internal/orchestrator/bricksindex/bricks_index_test.go @@ -20,150 +20,12 @@ import ( "testing" "github.com/arduino/go-paths-helper" - yaml "github.com/goccy/go-yaml" "github.com/stretchr/testify/require" ) -func TestBricksIndex(t *testing.T) { - x := `bricks: -- id: arduino:image_classification - name: Image Classification - description: "Brick for image classification using a pre-trained model. It processes\ - \ images and returns the predicted class label and confidence score.\nBrick is\ - \ designed to work with pre-trained models provided by framework or with custom\ - \ image classification models trained on Edge Impulse platform. \n" - require_container: true - require_model: true - ports: [] - model_name: mobilenet-image-classification - variables: - - name: CUSTOM_MODEL_PATH - default_value: /opt/models/ei/ - description: path to the custom model directory - - name: EI_CLASSIFICATION_MODEL - default_value: /models/ootb/ei/mobilenet-v2-224px.eim - description: path to the model file -- id: arduino:camera_scanner - name: Camera Scanner - description: Scans a camera for barcodes and QR codes - require_container: false - require_model: false - ports: [] -- id: arduino:streamlit_ui - name: Streamlit UI - description: A simplified user interface based on Streamlit and Python. - require_container: false - require_model: false - ports: - - 7000 -- id: arduino:keyword_spotter - name: Keyword Spotter - description: 'Brick for keyword spotting using a pre-trained model. It processes - audio input to detect specific keywords or phrases. - - Brick is designed to work with pre-trained models provided by framework or with - custom audio classification models trained on Edge Impulse platform. - - ' - require_container: true - require_model: true - ports: [] - model_name: keyword-spotting-hello-world - variables: - - name: CUSTOM_MODEL_PATH - default_value: /opt/models/ei/ - description: path to the custom model directory - - name: EI_KEYWORK_SPOTTING_MODEL - default_value: /models/ootb/ei/keyword-spotting-hello-world.eim - description: path to the model file -- id: arduino:mqtt - name: MQTT Connector - description: MQTT connector module. Acts as a client for receiving and publishing - messages to an MQTT broker. - require_container: false - require_model: false - ports: [] -- id: arduino:web_ui - name: Web UI - description: A user interface based on HTML and JavaScript that can rely on additional - APIs and a WebSocket exposed by a web server. - require_container: false - require_model: false - ports: - - 7000 -- id: arduino:dbstorage_tsstore - name: Database Storage - Time Series Store - description: Simplified time series database storage layer for Arduino sensor samples - built on top of InfluxDB. - require_container: true - require_model: false - ports: [] - variables: - - name: APP_HOME - default_value: . - - name: DB_PASSWORD - default_value: Arduino15 - description: Database password - - name: DB_USERNAME - default_value: admin - description: Edge Impulse project API key - - name: INFLUXDB_ADMIN_TOKEN - default_value: 392edbf2-b8a2-481f-979d-3f188b2c05f0 - description: InfluxDB admin token -- id: arduino:dbstorage_sqlstore - name: Database Storage - SQLStore - description: Simplified database storage layer for Arduino sensor data using SQLite - local database. - require_container: false - require_model: false - ports: [] -- id: arduino:object_detection - name: Object Detection - description: "Brick for object detection using a pre-trained model. It processes\ - \ images and returns the predicted class label, bounding-boxes and confidence\ - \ score.\nBrick is designed to work with pre-trained models provided by framework\ - \ or with custom object detection models trained on Edge Impulse platform. \n" - require_container: true - require_model: true - ports: [] - model_name: yolox-object-detection - variables: - - name: CUSTOM_MODEL_PATH - default_value: /opt/models/ei/ - description: path to the custom model directory - - name: EI_OBJ_DETECTION_MODEL - default_value: /models/ootb/ei/yolo-x-nano.eim - description: path to the model file -- id: arduino:weather_forecast - name: Weather Forecast - description: Online weather forecast module for Arduino using open-meteo.com geolocation - and weather APIs. Requires an internet connection. - require_container: false - require_model: false - ports: [] -- id: arduino:visual_anomaly_detection - name: Visual Anomaly Detection - description: "Brick for visual anomaly detection using a pre-trained model. It processes\ - \ images and returns detected anomalies and bounding-boxes.\nBrick is designed\ - \ to work with pre-trained models provided by framework or with custom object\ - \ detection models trained on Edge Impulse platform. \n" - require_container: true - require_model: true - ports: [] - model_name: concreate-crack-anomaly-detection - variables: - - name: CUSTOM_MODEL_PATH - default_value: /opt/models/ei/ - description: path to the custom model directory - - name: EI_V_ANOMALY_DETECTION_MODEL - default_value: /models/ootb/ei/concrete-crack-anomaly-detection.eim - description: path to the model file -` - - var index BricksIndex - err := yaml.Unmarshal([]byte(x), &index) +func TestGenerateBricksIndexFromFile(t *testing.T) { + index, err := GenerateBricksIndexFromFile(paths.New("testdata")) require.NoError(t, err) - require.Len(t, index.Bricks, 11) // Check if ports are correctly set b, found := index.FindBrickByID("arduino:web_ui") @@ -195,13 +57,13 @@ func TestBricksIndexYAMLFormats(t *testing.T) { expectedBricks []Brick }{ { - // TODO: the validator of the brick-list must not allow this + // TODO: add a validator fo the bricks-list to validate the field name: "missing bricks field does not cuase error", yamlContent: `other_field: value`, expectedBricks: nil, }, { - name: "bad YAML format - invalid indentation", + name: "bad YAML format invalid indentation", yamlContent: `bricks: - id: arduino:test_brick name: Test Brick @@ -214,7 +76,7 @@ func TestBricksIndexYAMLFormats(t *testing.T) { expectedBricks: []Brick{}, }, { - name: "bad YAML format - unclosed quotes", + name: "bad YAML format unclosed quotes", yamlContent: `bricks: - id: "arduino:test_brick name: Test Brick @@ -222,14 +84,14 @@ func TestBricksIndexYAMLFormats(t *testing.T) { expectedError: "could not find end character of double-quoted text", }, { - name: "bad YAML format - missing colon", + name: "bad YAML format missing colon", yamlContent: `bricks: - id arduino:test_brick name: Test Brick`, expectedError: "unexpected key name", }, { - name: "bad YAML format - invalid syntax", + name: "bad YAML format invalid syntax", yamlContent: `bricks: - id: arduino:test_brick name: Test Brick @@ -238,7 +100,7 @@ func TestBricksIndexYAMLFormats(t *testing.T) { expectedError: "sequence end token ']' not found", }, { - name: "bad YAML format - tab characters", + name: "bad YAML format tab characters", yamlContent: "bricks:\n\t- id: arduino:test_brick\n\t name: Test Brick", expectedError: "found character '\t' that cannot start any token", }, diff --git a/internal/orchestrator/bricksindex/testdata/bricks-list.yaml b/internal/orchestrator/bricksindex/testdata/bricks-list.yaml new file mode 100644 index 00000000..0bd036f8 --- /dev/null +++ b/internal/orchestrator/bricksindex/testdata/bricks-list.yaml @@ -0,0 +1,133 @@ +bricks: +- id: arduino:image_classification + name: Image Classification + description: "Brick for image classification using a pre-trained model. It processes\ + \ images and returns the predicted class label and confidence score.\nBrick is\ + \ designed to work with pre-trained models provided by framework or with custom\ + \ image classification models trained on Edge Impulse platform. \n" + require_container: true + require_model: true + ports: [] + model_name: mobilenet-image-classification + variables: + - name: CUSTOM_MODEL_PATH + default_value: /opt/models/ei/ + description: path to the custom model directory + - name: EI_CLASSIFICATION_MODEL + default_value: /models/ootb/ei/mobilenet-v2-224px.eim + description: path to the model file +- id: arduino:camera_scanner + name: Camera Scanner + description: Scans a camera for barcodes and QR codes + require_container: false + require_model: false + ports: [] +- id: arduino:streamlit_ui + name: Streamlit UI + description: A simplified user interface based on Streamlit and Python. + require_container: false + require_model: false + ports: + - 7000 +- id: arduino:keyword_spotter + name: Keyword Spotter + description: 'Brick for keyword spotting using a pre-trained model. It processes + audio input to detect specific keywords or phrases. + + Brick is designed to work with pre-trained models provided by framework or with + custom audio classification models trained on Edge Impulse platform. + + ' + require_container: true + require_model: true + ports: [] + model_name: keyword-spotting-hello-world + variables: + - name: CUSTOM_MODEL_PATH + default_value: /opt/models/ei/ + description: path to the custom model directory + - name: EI_KEYWORK_SPOTTING_MODEL + default_value: /models/ootb/ei/keyword-spotting-hello-world.eim + description: path to the model file +- id: arduino:mqtt + name: MQTT Connector + description: MQTT connector module. Acts as a client for receiving and publishing + messages to an MQTT broker. + require_container: false + require_model: false + ports: [] +- id: arduino:web_ui + name: Web UI + description: A user interface based on HTML and JavaScript that can rely on additional + APIs and a WebSocket exposed by a web server. + require_container: false + require_model: false + ports: + - 7000 +- id: arduino:dbstorage_tsstore + name: Database Storage - Time Series Store + description: Simplified time series database storage layer for Arduino sensor samples + built on top of InfluxDB. + require_container: true + require_model: false + ports: [] + variables: + - name: APP_HOME + default_value: . + - name: DB_PASSWORD + default_value: Arduino15 + description: Database password + - name: DB_USERNAME + default_value: admin + description: Edge Impulse project API key + - name: INFLUXDB_ADMIN_TOKEN + default_value: 392edbf2-b8a2-481f-979d-3f188b2c05f0 + description: InfluxDB admin token +- id: arduino:dbstorage_sqlstore + name: Database Storage - SQLStore + description: Simplified database storage layer for Arduino sensor data using SQLite + local database. + require_container: false + require_model: false + ports: [] +- id: arduino:object_detection + name: Object Detection + description: "Brick for object detection using a pre-trained model. It processes\ + \ images and returns the predicted class label, bounding-boxes and confidence\ + \ score.\nBrick is designed to work with pre-trained models provided by framework\ + \ or with custom object detection models trained on Edge Impulse platform. \n" + require_container: true + require_model: true + ports: [] + model_name: yolox-object-detection + variables: + - name: CUSTOM_MODEL_PATH + default_value: /opt/models/ei/ + description: path to the custom model directory + - name: EI_OBJ_DETECTION_MODEL + default_value: /models/ootb/ei/yolo-x-nano.eim + description: path to the model file +- id: arduino:weather_forecast + name: Weather Forecast + description: Online weather forecast module for Arduino using open-meteo.com geolocation + and weather APIs. Requires an internet connection. + require_container: false + require_model: false + ports: [] +- id: arduino:visual_anomaly_detection + name: Visual Anomaly Detection + description: "Brick for visual anomaly detection using a pre-trained model. It processes\ + \ images and returns detected anomalies and bounding-boxes.\nBrick is designed\ + \ to work with pre-trained models provided by framework or with custom object\ + \ detection models trained on Edge Impulse platform. \n" + require_container: true + require_model: true + ports: [] + model_name: concreate-crack-anomaly-detection + variables: + - name: CUSTOM_MODEL_PATH + default_value: /opt/models/ei/ + description: path to the custom model directory + - name: EI_V_ANOMALY_DETECTION_MODEL + default_value: /models/ootb/ei/concrete-crack-anomaly-detection.eim + description: path to the model file \ No newline at end of file From 3ff4767fd0c8f476ad83f99bfe431f613a6f1fba Mon Sep 17 00:00:00 2001 From: dido18 Date: Tue, 25 Nov 2025 17:41:28 +0100 Subject: [PATCH 03/10] refactor: rename index generation functions to Load for consistency --- .../internal/servicelocator/servicelocator.go | 4 ++-- internal/e2e/daemon/brick_test.go | 2 +- internal/orchestrator/bricks/bricks_test.go | 10 +++++----- internal/orchestrator/bricksindex/bricks_index.go | 2 +- internal/orchestrator/bricksindex/bricks_index_test.go | 4 ++-- internal/orchestrator/modelsindex/models_index.go | 2 +- internal/orchestrator/modelsindex/modelsindex_test.go | 4 ++-- internal/orchestrator/orchestrator_test.go | 8 ++++---- internal/orchestrator/provision_test.go | 4 ++-- 9 files changed, 20 insertions(+), 20 deletions(-) diff --git a/cmd/arduino-app-cli/internal/servicelocator/servicelocator.go b/cmd/arduino-app-cli/internal/servicelocator/servicelocator.go index edcd721d..22cbc58b 100644 --- a/cmd/arduino-app-cli/internal/servicelocator/servicelocator.go +++ b/cmd/arduino-app-cli/internal/servicelocator/servicelocator.go @@ -42,11 +42,11 @@ func Init(cfg config.Configuration) { var ( GetBricksIndex = sync.OnceValue(func() *bricksindex.BricksIndex { - return f.Must(bricksindex.GenerateBricksIndexFromFile(GetStaticStore().GetAssetsFolder())) + return f.Must(bricksindex.Load(GetStaticStore().GetAssetsFolder())) }) GetModelsIndex = sync.OnceValue(func() *modelsindex.ModelsIndex { - return f.Must(modelsindex.GenerateModelsIndexFromFile(GetStaticStore().GetAssetsFolder())) + return f.Must(modelsindex.Load(GetStaticStore().GetAssetsFolder())) }) GetProvisioner = sync.OnceValue(func() *orchestrator.Provision { diff --git a/internal/e2e/daemon/brick_test.go b/internal/e2e/daemon/brick_test.go index b5f2b6d8..02736884 100644 --- a/internal/e2e/daemon/brick_test.go +++ b/internal/e2e/daemon/brick_test.go @@ -72,7 +72,7 @@ func TestBricksList(t *testing.T) { require.NoError(t, err) staticStore := store.NewStaticStore(paths.New("testdata", "assets", cfg.RunnerVersion).String()) - brickIndex, err := bricksindex.GenerateBricksIndexFromFile(staticStore.GetAssetsFolder()) + brickIndex, err := bricksindex.Load(staticStore.GetAssetsFolder()) require.NoError(t, err) // Compare the response with the bricks index diff --git a/internal/orchestrator/bricks/bricks_test.go b/internal/orchestrator/bricks/bricks_test.go index f9804b25..7d116724 100644 --- a/internal/orchestrator/bricks/bricks_test.go +++ b/internal/orchestrator/bricks/bricks_test.go @@ -32,7 +32,7 @@ import ( ) func TestBrickCreate(t *testing.T) { - bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(paths.New("testdata")) + bricksIndex, err := bricksindex.Load(paths.New("testdata")) require.Nil(t, err) brickService := NewService(nil, bricksIndex, nil) @@ -102,7 +102,7 @@ func TestBrickCreate(t *testing.T) { require.Nil(t, err) err = paths.New("testdata/dummy-app").CopyDirTo(tempDummyApp) require.Nil(t, err) - bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(paths.New("testdata")) + bricksIndex, err := bricksindex.Load(paths.New("testdata")) require.Nil(t, err) brickService := NewService(nil, bricksIndex, nil) @@ -129,7 +129,7 @@ func TestBrickCreate(t *testing.T) { } func TestUpdateBrick(t *testing.T) { - bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(paths.New("testdata")) + bricksIndex, err := bricksindex.Load(paths.New("testdata")) require.Nil(t, err) brickService := NewService(nil, bricksIndex, nil) @@ -189,7 +189,7 @@ func TestUpdateBrick(t *testing.T) { tempDummyApp := paths.New("testdata/dummy-app.temp") require.Nil(t, tempDummyApp.RemoveAll()) require.Nil(t, paths.New("testdata/dummy-app").CopyDirTo(tempDummyApp)) - bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(paths.New("testdata")) + bricksIndex, err := bricksindex.Load(paths.New("testdata")) require.Nil(t, err) brickService := NewService(nil, bricksIndex, nil) @@ -218,7 +218,7 @@ func TestUpdateBrick(t *testing.T) { tempDummyApp := paths.New("testdata/dummy-app-for-update.temp") require.Nil(t, tempDummyApp.RemoveAll()) require.Nil(t, paths.New("testdata/dummy-app-for-update").CopyDirTo(tempDummyApp)) - bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(paths.New("testdata")) + bricksIndex, err := bricksindex.Load(paths.New("testdata")) require.Nil(t, err) brickService := NewService(nil, bricksIndex, nil) diff --git a/internal/orchestrator/bricksindex/bricks_index.go b/internal/orchestrator/bricksindex/bricks_index.go index e4b774f4..9e52f1c6 100644 --- a/internal/orchestrator/bricksindex/bricks_index.go +++ b/internal/orchestrator/bricksindex/bricks_index.go @@ -91,7 +91,7 @@ func unmarshalBricksIndex(content io.Reader) (*BricksIndex, error) { return &index, nil } -func GenerateBricksIndexFromFile(dir *paths.Path) (*BricksIndex, error) { +func Load(dir *paths.Path) (*BricksIndex, error) { content, err := dir.Join("bricks-list.yaml").Open() if err != nil { return nil, err diff --git a/internal/orchestrator/bricksindex/bricks_index_test.go b/internal/orchestrator/bricksindex/bricks_index_test.go index 8656c125..8b02c8e0 100644 --- a/internal/orchestrator/bricksindex/bricks_index_test.go +++ b/internal/orchestrator/bricksindex/bricks_index_test.go @@ -24,7 +24,7 @@ import ( ) func TestGenerateBricksIndexFromFile(t *testing.T) { - index, err := GenerateBricksIndexFromFile(paths.New("testdata")) + index, err := Load(paths.New("testdata")) require.NoError(t, err) // Check if ports are correctly set @@ -189,7 +189,7 @@ func TestBricksIndexYAMLFormats(t *testing.T) { err := os.WriteFile(brickIndex.String(), []byte(tc.yamlContent), 0600) require.NoError(t, err) - index, err := GenerateBricksIndexFromFile(paths.New(tempDir)) + index, err := Load(paths.New(tempDir)) if tc.expectedError != "" { require.Error(t, err) require.Contains(t, err.Error(), tc.expectedError) diff --git a/internal/orchestrator/modelsindex/models_index.go b/internal/orchestrator/modelsindex/models_index.go index c9a47347..c24eb203 100644 --- a/internal/orchestrator/modelsindex/models_index.go +++ b/internal/orchestrator/modelsindex/models_index.go @@ -95,7 +95,7 @@ func (m *ModelsIndex) GetModelsByBricks(bricks []string) []AIModel { return matchingModels } -func GenerateModelsIndexFromFile(dir *paths.Path) (*ModelsIndex, error) { +func Load(dir *paths.Path) (*ModelsIndex, error) { content, err := dir.Join("models-list.yaml").ReadFile() if err != nil { return nil, err diff --git a/internal/orchestrator/modelsindex/modelsindex_test.go b/internal/orchestrator/modelsindex/modelsindex_test.go index 53ffb585..7f760390 100644 --- a/internal/orchestrator/modelsindex/modelsindex_test.go +++ b/internal/orchestrator/modelsindex/modelsindex_test.go @@ -9,7 +9,7 @@ import ( ) func TestModelsIndex(t *testing.T) { - modelsIndex, err := GenerateModelsIndexFromFile(paths.New("testdata")) + modelsIndex, err := Load(paths.New("testdata")) require.NoError(t, err) require.NotNil(t, modelsIndex) @@ -40,7 +40,7 @@ func TestModelsIndex(t *testing.T) { t.Run("it fails if model-list.yaml does not exist", func(t *testing.T) { nonExistentPath := paths.New("nonexistentdir") - modelsIndex, err := GenerateModelsIndexFromFile(nonExistentPath) + modelsIndex, err := Load(nonExistentPath) assert.Error(t, err) assert.Nil(t, modelsIndex) }) diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index 223b4705..df539ab9 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -461,7 +461,7 @@ bricks: `) err = cfg.AssetsDir().Join("bricks-list.yaml").WriteFile(bricksIndexContent) require.NoError(t, err) - bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(cfg.AssetsDir()) + bricksIndex, err := bricksindex.Load(cfg.AssetsDir()) assert.NoError(t, err) modelsIndexContent := []byte(` @@ -483,7 +483,7 @@ models: `) err = cfg.AssetsDir().Join("models-list.yaml").WriteFile(modelsIndexContent) require.NoError(t, err) - modelIndex, err := modelsindex.GenerateModelsIndexFromFile(cfg.AssetsDir()) + modelIndex, err := modelsindex.Load(cfg.AssetsDir()) require.NoError(t, err) env := getAppEnvironmentVariables(appDesc, bricksIndex, modelIndex) @@ -546,7 +546,7 @@ bricks: `) err = cfg.AssetsDir().Join("bricks-list.yaml").WriteFile(bricksIndexContent) require.NoError(t, err) - bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(cfg.AssetsDir()) + bricksIndex, err := bricksindex.Load(cfg.AssetsDir()) assert.NoError(t, err) modelsIndexContent := []byte(` @@ -568,7 +568,7 @@ models: `) err = cfg.AssetsDir().Join("models-list.yaml").WriteFile(modelsIndexContent) require.NoError(t, err) - modelIndex, err := modelsindex.GenerateModelsIndexFromFile(cfg.AssetsDir()) + modelIndex, err := modelsindex.Load(cfg.AssetsDir()) require.NoError(t, err) env := getAppEnvironmentVariables(appDesc, bricksIndex, modelIndex) diff --git a/internal/orchestrator/provision_test.go b/internal/orchestrator/provision_test.go index b2ca0bb0..1bd7aa65 100644 --- a/internal/orchestrator/provision_test.go +++ b/internal/orchestrator/provision_test.go @@ -104,7 +104,7 @@ bricks: require.NoError(t, err) // Override brick index with custom test content - bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(cfg.AssetsDir()) + bricksIndex, err := bricksindex.Load(cfg.AssetsDir()) require.Nil(t, err, "Failed to load bricks index with custom content") br, ok := bricksIndex.FindBrickByID("arduino:video_object_detection") @@ -301,7 +301,7 @@ bricks: err := cfg.AssetsDir().Join("bricks-list.yaml").WriteFile(bricksIndexContent) require.NoError(t, err) - bricksIndex, err := bricksindex.GenerateBricksIndexFromFile(cfg.AssetsDir()) + bricksIndex, err := bricksindex.Load(cfg.AssetsDir()) require.Nil(t, err, "Failed to load bricks index with custom content") br, ok := bricksIndex.FindBrickByID("arduino:dbstorage_tsstore") require.True(t, ok, "Brick arduino:dbstorage_tsstore should exist in the index") From 8b1426af9b7ae8a222858129dc2b13f5732dde2d Mon Sep 17 00:00:00 2001 From: dido18 Date: Wed, 26 Nov 2025 11:18:37 +0100 Subject: [PATCH 04/10] use paths.path ad argument in app.load --- cmd/arduino-app-cli/app/app.go | 2 +- internal/api/handlers/app_delete.go | 2 +- internal/api/handlers/app_details.go | 4 +-- internal/api/handlers/app_logs.go | 2 +- internal/api/handlers/app_ports.go | 2 +- internal/api/handlers/app_sketch_libs.go | 6 ++-- internal/api/handlers/app_start.go | 2 +- internal/api/handlers/app_stop.go | 2 +- internal/api/handlers/bricks.go | 10 +++--- internal/orchestrator/app/app.go | 6 ++-- internal/orchestrator/app/app_test.go | 12 +++---- internal/orchestrator/app/parser_test.go | 2 +- internal/orchestrator/app_status.go | 2 +- internal/orchestrator/bricks/bricks.go | 2 +- internal/orchestrator/bricks/bricks_test.go | 38 ++++++++++----------- internal/orchestrator/helpers.go | 2 +- internal/orchestrator/orchestrator.go | 4 +-- internal/orchestrator/orchestrator_test.go | 22 ++++++------ 18 files changed, 61 insertions(+), 61 deletions(-) diff --git a/cmd/arduino-app-cli/app/app.go b/cmd/arduino-app-cli/app/app.go index d8aae2fb..fca105b8 100644 --- a/cmd/arduino-app-cli/app/app.go +++ b/cmd/arduino-app-cli/app/app.go @@ -50,5 +50,5 @@ func Load(idOrPath string) (app.ArduinoApp, error) { return app.ArduinoApp{}, fmt.Errorf("invalid app path: %s", idOrPath) } - return app.Load(id.ToPath().String()) + return app.Load(id.ToPath()) } diff --git a/internal/api/handlers/app_delete.go b/internal/api/handlers/app_delete.go index 6503d124..99d72cf1 100644 --- a/internal/api/handlers/app_delete.go +++ b/internal/api/handlers/app_delete.go @@ -42,7 +42,7 @@ func HandleAppDelete( return } - app, err := app.Load(id.ToPath().String()) + app, err := app.Load(id.ToPath()) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", id.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) diff --git a/internal/api/handlers/app_details.go b/internal/api/handlers/app_details.go index 72c74bdd..1795ae42 100644 --- a/internal/api/handlers/app_details.go +++ b/internal/api/handlers/app_details.go @@ -45,7 +45,7 @@ func HandleAppDetails( return } - app, err := app.Load(id.ToPath().String()) + app, err := app.Load(id.ToPath()) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", id.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) @@ -81,7 +81,7 @@ func HandleAppDetailsEdits( render.EncodeResponse(w, http.StatusPreconditionFailed, models.ErrorResponse{Details: "invalid id"}) return } - appToEdit, err := app.Load(id.ToPath().String()) + appToEdit, err := app.Load(id.ToPath()) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", id.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) diff --git a/internal/api/handlers/app_logs.go b/internal/api/handlers/app_logs.go index 2cf44c87..0fece050 100644 --- a/internal/api/handlers/app_logs.go +++ b/internal/api/handlers/app_logs.go @@ -43,7 +43,7 @@ func HandleAppLogs( return } - app, err := app.Load(id.ToPath().String()) + app, err := app.Load(id.ToPath()) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", id.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) diff --git a/internal/api/handlers/app_ports.go b/internal/api/handlers/app_ports.go index 8671cd7e..c405c3a3 100644 --- a/internal/api/handlers/app_ports.go +++ b/internal/api/handlers/app_ports.go @@ -47,7 +47,7 @@ func HandleAppPorts( return } - app, err := app.Load(id.ToPath().String()) + app, err := app.Load(id.ToPath()) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", id.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) diff --git a/internal/api/handlers/app_sketch_libs.go b/internal/api/handlers/app_sketch_libs.go index 43969572..7e280a2f 100644 --- a/internal/api/handlers/app_sketch_libs.go +++ b/internal/api/handlers/app_sketch_libs.go @@ -36,7 +36,7 @@ func HandleSketchAddLibrary(idProvider *app.IDProvider) http.HandlerFunc { render.EncodeResponse(w, http.StatusBadRequest, models.ErrorResponse{Details: "cannot alter examples"}) return } - app, err := app.Load(id.ToPath().String()) + app, err := app.Load(id.ToPath()) // Get query param addDeps (default false) addDeps, _ := strconv.ParseBool(r.URL.Query().Get("add_deps")) @@ -78,7 +78,7 @@ func HandleSketchRemoveLibrary(idProvider *app.IDProvider) http.HandlerFunc { render.EncodeResponse(w, http.StatusBadRequest, models.ErrorResponse{Details: "cannot alter examples"}) return } - app, err := app.Load(id.ToPath().String()) + app, err := app.Load(id.ToPath()) if err != nil { render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) return @@ -114,7 +114,7 @@ func HandleSketchListLibraries(idProvider *app.IDProvider) http.HandlerFunc { render.EncodeResponse(w, http.StatusPreconditionFailed, models.ErrorResponse{Details: "invalid id"}) return } - app, err := app.Load(id.ToPath().String()) + app, err := app.Load(id.ToPath()) if err != nil { render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) return diff --git a/internal/api/handlers/app_start.go b/internal/api/handlers/app_start.go index 0293aff0..bfd8a034 100644 --- a/internal/api/handlers/app_start.go +++ b/internal/api/handlers/app_start.go @@ -47,7 +47,7 @@ func HandleAppStart( return } - app, err := app.Load(id.ToPath().String()) + app, err := app.Load(id.ToPath()) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", id.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) diff --git a/internal/api/handlers/app_stop.go b/internal/api/handlers/app_stop.go index f5a9979c..a013ef9e 100644 --- a/internal/api/handlers/app_stop.go +++ b/internal/api/handlers/app_stop.go @@ -38,7 +38,7 @@ func HandleAppStop( return } - app, err := app.Load(id.ToPath().String()) + app, err := app.Load(id.ToPath()) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", id.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) diff --git a/internal/api/handlers/bricks.go b/internal/api/handlers/bricks.go index 9ac76632..f3d22c6d 100644 --- a/internal/api/handlers/bricks.go +++ b/internal/api/handlers/bricks.go @@ -55,7 +55,7 @@ func HandleAppBrickInstancesList( } appPath := appId.ToPath() - app, err := app.Load(appPath.String()) + app, err := app.Load(appPath) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", appId.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) @@ -85,7 +85,7 @@ func HandleAppBrickInstanceDetails( } appPath := appId.ToPath() - app, err := app.Load(appPath.String()) + app, err := app.Load(appPath) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", appId.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) @@ -120,7 +120,7 @@ func HandleBrickCreate( } appPath := appId.ToPath() - app, err := app.Load(appPath.String()) + app, err := app.Load(appPath) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", appId.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) @@ -190,7 +190,7 @@ func HandleBrickUpdates( } appPath := appId.ToPath() - app, err := app.Load(appPath.String()) + app, err := app.Load(appPath) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", appId.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) @@ -236,7 +236,7 @@ func HandleBrickDelete( } appPath := appId.ToPath() - app, err := app.Load(appPath.String()) + app, err := app.Load(appPath) if err != nil { slog.Error("Unable to parse the app.yaml", slog.String("error", err.Error()), slog.String("path", appId.String())) render.EncodeResponse(w, http.StatusInternalServerError, models.ErrorResponse{Details: "unable to find the app"}) diff --git a/internal/orchestrator/app/app.go b/internal/orchestrator/app/app.go index c40c9009..0acaa01e 100644 --- a/internal/orchestrator/app/app.go +++ b/internal/orchestrator/app/app.go @@ -37,10 +37,10 @@ type ArduinoApp struct { // Load creates an App instance by reading all the files composing an app and grouping them // by file type. -func Load(appPath string) (ArduinoApp, error) { - path := paths.New(appPath) +// TODO: use the *paths.Path as argument +func Load(path *paths.Path) (ArduinoApp, error) { if path == nil { - return ArduinoApp{}, errors.New("empty app path") + return ArduinoApp{}, errors.New("path cannot be empty") } exist, err := path.IsDirCheck() diff --git a/internal/orchestrator/app/app_test.go b/internal/orchestrator/app/app_test.go index 47e3f53f..58e5aa81 100644 --- a/internal/orchestrator/app/app_test.go +++ b/internal/orchestrator/app/app_test.go @@ -26,26 +26,26 @@ import ( func TestLoad(t *testing.T) { t.Run("it fails if the app path is empty", func(t *testing.T) { - app, err := Load("") + app, err := Load(nil) assert.Error(t, err) assert.Empty(t, app) assert.Contains(t, err.Error(), "empty app path") }) t.Run("it fails if the app path exist but it's a file", func(t *testing.T) { - _, err := Load("testdata/app.yaml") + _, err := Load(paths.New("testdata/app.yaml")) assert.Error(t, err) assert.Contains(t, err.Error(), "app path must be a directory") }) t.Run("it fails if the app path does not exist", func(t *testing.T) { - _, err := Load("testdata/this-folder-does-not-exist") + _, err := Load(paths.New("testdata/this-folder-does-not-exist")) assert.Error(t, err) assert.Contains(t, err.Error(), "app path is not valid") }) t.Run("it loads an app correctly", func(t *testing.T) { - app, err := Load("testdata/AppSimple") + app, err := Load(paths.New("testdata/AppSimple")) assert.NoError(t, err) assert.NotEmpty(t, app) @@ -61,7 +61,7 @@ func TestMissingDescriptor(t *testing.T) { appFolderPath := paths.New("testdata", "MissingDescriptor") // Load app - app, err := Load(appFolderPath.String()) + app, err := Load(appFolderPath) assert.Error(t, err) assert.ErrorContains(t, err, "descriptor app.yaml file missing from app") assert.Empty(t, app) @@ -71,7 +71,7 @@ func TestMissingMains(t *testing.T) { appFolderPath := paths.New("testdata", "MissingMains") // Load app - app, err := Load(appFolderPath.String()) + app, err := Load(appFolderPath) assert.Error(t, err) assert.ErrorContains(t, err, "main python file and sketch file missing from app") assert.Empty(t, app) diff --git a/internal/orchestrator/app/parser_test.go b/internal/orchestrator/app/parser_test.go index 123d38f4..bf4c6b73 100644 --- a/internal/orchestrator/app/parser_test.go +++ b/internal/orchestrator/app/parser_test.go @@ -115,7 +115,7 @@ bricks: err = os.WriteFile(appYaml.String(), []byte(appDescriptor), 0600) require.NoError(t, err) - app, err := Load(tempDir) + app, err := Load(appYaml) require.NoError(t, err) require.Equal(t, "Test App", app.Name) require.Equal(t, 1, len(app.Descriptor.Bricks)) diff --git a/internal/orchestrator/app_status.go b/internal/orchestrator/app_status.go index 481e240f..7e9c32fc 100644 --- a/internal/orchestrator/app_status.go +++ b/internal/orchestrator/app_status.go @@ -97,7 +97,7 @@ func parseDockerStatusEvent(ctx context.Context, cfg config.Configuration, docke } // FIXME: create an helper function to transform an app.ArduinoApp into an ortchestrator.AppInfo - app, err := app.Load(appStatus.AppPath.String()) + app, err := app.Load(appStatus.AppPath) if err != nil { slog.Warn("error loading app", "appPath", appStatus.AppPath.String(), "error", err) return AppInfo{}, err diff --git a/internal/orchestrator/bricks/bricks.go b/internal/orchestrator/bricks/bricks.go index 3b722b3d..cc9128c7 100644 --- a/internal/orchestrator/bricks/bricks.go +++ b/internal/orchestrator/bricks/bricks.go @@ -247,7 +247,7 @@ func getUsedByApps( } for _, file := range appPaths { - app, err := app.Load(file.String()) + app, err := app.Load(file) if err != nil { // we are not considering the broken apps slog.Warn("unable to parse app.yaml, skipping", "path", file.String(), "error", err.Error()) diff --git a/internal/orchestrator/bricks/bricks_test.go b/internal/orchestrator/bricks/bricks_test.go index 7d116724..4fee6fde 100644 --- a/internal/orchestrator/bricks/bricks_test.go +++ b/internal/orchestrator/bricks/bricks_test.go @@ -37,7 +37,7 @@ func TestBrickCreate(t *testing.T) { brickService := NewService(nil, bricksIndex, nil) t.Run("fails if brick id does not exist", func(t *testing.T) { - err = brickService.BrickCreate(BrickCreateUpdateRequest{ID: "not-existing-id"}, f.Must(app.Load("testdata/dummy-app"))) + err = brickService.BrickCreate(BrickCreateUpdateRequest{ID: "not-existing-id"}, f.Must(app.Load(paths.New("testdata/dummy-app")))) require.Error(t, err) require.Equal(t, "brick \"not-existing-id\" not found", err.Error()) }) @@ -46,7 +46,7 @@ func TestBrickCreate(t *testing.T) { req := BrickCreateUpdateRequest{ID: "arduino:arduino_cloud", Variables: map[string]string{ "NON_EXISTING_VARIABLE": "some-value", }} - err = brickService.BrickCreate(req, f.Must(app.Load("testdata/dummy-app"))) + err = brickService.BrickCreate(req, f.Must(app.Load(paths.New("testdata/dummy-app")))) require.Error(t, err) require.Equal(t, "variable \"NON_EXISTING_VARIABLE\" does not exist on brick \"arduino:arduino_cloud\"", err.Error()) }) @@ -56,7 +56,7 @@ func TestBrickCreate(t *testing.T) { "ARDUINO_DEVICE_ID": "", "ARDUINO_SECRET": "a-secret-a", }} - err = brickService.BrickCreate(req, f.Must(app.Load("testdata/dummy-app"))) + err = brickService.BrickCreate(req, f.Must(app.Load(paths.New("testdata/dummy-app")))) require.Error(t, err) require.Equal(t, "required variable \"ARDUINO_DEVICE_ID\" cannot be empty", err.Error()) }) @@ -70,10 +70,10 @@ func TestBrickCreate(t *testing.T) { req := BrickCreateUpdateRequest{ID: "arduino:arduino_cloud", Variables: map[string]string{ "ARDUINO_SECRET": "a-secret-a", }} - err = brickService.BrickCreate(req, f.Must(app.Load(tempDummyApp.String()))) + err = brickService.BrickCreate(req, f.Must(app.Load(tempDummyApp))) require.NoError(t, err) - after, err := app.Load(tempDummyApp.String()) + after, err := app.Load(tempDummyApp) require.Nil(t, err) require.Len(t, after.Descriptor.Bricks, 1) require.Equal(t, "arduino:arduino_cloud", after.Descriptor.Bricks[0].ID) @@ -88,9 +88,9 @@ func TestBrickCreate(t *testing.T) { require.Nil(t, paths.New("testdata/dummy-app").CopyDirTo(tempDummyApp)) req := BrickCreateUpdateRequest{ID: "arduino:dbstorage_sqlstore"} - err = brickService.BrickCreate(req, f.Must(app.Load(tempDummyApp.String()))) + err = brickService.BrickCreate(req, f.Must(app.Load(tempDummyApp))) require.Nil(t, err) - after, err := app.Load(tempDummyApp.String()) + after, err := app.Load(tempDummyApp) require.Nil(t, err) require.Len(t, after.Descriptor.Bricks, 2) require.Equal(t, "arduino:dbstorage_sqlstore", after.Descriptor.Bricks[1].ID) @@ -116,10 +116,10 @@ func TestBrickCreate(t *testing.T) { }, } - err = brickService.BrickCreate(req, f.Must(app.Load(tempDummyApp.String()))) + err = brickService.BrickCreate(req, f.Must(app.Load(tempDummyApp))) require.Nil(t, err) - after, err := app.Load(tempDummyApp.String()) + after, err := app.Load(tempDummyApp) require.Nil(t, err) require.Len(t, after.Descriptor.Bricks, 1) require.Equal(t, "arduino:arduino_cloud", after.Descriptor.Bricks[0].ID) @@ -134,13 +134,13 @@ func TestUpdateBrick(t *testing.T) { brickService := NewService(nil, bricksIndex, nil) t.Run("fails if brick id does not exist into brick index", func(t *testing.T) { - err = brickService.BrickUpdate(BrickCreateUpdateRequest{ID: "not-existing-id"}, f.Must(app.Load("testdata/dummy-app"))) + err = brickService.BrickUpdate(BrickCreateUpdateRequest{ID: "not-existing-id"}, f.Must(app.Load(paths.New("testdata/dummy-app")))) require.Error(t, err) require.Equal(t, "brick \"not-existing-id\" not found into the brick index", err.Error()) }) t.Run("fails if brick is present into the index but not in the app ", func(t *testing.T) { - err = brickService.BrickUpdate(BrickCreateUpdateRequest{ID: "arduino:dbstorage_sqlstore"}, f.Must(app.Load("testdata/dummy-app"))) + err = brickService.BrickUpdate(BrickCreateUpdateRequest{ID: "arduino:dbstorage_sqlstore"}, f.Must(app.Load(paths.New("testdata/dummy-app")))) require.Error(t, err) require.Equal(t, "brick \"arduino:dbstorage_sqlstore\" not found into the bricks of the app", err.Error()) }) @@ -149,7 +149,7 @@ func TestUpdateBrick(t *testing.T) { req := BrickCreateUpdateRequest{ID: "arduino:arduino_cloud", Variables: map[string]string{ "NON_EXISTING_VARIABLE": "some-value", }} - err = brickService.BrickUpdate(req, f.Must(app.Load("testdata/dummy-app"))) + err = brickService.BrickUpdate(req, f.Must(app.Load(paths.New("testdata/dummy-app")))) require.Error(t, err) require.Equal(t, "variable \"NON_EXISTING_VARIABLE\" does not exist on brick \"arduino:arduino_cloud\"", err.Error()) }) @@ -160,7 +160,7 @@ func TestUpdateBrick(t *testing.T) { "ARDUINO_DEVICE_ID": "", "ARDUINO_SECRET": "a-secret-a", }} - err = brickService.BrickUpdate(req, f.Must(app.Load("testdata/dummy-app"))) + err = brickService.BrickUpdate(req, f.Must(app.Load(paths.New("testdata/dummy-app")))) require.Error(t, err) require.Equal(t, "required variable \"ARDUINO_DEVICE_ID\" cannot be empty", err.Error()) }) @@ -174,10 +174,10 @@ func TestUpdateBrick(t *testing.T) { req := BrickCreateUpdateRequest{ID: "arduino:arduino_cloud", Variables: map[string]string{ "ARDUINO_SECRET": "a-secret-a", }} - err = brickService.BrickUpdate(req, f.Must(app.Load(tempDummyApp.String()))) + err = brickService.BrickUpdate(req, f.Must(app.Load(tempDummyApp))) require.NoError(t, err) - after, err := app.Load(tempDummyApp.String()) + after, err := app.Load(tempDummyApp) require.Nil(t, err) require.Len(t, after.Descriptor.Bricks, 1) require.Equal(t, "arduino:arduino_cloud", after.Descriptor.Bricks[0].ID) @@ -203,10 +203,10 @@ func TestUpdateBrick(t *testing.T) { }, } - err = brickService.BrickUpdate(req, f.Must(app.Load(tempDummyApp.String()))) + err = brickService.BrickUpdate(req, f.Must(app.Load(tempDummyApp))) require.Nil(t, err) - after, err := app.Load(tempDummyApp.String()) + after, err := app.Load(tempDummyApp) require.Nil(t, err) require.Len(t, after.Descriptor.Bricks, 1) require.Equal(t, "arduino:arduino_cloud", after.Descriptor.Bricks[0].ID) @@ -231,10 +231,10 @@ func TestUpdateBrick(t *testing.T) { }, } - err = brickService.BrickUpdate(req, f.Must(app.Load(tempDummyApp.String()))) + err = brickService.BrickUpdate(req, f.Must(app.Load(tempDummyApp))) require.Nil(t, err) - after, err := app.Load(tempDummyApp.String()) + after, err := app.Load(tempDummyApp) require.Nil(t, err) require.Len(t, after.Descriptor.Bricks, 1) require.Equal(t, "arduino:arduino_cloud", after.Descriptor.Bricks[0].ID) diff --git a/internal/orchestrator/helpers.go b/internal/orchestrator/helpers.go index 5a637b07..3b94b654 100644 --- a/internal/orchestrator/helpers.go +++ b/internal/orchestrator/helpers.go @@ -193,7 +193,7 @@ func getRunningApp( if idx == -1 { return nil, nil } - app, err := app.Load(apps[idx].AppPath.String()) + app, err := app.Load(apps[idx].AppPath) if err != nil { return nil, fmt.Errorf("failed to load running app: %w", err) } diff --git a/internal/orchestrator/orchestrator.go b/internal/orchestrator/orchestrator.go index 8eedff6f..1a61145c 100644 --- a/internal/orchestrator/orchestrator.go +++ b/internal/orchestrator/orchestrator.go @@ -614,7 +614,7 @@ func ListApps( } for _, file := range appPaths { - app, err := app.Load(file.String()) + app, err := app.Load(file) if err != nil { result.BrokenApps = append(result.BrokenApps, BrokenAppInfo{ Name: file.Base(), @@ -956,7 +956,7 @@ func GetDefaultApp(cfg config.Configuration) (*app.ArduinoApp, error) { return nil, nil } - app, err := app.Load(string(defaultAppPath)) + app, err := app.Load(paths.New(string(defaultAppPath))) if err != nil { // If the app is not valid, we remove the file slog.Warn("default app is not valid", slog.String("path", string(defaultAppPath)), slog.String("error", err.Error())) diff --git a/internal/orchestrator/orchestrator_test.go b/internal/orchestrator/orchestrator_test.go index df539ab9..d6c6a4dc 100644 --- a/internal/orchestrator/orchestrator_test.go +++ b/internal/orchestrator/orchestrator_test.go @@ -90,7 +90,7 @@ func TestCloneApp(t *testing.T) { }) // The app.yaml will have the name set to the new-name - clonedApp := f.Must(app.Load(appDir.String())) + clonedApp := f.Must(app.Load(appDir)) require.Equal(t, "new-name", clonedApp.Name) }) t.Run("with icon", func(t *testing.T) { @@ -108,7 +108,7 @@ func TestCloneApp(t *testing.T) { }) // The app.yaml will have the icon set to 🦄 - clonedApp := f.Must(app.Load(appDir.String())) + clonedApp := f.Must(app.Load(appDir)) require.Equal(t, "with-icon", clonedApp.Name) require.Equal(t, "🦄", clonedApp.Descriptor.Icon) }) @@ -164,7 +164,7 @@ func TestEditApp(t *testing.T) { appDir := cfg.AppsDir().Join("app-default") t.Run("previously not default", func(t *testing.T) { - app := f.Must(app.Load(appDir.String())) + app := f.Must(app.Load(appDir)) previousDefaultApp, err := GetDefaultApp(cfg) require.NoError(t, err) @@ -178,7 +178,7 @@ func TestEditApp(t *testing.T) { require.True(t, appDir.EquivalentTo(currentDefaultApp.FullPath)) }) t.Run("previously default", func(t *testing.T) { - app := f.Must(app.Load(appDir.String())) + app := f.Must(app.Load(appDir)) err := SetDefaultApp(&app, cfg) require.NoError(t, err) @@ -200,12 +200,12 @@ func TestEditApp(t *testing.T) { _, err := CreateApp(t.Context(), CreateAppRequest{Name: originalAppName}, idProvider, cfg) require.NoError(t, err) appDir := cfg.AppsDir().Join(originalAppName) - userApp := f.Must(app.Load(appDir.String())) + userApp := f.Must(app.Load(appDir)) originalPath := userApp.FullPath err = EditApp(AppEditRequest{Name: f.Ptr("new-name")}, &userApp, cfg) require.NoError(t, err) - editedApp, err := app.Load(cfg.AppsDir().Join("new-name").String()) + editedApp, err := app.Load(cfg.AppsDir().Join("new-name")) require.NoError(t, err) require.Equal(t, "new-name", editedApp.Name) require.True(t, originalPath.NotExist()) // The original app directory should be removed after renaming @@ -215,7 +215,7 @@ func TestEditApp(t *testing.T) { _, err := CreateApp(t.Context(), CreateAppRequest{Name: existingAppName}, idProvider, cfg) require.NoError(t, err) appDir := cfg.AppsDir().Join(existingAppName) - existingApp := f.Must(app.Load(appDir.String())) + existingApp := f.Must(app.Load(appDir)) err = EditApp(AppEditRequest{Name: f.Ptr(existingAppName)}, &existingApp, cfg) require.ErrorIs(t, err, ErrAppAlreadyExists) @@ -227,14 +227,14 @@ func TestEditApp(t *testing.T) { _, err := CreateApp(t.Context(), CreateAppRequest{Name: commonAppName}, idProvider, cfg) require.NoError(t, err) commonAppDir := cfg.AppsDir().Join(commonAppName) - commonApp := f.Must(app.Load(commonAppDir.String())) + commonApp := f.Must(app.Load(commonAppDir)) err = EditApp(AppEditRequest{ Icon: f.Ptr("💻"), Description: f.Ptr("new desc"), }, &commonApp, cfg) require.NoError(t, err) - editedApp := f.Must(app.Load(commonAppDir.String())) + editedApp := f.Must(app.Load(commonAppDir)) require.Equal(t, "new desc", editedApp.Descriptor.Description) require.Equal(t, "💻", editedApp.Descriptor.Icon) }) @@ -427,7 +427,7 @@ func TestGetAppEnvironmentVariablesWithDefaults(t *testing.T) { require.NoError(t, err) appId := createApp(t, "app1", false, idProvider, cfg) - appDesc, err := app.Load(appId.ToPath().String()) + appDesc, err := app.Load(appId.ToPath()) require.NoError(t, err) appDesc.Descriptor.Bricks = []app.Brick{ { @@ -512,7 +512,7 @@ func TestGetAppEnvironmentVariablesWithCustomModelOverrides(t *testing.T) { require.NoError(t, err) appId := createApp(t, "app1", false, idProvider, cfg) - appDesc, err := app.Load(appId.ToPath().String()) + appDesc, err := app.Load(appId.ToPath()) require.NoError(t, err) appDesc.Descriptor.Bricks = []app.Brick{ { From 825103e67ea8d5ed5bea097a3d1760b98513f52b Mon Sep 17 00:00:00 2001 From: dido18 Date: Wed, 26 Nov 2025 11:25:20 +0100 Subject: [PATCH 05/10] use dir instead of path in app.Load --- internal/orchestrator/app/app.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/internal/orchestrator/app/app.go b/internal/orchestrator/app/app.go index 0acaa01e..496fc6dd 100644 --- a/internal/orchestrator/app/app.go +++ b/internal/orchestrator/app/app.go @@ -37,26 +37,25 @@ type ArduinoApp struct { // Load creates an App instance by reading all the files composing an app and grouping them // by file type. -// TODO: use the *paths.Path as argument -func Load(path *paths.Path) (ArduinoApp, error) { - if path == nil { +func Load(dir *paths.Path) (ArduinoApp, error) { + if dir == nil { return ArduinoApp{}, errors.New("path cannot be empty") } - exist, err := path.IsDirCheck() + exist, err := dir.IsDirCheck() if err != nil { return ArduinoApp{}, fmt.Errorf("app path is not valid: %w", err) } if !exist { - return ArduinoApp{}, fmt.Errorf("app path must be a directory: %s", path) + return ArduinoApp{}, fmt.Errorf("app path must be a directory: %s", dir) } - path, err = path.Abs() + dir, err = dir.Abs() if err != nil { return ArduinoApp{}, fmt.Errorf("cannot get absolute path for app: %w", err) } app := ArduinoApp{ - FullPath: path, + FullPath: dir, Descriptor: AppDescriptor{}, } @@ -71,13 +70,13 @@ func Load(path *paths.Path) (ArduinoApp, error) { return ArduinoApp{}, errors.New("descriptor app.yaml file missing from app") } - if path.Join("python", "main.py").Exist() { - app.MainPythonFile = path.Join("python", "main.py") + if dir.Join("python", "main.py").Exist() { + app.MainPythonFile = dir.Join("python", "main.py") } - if path.Join("sketch", "sketch.ino").Exist() { + if dir.Join("sketch", "sketch.ino").Exist() { // TODO: check sketch casing? - app.MainSketchPath = path.Join("sketch") + app.MainSketchPath = dir.Join("sketch") } if app.MainPythonFile == nil && app.MainSketchPath == nil { From 0f35b9dc4fc91fd7ef1b61befd43308c6b484e4b Mon Sep 17 00:00:00 2001 From: dido18 Date: Wed, 26 Nov 2025 11:40:20 +0100 Subject: [PATCH 06/10] fix(tests): update Load function call to use paths.New for app.yaml --- internal/orchestrator/app/parser_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/orchestrator/app/parser_test.go b/internal/orchestrator/app/parser_test.go index bf4c6b73..fe8c8f59 100644 --- a/internal/orchestrator/app/parser_test.go +++ b/internal/orchestrator/app/parser_test.go @@ -115,7 +115,7 @@ bricks: err = os.WriteFile(appYaml.String(), []byte(appDescriptor), 0600) require.NoError(t, err) - app, err := Load(appYaml) + app, err := Load(paths.New(tempDir)) require.NoError(t, err) require.Equal(t, "Test App", app.Name) require.Equal(t, 1, len(app.Descriptor.Bricks)) From b973c2dd07b3c19540d69506013cbe04100314fd Mon Sep 17 00:00:00 2001 From: dido18 Date: Wed, 26 Nov 2025 11:43:24 +0100 Subject: [PATCH 07/10] fix(app): update error message for empty app path in Load function --- internal/orchestrator/app/app.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/orchestrator/app/app.go b/internal/orchestrator/app/app.go index 496fc6dd..c379ee60 100644 --- a/internal/orchestrator/app/app.go +++ b/internal/orchestrator/app/app.go @@ -39,7 +39,7 @@ type ArduinoApp struct { // by file type. func Load(dir *paths.Path) (ArduinoApp, error) { if dir == nil { - return ArduinoApp{}, errors.New("path cannot be empty") + return ArduinoApp{}, errors.New("empty app path") } exist, err := dir.IsDirCheck() From ce39859d7f4f99e00d7618a4031296e9eab2b63b Mon Sep 17 00:00:00 2001 From: Davide Date: Wed, 26 Nov 2025 12:03:33 +0100 Subject: [PATCH 08/10] Update internal/orchestrator/app/app_test.go Co-authored-by: Luca Rinaldi --- internal/orchestrator/app/app_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/orchestrator/app/app_test.go b/internal/orchestrator/app/app_test.go index 58e5aa81..963698e4 100644 --- a/internal/orchestrator/app/app_test.go +++ b/internal/orchestrator/app/app_test.go @@ -26,7 +26,7 @@ import ( func TestLoad(t *testing.T) { t.Run("it fails if the app path is empty", func(t *testing.T) { - app, err := Load(nil) + app, err := Load(path.New("")) assert.Error(t, err) assert.Empty(t, app) assert.Contains(t, err.Error(), "empty app path") From 5af6bb3e5bb510d982d4fb6942748f1bd7d77724 Mon Sep 17 00:00:00 2001 From: dido18 Date: Wed, 26 Nov 2025 12:04:15 +0100 Subject: [PATCH 09/10] fix(app): rename parameter from dir to appPath in Load function for clarity --- internal/orchestrator/app/app.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/internal/orchestrator/app/app.go b/internal/orchestrator/app/app.go index c379ee60..813ca64c 100644 --- a/internal/orchestrator/app/app.go +++ b/internal/orchestrator/app/app.go @@ -37,25 +37,25 @@ type ArduinoApp struct { // Load creates an App instance by reading all the files composing an app and grouping them // by file type. -func Load(dir *paths.Path) (ArduinoApp, error) { - if dir == nil { +func Load(appPath *paths.Path) (ArduinoApp, error) { + if appPath == nil { return ArduinoApp{}, errors.New("empty app path") } - exist, err := dir.IsDirCheck() + exist, err := appPath.IsDirCheck() if err != nil { return ArduinoApp{}, fmt.Errorf("app path is not valid: %w", err) } if !exist { - return ArduinoApp{}, fmt.Errorf("app path must be a directory: %s", dir) + return ArduinoApp{}, fmt.Errorf("app path must be a directory: %s", appPath) } - dir, err = dir.Abs() + appPath, err = appPath.Abs() if err != nil { return ArduinoApp{}, fmt.Errorf("cannot get absolute path for app: %w", err) } app := ArduinoApp{ - FullPath: dir, + FullPath: appPath, Descriptor: AppDescriptor{}, } @@ -70,13 +70,13 @@ func Load(dir *paths.Path) (ArduinoApp, error) { return ArduinoApp{}, errors.New("descriptor app.yaml file missing from app") } - if dir.Join("python", "main.py").Exist() { - app.MainPythonFile = dir.Join("python", "main.py") + if appPath.Join("python", "main.py").Exist() { + app.MainPythonFile = appPath.Join("python", "main.py") } - if dir.Join("sketch", "sketch.ino").Exist() { + if appPath.Join("sketch", "sketch.ino").Exist() { // TODO: check sketch casing? - app.MainSketchPath = dir.Join("sketch") + app.MainSketchPath = appPath.Join("sketch") } if app.MainPythonFile == nil && app.MainSketchPath == nil { From fe18e55607f1eb335b8fe2ec47235ab6c5f1e129 Mon Sep 17 00:00:00 2001 From: dido18 Date: Wed, 26 Nov 2025 12:05:42 +0100 Subject: [PATCH 10/10] fix(tests): add test case for nil app path in Load function --- internal/orchestrator/app/app_test.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/internal/orchestrator/app/app_test.go b/internal/orchestrator/app/app_test.go index 963698e4..db9cc048 100644 --- a/internal/orchestrator/app/app_test.go +++ b/internal/orchestrator/app/app_test.go @@ -25,8 +25,15 @@ import ( ) func TestLoad(t *testing.T) { + t.Run("it fails if the app path is nil", func(t *testing.T) { + app, err := Load(nil) + assert.Error(t, err) + assert.Empty(t, app) + assert.Contains(t, err.Error(), "empty app path") + }) + t.Run("it fails if the app path is empty", func(t *testing.T) { - app, err := Load(path.New("")) + app, err := Load(paths.New("")) assert.Error(t, err) assert.Empty(t, app) assert.Contains(t, err.Error(), "empty app path")