From ebca955cebd75e1e3506865da4af2de0c6ce1c60 Mon Sep 17 00:00:00 2001 From: Roberto Sora Date: Tue, 29 Oct 2019 15:08:06 +0100 Subject: [PATCH 1/6] add testcases to cover https://github.com/arduino/arduino-cli/issues/358 (Symbolic link as sketch directory) and https://github.com/arduino/arduino-cli/issues/424 (folder with .ino extension) --- arduino/builder/sketch_test.go | 40 +++++++++++++++++++ .../TestLoadSketchFolder.ino | 7 ++++ .../testdata/TestLoadSketchFolderSymlink | 1 + .../.#sketch.ino | 2 + .../TestLoadSketchFolderSymlink.ino | 7 ++++ .../TestLoadSketchFolderSymlinkSrc/doc.txt | 0 .../TestLoadSketchFolderSymlinkSrc/header.h | 1 + .../TestLoadSketchFolderSymlinkSrc/old.pde | 0 .../TestLoadSketchFolderSymlinkSrc/other.ino | 3 ++ .../TestLoadSketchFolderSymlinkSrc/s_file.S | 0 .../src/dont_load_me.ino | 2 + .../src/helper.h | 0 12 files changed, 63 insertions(+) create mode 100644 arduino/builder/testdata/TestLoadSketchFolderIno/TestLoadSketchFolderIno.ino/TestLoadSketchFolder.ino create mode 120000 arduino/builder/testdata/TestLoadSketchFolderSymlink create mode 100644 arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/.#sketch.ino create mode 100644 arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/TestLoadSketchFolderSymlink.ino create mode 100644 arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/doc.txt create mode 100644 arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/header.h create mode 100644 arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/old.pde create mode 100644 arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/other.ino create mode 100644 arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/s_file.S create mode 100644 arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/src/dont_load_me.ino create mode 100644 arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/src/helper.h diff --git a/arduino/builder/sketch_test.go b/arduino/builder/sketch_test.go index e6526f6fbc8..0058bdc6085 100644 --- a/arduino/builder/sketch_test.go +++ b/arduino/builder/sketch_test.go @@ -82,6 +82,46 @@ func TestLoadSketchFolder(t *testing.T) { require.Equal(t, "helper.h", filepath.Base(s.AdditionalFiles[2].Path)) } +func TestLoadSketchFolderSymlink(t *testing.T) { + // pass the path to the sketch folder + sketchPath := filepath.Join("testdata", t.Name()) + mainFilePath := filepath.Join(sketchPath, t.Name()+".ino") + s, err := builder.SketchLoad(sketchPath, "") + require.Nil(t, err) + require.NotNil(t, s) + require.Equal(t, mainFilePath, s.MainFile.Path) + require.Equal(t, sketchPath, s.LocationPath) + require.Len(t, s.OtherSketchFiles, 2) + require.Equal(t, "old.pde", filepath.Base(s.OtherSketchFiles[0].Path)) + require.Equal(t, "other.ino", filepath.Base(s.OtherSketchFiles[1].Path)) + require.Len(t, s.AdditionalFiles, 3) + require.Equal(t, "header.h", filepath.Base(s.AdditionalFiles[0].Path)) + require.Equal(t, "s_file.S", filepath.Base(s.AdditionalFiles[1].Path)) + require.Equal(t, "helper.h", filepath.Base(s.AdditionalFiles[2].Path)) + + // pass the path to the main file + sketchPath = mainFilePath + s, err = builder.SketchLoad(sketchPath, "") + require.Nil(t, err) + require.NotNil(t, s) + require.Equal(t, mainFilePath, s.MainFile.Path) + require.Len(t, s.OtherSketchFiles, 2) + require.Equal(t, "old.pde", filepath.Base(s.OtherSketchFiles[0].Path)) + require.Equal(t, "other.ino", filepath.Base(s.OtherSketchFiles[1].Path)) + require.Len(t, s.AdditionalFiles, 3) + require.Equal(t, "header.h", filepath.Base(s.AdditionalFiles[0].Path)) + require.Equal(t, "s_file.S", filepath.Base(s.AdditionalFiles[1].Path)) + require.Equal(t, "helper.h", filepath.Base(s.AdditionalFiles[2].Path)) +} + +func TestLoadSketchFolderIno(t *testing.T) { + // pass the path to the sketch folder + sketchPath := filepath.Join("testdata", t.Name()) + _, err := builder.SketchLoad(sketchPath, "") + require.Error(t, err) + require.Contains(t, err.Error(), "sketch must not be a directory") +} + func TestLoadSketchFolderWrongMain(t *testing.T) { sketchPath := filepath.Join("testdata", t.Name()) _, err := builder.SketchLoad(sketchPath, "") diff --git a/arduino/builder/testdata/TestLoadSketchFolderIno/TestLoadSketchFolderIno.ino/TestLoadSketchFolder.ino b/arduino/builder/testdata/TestLoadSketchFolderIno/TestLoadSketchFolderIno.ino/TestLoadSketchFolder.ino new file mode 100644 index 00000000000..0d5e0f5ceb9 --- /dev/null +++ b/arduino/builder/testdata/TestLoadSketchFolderIno/TestLoadSketchFolderIno.ino/TestLoadSketchFolder.ino @@ -0,0 +1,7 @@ +void setup() { + +} + +void loop() { + +} \ No newline at end of file diff --git a/arduino/builder/testdata/TestLoadSketchFolderSymlink b/arduino/builder/testdata/TestLoadSketchFolderSymlink new file mode 120000 index 00000000000..77ae3d2198d --- /dev/null +++ b/arduino/builder/testdata/TestLoadSketchFolderSymlink @@ -0,0 +1 @@ +TestLoadSketchFolderSymlinkSrc \ No newline at end of file diff --git a/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/.#sketch.ino b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/.#sketch.ino new file mode 100644 index 00000000000..71048175432 --- /dev/null +++ b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/.#sketch.ino @@ -0,0 +1,2 @@ +void setup() +void loop) } \ No newline at end of file diff --git a/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/TestLoadSketchFolderSymlink.ino b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/TestLoadSketchFolderSymlink.ino new file mode 100644 index 00000000000..0d5e0f5ceb9 --- /dev/null +++ b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/TestLoadSketchFolderSymlink.ino @@ -0,0 +1,7 @@ +void setup() { + +} + +void loop() { + +} \ No newline at end of file diff --git a/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/doc.txt b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/doc.txt new file mode 100644 index 00000000000..e69de29bb2d diff --git a/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/header.h b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/header.h new file mode 100644 index 00000000000..0e7d3b1a6a9 --- /dev/null +++ b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/header.h @@ -0,0 +1 @@ +#define FOO "BAR" \ No newline at end of file diff --git a/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/old.pde b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/old.pde new file mode 100644 index 00000000000..e69de29bb2d diff --git a/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/other.ino b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/other.ino new file mode 100644 index 00000000000..c426196c017 --- /dev/null +++ b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/other.ino @@ -0,0 +1,3 @@ +String hello() { + return "world"; +} \ No newline at end of file diff --git a/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/s_file.S b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/s_file.S new file mode 100644 index 00000000000..e69de29bb2d diff --git a/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/src/dont_load_me.ino b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/src/dont_load_me.ino new file mode 100644 index 00000000000..46b07018d09 --- /dev/null +++ b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/src/dont_load_me.ino @@ -0,0 +1,2 @@ +#include +#error "Whattya looking at?" diff --git a/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/src/helper.h b/arduino/builder/testdata/TestLoadSketchFolderSymlinkSrc/src/helper.h new file mode 100644 index 00000000000..e69de29bb2d From f1c28ff023c9daa24af413609accd832b01a3cba Mon Sep 17 00:00:00 2001 From: Roberto Sora Date: Tue, 29 Oct 2019 18:06:48 +0100 Subject: [PATCH 2/6] add test to check error throw on symlink loop detection --- test/test_compile.py | 54 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/test/test_compile.py b/test/test_compile.py index 08756867e10..95ef229b539 100644 --- a/test/test_compile.py +++ b/test/test_compile.py @@ -70,6 +70,60 @@ def test_compile_with_simple_sketch(run_command, data_dir): assert is_message_sequence_in_json_log_traces(expected_trace_sequence, json_log_lines) +def test_compile_with_sketch_with_symlink_selfloop(run_command, data_dir): + # Init the environment explicitly + result = run_command("core update-index") + assert result.ok + + # # Download latest AVR + result = run_command("core install arduino:avr") + assert result.ok + + sketch_name = "CompileIntegrationTestSymlinkSelfLoop" + sketch_path = os.path.join(data_dir, sketch_name) + fqbn = "arduino:avr:uno" + + # Create a test sketch + result = run_command("sketch new {}".format(sketch_path)) + assert result.ok + assert "Sketch created in: {}".format(sketch_path) in result.stdout + + # create a symlink that loops on himself + loop_file_path = os.path.join(sketch_path, "loop") + os.symlink(loop_file_path, loop_file_path) + + # Build sketch for arduino:avr:uno + result = run_command( + "compile -b {fqbn} {sketch_path}".format( + fqbn=fqbn, sketch_path=sketch_path)) + assert "Error during sketch processing: stat {loop_file_path}: too many levels of symbolic links".format( + loop_file_path=loop_file_path) in result.stderr + assert not result.ok + + sketch_name = "CompileIntegrationTestSymlinkDirLoop" + sketch_path = os.path.join(data_dir, sketch_name) + fqbn = "arduino:avr:uno" + + # Create a test sketch + result = run_command("sketch new {}".format(sketch_path)) + assert result.ok + assert "Sketch created in: {}".format(sketch_path) in result.stdout + + # create a symlink that loops on the upper level + loop_dir_path = os.path.join(sketch_path, "loop_dir") + os.mkdir(loop_dir_path) + loop_dir_symlink_path = os.path.join(loop_dir_path, "loop_dir_symlink") + os.symlink(loop_dir_path, loop_dir_symlink_path) + + # Build sketch for arduino:avr:uno + result = run_command( + "compile -b {fqbn} {sketch_path}".format( + fqbn=fqbn, sketch_path=sketch_path)) + assert "Filesystem bottom is too deep (directory recursion or filesystem really deep)".format( + loop_file_path=loop_file_path) in result.stderr + assert not result.ok + + @pytest.mark.skipif(running_on_ci(), reason="VMs have no serial ports") def test_compile_and_compile_combo(run_command, data_dir): # Init the environment explicitly From 7f66563caf6f49ccdd74debecaf8211ece2ab7c6 Mon Sep 17 00:00:00 2001 From: Roberto Sora Date: Wed, 30 Oct 2019 11:02:41 +0100 Subject: [PATCH 3/6] create symlink via test code to ensure cross platform creation --- arduino/builder/sketch_test.go | 14 ++++++++------ .../builder/testdata/TestLoadSketchFolderSymlink | 1 - 2 files changed, 8 insertions(+), 7 deletions(-) delete mode 120000 arduino/builder/testdata/TestLoadSketchFolderSymlink diff --git a/arduino/builder/sketch_test.go b/arduino/builder/sketch_test.go index 0058bdc6085..409e258d541 100644 --- a/arduino/builder/sketch_test.go +++ b/arduino/builder/sketch_test.go @@ -84,13 +84,15 @@ func TestLoadSketchFolder(t *testing.T) { func TestLoadSketchFolderSymlink(t *testing.T) { // pass the path to the sketch folder - sketchPath := filepath.Join("testdata", t.Name()) - mainFilePath := filepath.Join(sketchPath, t.Name()+".ino") - s, err := builder.SketchLoad(sketchPath, "") + symlinkSketchPath := filepath.Join("testdata", t.Name()) + srcSketchPath := t.Name() + "Src" + os.Symlink(srcSketchPath, symlinkSketchPath) + mainFilePath := filepath.Join(symlinkSketchPath, t.Name()+".ino") + s, err := builder.SketchLoad(symlinkSketchPath, "") require.Nil(t, err) require.NotNil(t, s) require.Equal(t, mainFilePath, s.MainFile.Path) - require.Equal(t, sketchPath, s.LocationPath) + require.Equal(t, symlinkSketchPath, s.LocationPath) require.Len(t, s.OtherSketchFiles, 2) require.Equal(t, "old.pde", filepath.Base(s.OtherSketchFiles[0].Path)) require.Equal(t, "other.ino", filepath.Base(s.OtherSketchFiles[1].Path)) @@ -100,8 +102,8 @@ func TestLoadSketchFolderSymlink(t *testing.T) { require.Equal(t, "helper.h", filepath.Base(s.AdditionalFiles[2].Path)) // pass the path to the main file - sketchPath = mainFilePath - s, err = builder.SketchLoad(sketchPath, "") + symlinkSketchPath = mainFilePath + s, err = builder.SketchLoad(symlinkSketchPath, "") require.Nil(t, err) require.NotNil(t, s) require.Equal(t, mainFilePath, s.MainFile.Path) diff --git a/arduino/builder/testdata/TestLoadSketchFolderSymlink b/arduino/builder/testdata/TestLoadSketchFolderSymlink deleted file mode 120000 index 77ae3d2198d..00000000000 --- a/arduino/builder/testdata/TestLoadSketchFolderSymlink +++ /dev/null @@ -1 +0,0 @@ -TestLoadSketchFolderSymlinkSrc \ No newline at end of file From 1ea0685c2600ab0d96ae09aaa6cfd2c493397ed7 Mon Sep 17 00:00:00 2001 From: Roberto Sora Date: Wed, 30 Oct 2019 11:26:02 +0100 Subject: [PATCH 4/6] fixed assert to make the check cross platform --- test/test_compile.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/test/test_compile.py b/test/test_compile.py index 95ef229b539..b2be5dfe054 100644 --- a/test/test_compile.py +++ b/test/test_compile.py @@ -96,7 +96,8 @@ def test_compile_with_sketch_with_symlink_selfloop(run_command, data_dir): result = run_command( "compile -b {fqbn} {sketch_path}".format( fqbn=fqbn, sketch_path=sketch_path)) - assert "Error during sketch processing: stat {loop_file_path}: too many levels of symbolic links".format( + assert "Error during sketch processing: stat {loop_file_path}: " \ + "".format( loop_file_path=loop_file_path) in result.stderr assert not result.ok @@ -119,8 +120,9 @@ def test_compile_with_sketch_with_symlink_selfloop(run_command, data_dir): result = run_command( "compile -b {fqbn} {sketch_path}".format( fqbn=fqbn, sketch_path=sketch_path)) - assert "Filesystem bottom is too deep (directory recursion or filesystem really deep)".format( - loop_file_path=loop_file_path) in result.stderr + # The assertion is a bit relaxed in this case because macOS behaves differently from win and linux: + # the cli does not follow recursively the symlink til breaking + assert "Error during sketch processing" in result.stderr assert not result.ok From 3d63c789422806edf5fad612e08b26c72b87d568 Mon Sep 17 00:00:00 2001 From: Roberto Sora Date: Wed, 30 Oct 2019 11:44:53 +0100 Subject: [PATCH 5/6] fixed all symlink check asserts to make the check cross platform --- test/test_compile.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/test_compile.py b/test/test_compile.py index b2be5dfe054..d3683fc5f79 100644 --- a/test/test_compile.py +++ b/test/test_compile.py @@ -96,7 +96,9 @@ def test_compile_with_sketch_with_symlink_selfloop(run_command, data_dir): result = run_command( "compile -b {fqbn} {sketch_path}".format( fqbn=fqbn, sketch_path=sketch_path)) - assert "Error during sketch processing: stat {loop_file_path}: " \ + # The assertion is a bit relaxed in this case because win behaves differently from macOs and linux + # returning a different error detailed message + assert "Error during sketch processing" \ "".format( loop_file_path=loop_file_path) in result.stderr assert not result.ok @@ -120,7 +122,7 @@ def test_compile_with_sketch_with_symlink_selfloop(run_command, data_dir): result = run_command( "compile -b {fqbn} {sketch_path}".format( fqbn=fqbn, sketch_path=sketch_path)) - # The assertion is a bit relaxed in this case because macOS behaves differently from win and linux: + # The assertion is a bit relaxed also in this case because macOS behaves differently from win and linux: # the cli does not follow recursively the symlink til breaking assert "Error during sketch processing" in result.stderr assert not result.ok From 3db1dc953b71f4415c4d3c5959c56254f0fdbf5d Mon Sep 17 00:00:00 2001 From: Roberto Sora Date: Wed, 30 Oct 2019 15:44:14 +0100 Subject: [PATCH 6/6] nitpicking --- test/test_compile.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/test/test_compile.py b/test/test_compile.py index d3683fc5f79..ad15f20f719 100644 --- a/test/test_compile.py +++ b/test/test_compile.py @@ -39,7 +39,7 @@ def test_compile_with_simple_sketch(run_command, data_dir): result = run_command("core update-index") assert result.ok - # # Download latest AVR + # Download latest AVR result = run_command("core install arduino:avr") assert result.ok @@ -75,7 +75,7 @@ def test_compile_with_sketch_with_symlink_selfloop(run_command, data_dir): result = run_command("core update-index") assert result.ok - # # Download latest AVR + # Download latest AVR result = run_command("core install arduino:avr") assert result.ok @@ -98,9 +98,7 @@ def test_compile_with_sketch_with_symlink_selfloop(run_command, data_dir): fqbn=fqbn, sketch_path=sketch_path)) # The assertion is a bit relaxed in this case because win behaves differently from macOs and linux # returning a different error detailed message - assert "Error during sketch processing" \ - "".format( - loop_file_path=loop_file_path) in result.stderr + assert "Error during sketch processing" in result.stderr assert not result.ok sketch_name = "CompileIntegrationTestSymlinkDirLoop"