Skip to content

Commit ad54d74

Browse files
committed
fix: improve warning message for missing mandatory variables in BrickCreate and update test case descriptions
1 parent 289e6af commit ad54d74

File tree

2 files changed

+8
-7
lines changed

2 files changed

+8
-7
lines changed

internal/orchestrator/bricks/bricks.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,9 +290,7 @@ func (s *Service) BrickCreate(
290290
for _, brickVar := range brick.Variables {
291291
if brickVar.DefaultValue == "" {
292292
if _, exist := req.Variables[brickVar.Name]; !exist {
293-
// PATCH: to allow the AppLab to add a brick to a app created from scratch because currently
294-
// the FE does not send the required variables in the request.
295-
slog.Warn("[Skip] variable has no default value and it is not set by user", "variable", brickVar.Name)
293+
slog.Warn("[Skip] a required variable is not set by user", "variable", brickVar.Name)
296294
}
297295
}
298296
}

internal/orchestrator/bricks/bricks_test.go

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ func TestBrickCreate(t *testing.T) {
5959
require.Equal(t, "variable \"ARDUINO_DEVICE_ID\" cannot be empty", err.Error())
6060
})
6161

62-
t.Run("log a warning if a mandatory variable is not present in the request", func(t *testing.T) {
63-
tempApp, _ := copyToTempApp(t, paths.New("testdata/AppFromExample"))
64-
// defer cleanUp()
62+
t.Run("omit a mandatory variable is not present in the request", func(t *testing.T) {
63+
tempApp, cleanUp := copyToTempApp(t, paths.New("testdata/AppFromExample"))
64+
defer cleanUp()
6565

6666
req := BrickCreateUpdateRequest{ID: "arduino:arduino_cloud", Variables: map[string]string{
6767
"ARDUINO_SECRET": "a-secret-a",
@@ -73,7 +73,10 @@ func TestBrickCreate(t *testing.T) {
7373
require.Nil(t, err)
7474
require.Len(t, after.Descriptor.Bricks, 1)
7575
require.Equal(t, "arduino:arduino_cloud", after.Descriptor.Bricks[0].ID)
76-
require.Equal(t, "", after.Descriptor.Bricks[0].Variables["ARDUINO_DEVICE_ID"]) // <-- the DEVICE_ID is empty
76+
// NOTE: currently it is not possible to distinguish a field with empty string or missing field into the yaml.
77+
// The 'ARDUINO_DEVICE_ID' is missing from the app.yaml but here we check the empty string.
78+
// A better aproach is to use golden files
79+
require.Equal(t, "", after.Descriptor.Bricks[0].Variables["ARDUINO_DEVICE_ID"])
7780
require.Equal(t, "a-secret-a", after.Descriptor.Bricks[0].Variables["ARDUINO_SECRET"])
7881
})
7982

0 commit comments

Comments
 (0)