Fix path in error messages on sketch loading (arduino#1805)

cmaglie · web-flow · commit f0b218aa067c · 2022-07-18T12:09:47.000+02:00
* Fix path in error messages on sketch loading

* Added integration tests

* Updated unit-test

* Updated integration test

* Even better error messages
diff --git a/arduino/sketch/sketch.go b/arduino/sketch/sketch.go
@@ -64,7 +64,12 @@ func New(path *paths.Path) (*Sketch, error) {
 	}
 
 	path = path.Canonical()
-	if !path.IsDir() {
+	if exist, err := path.ExistCheck(); err != nil {
+		return nil, fmt.Errorf("%s: %s", tr("sketch path is not valid"), err)
+	} else if !exist {
+		return nil, fmt.Errorf("%s: %s", tr("no such file or directory"), path)
+	}
+	if _, validIno := globals.MainFileValidExtensions[path.Ext()]; validIno && !path.IsDir() {
 		path = path.Parent()
 	}
 
@@ -82,6 +87,9 @@ func New(path *paths.Path) (*Sketch, error) {
 			}
 		}
 	}
+	if mainFile == nil {
+		return nil, fmt.Errorf(tr("main file missing from sketch: %s", path.Join(path.Base()+globals.MainFileValidExtension)))
+	}
 
 	sketch := &Sketch{
 		Name:             path.Base(),
@@ -269,7 +277,6 @@ func (s *Sketch) checkSketchCasing() error {
 		return &InvalidSketchFolderNameError{
 			SketchFolder: s.FullPath,
 			SketchFile:   sketchFile,
-			Sketch:       s,
 		}
 	}
 
@@ -280,7 +287,6 @@ func (s *Sketch) checkSketchCasing() error {
 type InvalidSketchFolderNameError struct {
 	SketchFolder *paths.Path
 	SketchFile   *paths.Path
-	Sketch       *Sketch
 }
 
 func (e *InvalidSketchFolderNameError) Error() string {
diff --git a/arduino/sketch/sketch_test.go b/arduino/sketch/sketch_test.go
@@ -100,30 +100,52 @@ func TestNewSketchWrongMain(t *testing.T) {
 	require.Error(t, err)
 	sketchFolderPath, _ = sketchFolderPath.Abs()
 	expectedMainFile := sketchFolderPath.Join(sketchName)
-	expectedError := fmt.Sprintf("no valid sketch found in %s: missing %s", sketchFolderPath, expectedMainFile)
+	expectedError := fmt.Sprintf("main file missing from sketch: %s", expectedMainFile)
 	require.Contains(t, err.Error(), expectedError)
 
 	sketchFolderPath = paths.New("testdata", sketchName)
 	mainFilePath := sketchFolderPath.Join(fmt.Sprintf("%s.ino", sketchName))
 	sketch, err = New(mainFilePath)
 	require.Nil(t, sketch)
 	require.Error(t, err)
-	sketchFolderPath, _ = sketchFolderPath.Abs()
-	expectedError = fmt.Sprintf("no valid sketch found in %s: missing %s", sketchFolderPath, expectedMainFile)
+	expectedError = fmt.Sprintf("no such file or directory: %s", expectedMainFile)
 	require.Contains(t, err.Error(), expectedError)
 }
 
 func TestNewSketchCasingWrong(t *testing.T) {
-	sketchPath := paths.New("testdata", "SketchWithWrongMain")
-	sketch, err := New(sketchPath)
-	assert.Nil(t, sketch)
-	assert.Error(t, err)
-	assert.IsType(t, &InvalidSketchFolderNameError{}, err)
-	e := err.(*InvalidSketchFolderNameError)
-	assert.NotNil(t, e.Sketch)
-	sketchPath, _ = sketchPath.Abs()
-	expectedError := fmt.Sprintf("no valid sketch found in %s: missing %s", sketchPath.String(), sketchPath.Join(sketchPath.Base()+".ino"))
-	assert.EqualError(t, err, expectedError)
+	{
+		sketchPath := paths.New("testdata", "SketchWithWrongMain")
+		sketch, err := New(sketchPath)
+		assert.Nil(t, sketch)
+		assert.Error(t, err)
+		_, ok := err.(*InvalidSketchFolderNameError)
+		assert.False(t, ok)
+		sketchPath, _ = sketchPath.Abs()
+		expectedError := fmt.Sprintf("main file missing from sketch: %s", sketchPath.Join(sketchPath.Base()+".ino"))
+		assert.EqualError(t, err, expectedError)
+	}
+	{
+		sketchPath := paths.New("testdata", "SketchWithWrongMain", "main.ino")
+		sketch, err := New(sketchPath)
+		assert.Nil(t, sketch)
+		assert.Error(t, err)
+		_, ok := err.(*InvalidSketchFolderNameError)
+		assert.False(t, ok)
+		sketchPath, _ = sketchPath.Parent().Abs()
+		expectedError := fmt.Sprintf("main file missing from sketch: %s", sketchPath.Join(sketchPath.Base()+".ino"))
+		assert.EqualError(t, err, expectedError)
+	}
+	{
+		sketchPath := paths.New("testdata", "non-existent")
+		sketch, skerr := New(sketchPath)
+		require.Nil(t, sketch)
+		require.Error(t, skerr)
+		_, ok := skerr.(*InvalidSketchFolderNameError)
+		assert.False(t, ok)
+		sketchPath, _ = sketchPath.Abs()
+		expectedError := fmt.Sprintf("no such file or directory: %s", sketchPath)
+		require.EqualError(t, skerr, expectedError)
+	}
 }
 
 func TestNewSketchCasingCorrect(t *testing.T) {
diff --git a/legacy/builder/container_setup.go b/legacy/builder/container_setup.go
@@ -16,8 +16,6 @@
 package builder
 
 import (
-	"fmt"
-
 	sk "github.com/arduino/arduino-cli/arduino/sketch"
 	"github.com/arduino/arduino-cli/legacy/builder/types"
 	"github.com/pkg/errors"
@@ -61,17 +59,9 @@ func (s *ContainerSetupHardwareToolsLibsSketchAndProps) Run(ctx *types.Context)
 
 		// load sketch
 		sketch, err := sk.New(sketchLocation)
-		if e, ok := err.(*sk.InvalidSketchFolderNameError); ctx.IgnoreSketchFolderNameErrors && ok {
-			// ignore error
-			// This is only done by the arduino-builder since the Arduino Java IDE
-			// supports sketches with invalid names
-			sketch = e.Sketch
-		} else if err != nil {
+		if err != nil {
 			return errors.WithStack(err)
 		}
-		if sketch.MainFile == nil {
-			return fmt.Errorf(tr("main file missing from sketch"))
-		}
 		sketch.BuildPath = ctx.BuildPath
 		ctx.SketchLocation = sketch.MainFile
 		ctx.Sketch = sketch
diff --git a/test/test_compile_part_1.py b/test/test_compile_part_1.py
@@ -37,6 +37,42 @@ def test_compile_without_fqbn(run_command):
     assert result.failed
 
 
+def test_compile_error_message(run_command, working_dir):
+    # Init the environment explicitly
+    run_command(["core", "update-index"])
+
+    # Download latest AVR
+    run_command(["core", "install", "arduino:avr"])
+
+    # Run a batch of bogus compile in a temp dir to check the error messages
+    with tempfile.TemporaryDirectory() as tmp_dir:
+        tmp = Path(tmp_dir)
+        abcdef = tmp / "ABCDEF"
+        res = run_command(["compile", "-b", "arduino:avr:uno", abcdef])
+        assert res.failed
+        assert "no such file or directory:" in res.stderr
+        res = run_command(["compile", "-b", "arduino:avr:uno", abcdef / "ABCDEF.ino"])
+        assert res.failed
+        assert "no such file or directory:" in res.stderr
+        res = run_command(["compile", "-b", "arduino:avr:uno", abcdef / "QWERTY"])
+        assert res.failed
+        assert "no such file or directory:" in res.stderr
+
+        abcdef.mkdir()
+        res = run_command(["compile", "-b", "arduino:avr:uno", abcdef])
+        assert res.failed
+        assert "main file missing from sketch:" in res.stderr
+        res = run_command(["compile", "-b", "arduino:avr:uno", abcdef / "ABCDEF.ino"])
+        assert res.failed
+        assert "no such file or directory:" in res.stderr
+
+        qwerty_ino = abcdef / "QWERTY.ino"
+        qwerty_ino.touch()
+        res = run_command(["compile", "-b", "arduino:avr:uno", qwerty_ino])
+        assert res.failed
+        assert "main file missing from sketch:" in res.stderr
+
+
 def test_compile_with_simple_sketch(run_command, data_dir, working_dir):
     # Init the environment explicitly
     run_command(["core", "update-index"])
diff --git a/test/test_compile_part_3.py b/test/test_compile_part_3.py
@@ -124,15 +124,15 @@ def test_compile_sketch_case_mismatch_fails(run_command, data_dir):
     # * Compiling with sketch path
     res = run_command(["compile", "--clean", "-b", fqbn, sketch_path])
     assert res.failed
-    assert "Error opening sketch: no valid sketch found" in res.stderr
+    assert "Error opening sketch:" in res.stderr
     # * Compiling with sketch main file
     res = run_command(["compile", "--clean", "-b", fqbn, sketch_main_file])
     assert res.failed
-    assert "Error opening sketch: no valid sketch found" in res.stderr
+    assert "Error opening sketch:" in res.stderr
     # * Compiling in sketch path
     res = run_command(["compile", "--clean", "-b", fqbn], custom_working_dir=sketch_path)
     assert res.failed
-    assert "Error opening sketch: no valid sketch found" in res.stderr
+    assert "Error opening sketch:" in res.stderr
 
 
 def test_compile_with_only_compilation_database_flag(run_command, data_dir):
diff --git a/test/test_sketch.py b/test/test_sketch.py
@@ -896,4 +896,4 @@ def test_sketch_archive_case_mismatch_fails(run_command, data_dir):
 
     res = run_command(["sketch", "archive", sketch_path])
     assert res.failed
-    assert "Error archiving: Can't open sketch: no valid sketch found" in res.stderr
+    assert "Error archiving: Can't open sketch:" in res.stderr
diff --git a/test/test_upload.py b/test/test_upload.py
@@ -364,7 +364,7 @@ def test_compile_and_upload_combo_sketch_with_mismatched_casing(run_command, dat
         # Try to compile
         res = run_command(["compile", "--clean", "-b", board.fqbn, "-u", "-p", board.address, sketch_path])
         assert res.failed
-        assert "Error opening sketch: no valid sketch found in" in res.stderr
+        assert "Error opening sketch:" in res.stderr
 
 
 def test_upload_sketch_with_mismatched_casing(run_command, data_dir, detected_boards, wait_for_board):
@@ -387,7 +387,7 @@ def test_upload_sketch_with_mismatched_casing(run_command, data_dir, detected_bo
         # searching for binaries since the sketch is not valid
         res = run_command(["upload", "-b", board.fqbn, "-p", board.address, sketch_path])
         assert res.failed
-        assert "Error during Upload: no valid sketch found in" in res.stderr
+        assert "Error during Upload:" in res.stderr
 
 
 def test_upload_to_port_with_board_autodetect(run_command, data_dir, detected_boards):

Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,12 @@ func New(path paths.Path) (Sketch, error) {`
`64`	`64`	`}`
`65`	`65`
`66`	`66`	`path = path.Canonical()`
`67`		`- if !path.IsDir() {`
	`67`	`+ if exist, err := path.ExistCheck(); err != nil {`
	`68`	`+ return nil, fmt.Errorf("%s: %s", tr("sketch path is not valid"), err)`
	`69`	`+ } else if !exist {`
	`70`	`+ return nil, fmt.Errorf("%s: %s", tr("no such file or directory"), path)`
	`71`	`+ }`
	`72`	`+ if _, validIno := globals.MainFileValidExtensions[path.Ext()]; validIno && !path.IsDir() {`
`68`	`73`	`path = path.Parent()`
`69`	`74`	`}`
`70`	`75`
`@@ -82,6 +87,9 @@ func New(path paths.Path) (Sketch, error) {`
`82`	`87`	`}`
`83`	`88`	`}`
`84`	`89`	`}`
	`90`	`+ if mainFile == nil {`
	`91`	`+ return nil, fmt.Errorf(tr("main file missing from sketch: %s", path.Join(path.Base()+globals.MainFileValidExtension)))`
	`92`	`+ }`
`85`	`93`
`86`	`94`	`sketch := &Sketch{`
`87`	`95`	`Name: path.Base(),`
`@@ -269,7 +277,6 @@ func (s *Sketch) checkSketchCasing() error {`
`269`	`277`	`return &InvalidSketchFolderNameError{`
`270`	`278`	`SketchFolder: s.FullPath,`
`271`	`279`	`SketchFile: sketchFile,`
`272`		`- Sketch: s,`
`273`	`280`	`}`
`274`	`281`	`}`
`275`	`282`
`@@ -280,7 +287,6 @@ func (s *Sketch) checkSketchCasing() error {`
`280`	`287`	`type InvalidSketchFolderNameError struct {`
`281`	`288`	`SketchFolder *paths.Path`
`282`	`289`	`SketchFile *paths.Path`
`283`		`- Sketch *Sketch`
`284`	`290`	`}`
`285`	`291`
`286`	`292`	`func (e *InvalidSketchFolderNameError) Error() string {`