From d56e8d495399ffcc3e872c9aca1e274d680c96ee Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 29 Sep 2020 12:13:44 +0200 Subject: [PATCH 1/2] Sligthly simplified upload test cases handling --- commands/upload/upload_test.go | 41 ++++++++++++++++------------------ 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/commands/upload/upload_test.go b/commands/upload/upload_test.go index 54f34155e7e..eff922fca03 100644 --- a/commands/upload/upload_test.go +++ b/commands/upload/upload_test.go @@ -55,7 +55,6 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) { fqbn *cores.FQBN resBuildPath string resSketchName string - hasError bool } blonk, err := sketches.NewSketchFromPath(paths.New("testdata/Blonk")) @@ -66,52 +65,50 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) { tests := []test{ // 00: error: no data passed in - {"", "", nil, nil, "", "", true}, + {"", "", nil, nil, "", ""}, // 01: use importFile to detect build.path and project_name - {"testdata/build_path_2/Blink.ino.hex", "", nil, nil, "testdata/build_path_2", "Blink.ino", false}, + {"testdata/build_path_2/Blink.ino.hex", "", nil, nil, "testdata/build_path_2", "Blink.ino"}, // 02: use importPath as build.path and project_name - {"", "testdata/build_path_2", nil, nil, "testdata/build_path_2", "Blink.ino", false}, + {"", "testdata/build_path_2", nil, nil, "testdata/build_path_2", "Blink.ino"}, // 03: error: used both importPath and importFile - {"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", nil, nil, "", "", true}, + {"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", nil, nil, "", ""}, // 04: error: only sketch without FQBN - {"", "", blonk, nil, "", "", true}, + {"", "", blonk, nil, "", ""}, // 05: use importFile to detect build.path and project_name, sketch is ignored. - {"testdata/build_path_2/Blink.ino.hex", "", blonk, nil, "testdata/build_path_2", "Blink.ino", false}, + {"testdata/build_path_2/Blink.ino.hex", "", blonk, nil, "testdata/build_path_2", "Blink.ino"}, // 06: use importPath as build.path and Blink as project name, ignore the sketch Blonk - {"", "testdata/build_path_2", blonk, nil, "testdata/build_path_2", "Blink.ino", false}, + {"", "testdata/build_path_2", blonk, nil, "testdata/build_path_2", "Blink.ino"}, // 07: error: used both importPath and importFile - {"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", blonk, nil, "", "", true}, + {"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", blonk, nil, "", ""}, // 08: error: no data passed in - {"", "", nil, fqbn, "", "", true}, + {"", "", nil, fqbn, "", ""}, // 09: use importFile to detect build.path and project_name, fqbn ignored - {"testdata/build_path_2/Blink.ino.hex", "", nil, fqbn, "testdata/build_path_2", "Blink.ino", false}, + {"testdata/build_path_2/Blink.ino.hex", "", nil, fqbn, "testdata/build_path_2", "Blink.ino"}, // 10: use importPath as build.path and project_name, fqbn ignored - {"", "testdata/build_path_2", nil, fqbn, "testdata/build_path_2", "Blink.ino", false}, + {"", "testdata/build_path_2", nil, fqbn, "testdata/build_path_2", "Blink.ino"}, // 11: error: used both importPath and importFile - {"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", nil, fqbn, "", "", true}, + {"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", nil, fqbn, "", ""}, // 12: use sketch to determine project name and sketch+fqbn to determine build path - {"", "", blonk, fqbn, "testdata/Blonk/build/arduino.samd.mkr1000", "Blonk.ino", false}, + {"", "", blonk, fqbn, "testdata/Blonk/build/arduino.samd.mkr1000", "Blonk.ino"}, // 13: use importFile to detect build.path and project_name, sketch+fqbn is ignored. - {"testdata/build_path_2/Blink.ino.hex", "", blonk, fqbn, "testdata/build_path_2", "Blink.ino", false}, + {"testdata/build_path_2/Blink.ino.hex", "", blonk, fqbn, "testdata/build_path_2", "Blink.ino"}, // 14: use importPath as build.path and Blink as project name, ignore the sketch Blonk, ignore fqbn - {"", "testdata/build_path_2", blonk, fqbn, "testdata/build_path_2", "Blink.ino", false}, + {"", "testdata/build_path_2", blonk, fqbn, "testdata/build_path_2", "Blink.ino"}, // 15: error: used both importPath and importFile - {"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", blonk, fqbn, "", "", true}, + {"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", blonk, fqbn, "", ""}, } for i, test := range tests { t.Run(fmt.Sprintf("SubTest%02d", i), func(t *testing.T) { buildPath, sketchName, err := determineBuildPathAndSketchName(test.importFile, test.importDir, test.sketch, test.fqbn) - if test.hasError { - require.Error(t, err) - } else { - require.NoError(t, err) - } if test.resBuildPath == "" { + require.Error(t, err) require.Nil(t, buildPath) } else { + require.NoError(t, err) resBuildPath := paths.New(test.resBuildPath) require.NoError(t, resBuildPath.ToAbs()) + require.NotNil(t, buildPath) require.NoError(t, buildPath.ToAbs()) require.Equal(t, resBuildPath.String(), buildPath.String()) } From 22fdd191047653788961ec1468701ea5d4eb729c Mon Sep 17 00:00:00 2001 From: Cristian Maglie Date: Tue, 29 Sep 2020 12:14:16 +0200 Subject: [PATCH 2/2] Make --input-dir handle gracefully some rare cases Using `--input-dir path/to/firmware` with a directory containing: path/to/firmware/firmware.ino.bin path/to/firmware/some_other_firmware_name.ino.bin should not fail but select `firmware.ino.bin` for upload because the containing folder has the same name. See https://github.com/arduino/arduino-cli/issues/765#issuecomment-699678646 --- .../upload/testdata/firmware/another_firmware.ino.bin | 0 commands/upload/testdata/firmware/firmware.ino.bin | 0 commands/upload/upload.go | 9 +++++++++ commands/upload/upload_test.go | 5 +++++ 4 files changed, 14 insertions(+) create mode 100644 commands/upload/testdata/firmware/another_firmware.ino.bin create mode 100644 commands/upload/testdata/firmware/firmware.ino.bin diff --git a/commands/upload/testdata/firmware/another_firmware.ino.bin b/commands/upload/testdata/firmware/another_firmware.ino.bin new file mode 100644 index 00000000000..e69de29bb2d diff --git a/commands/upload/testdata/firmware/firmware.ino.bin b/commands/upload/testdata/firmware/firmware.ino.bin new file mode 100644 index 00000000000..e69de29bb2d diff --git a/commands/upload/upload.go b/commands/upload/upload.go index a26bdb6ceff..396e784585c 100644 --- a/commands/upload/upload.go +++ b/commands/upload/upload.go @@ -438,6 +438,15 @@ func detectSketchNameFromBuildPath(buildPath *paths.Path) (string, error) { return "", err } + if absBuildPath, err := buildPath.Abs(); err == nil { + candidateName := absBuildPath.Base() + ".ino" + f := files.Clone() + f.FilterPrefix(candidateName + ".") + if f.Len() > 0 { + return candidateName, nil + } + } + candidateName := "" var candidateFile *paths.Path for _, file := range files { diff --git a/commands/upload/upload_test.go b/commands/upload/upload_test.go index eff922fca03..a8e0ad74dc2 100644 --- a/commands/upload/upload_test.go +++ b/commands/upload/upload_test.go @@ -97,6 +97,11 @@ func TestDetermineBuildPathAndSketchName(t *testing.T) { {"", "testdata/build_path_2", blonk, fqbn, "testdata/build_path_2", "Blink.ino"}, // 15: error: used both importPath and importFile {"testdata/build_path_2/Blink.ino.hex", "testdata/build_path_2", blonk, fqbn, "", ""}, + + // 16: importPath containing multiple firmwares, but one has the same name as the containing folder + {"", "testdata/firmware", nil, fqbn, "testdata/firmware", "firmware.ino"}, + // 17: importFile among multiple firmwares + {"testdata/firmware/another_firmware.ino.bin", "", nil, fqbn, "testdata/firmware", "another_firmware.ino"}, } for i, test := range tests { t.Run(fmt.Sprintf("SubTest%02d", i), func(t *testing.T) {