Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 16 commits into from
Sep 11, 2019
Merged

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

merged 16 commits into from
Sep 11, 2019

Conversation

rsora
Copy link
Contributor

@rsora rsora commented Sep 5, 2019 •
edited
Loading

This PR closes #59

This PR extends the following path logic:

  1. if a sketch name only is passed as parameter, assume that the sketch directory is the current directory
  2. If a relative path for the sketch is passed, join it with the current directory path
  3. is an absolute path for the sketch is passed, simply use it

to the sketch new command in addition to compile and upload commands

@rsora rsora marked this pull request as ready for review September 5, 2019 17:07
@rsora rsora added this to the 0.6.0 milestone Sep 6, 2019
masci
masci suggested changes Sep 9, 2019
View reviewed changes
test/test_compile.py Outdated
import pytest

from .common import running_on_ci


def test_sketch_new(run_command):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should probably create a new module test_sketch.py for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Module created. Additionally, I applyied a small modification to the run_command fixture in order to have a temp working dir as a sandbox for the sketch creation e2e tests

@rsora rsora force-pushed the rsora/path-sketch-new branch from a5a82c0 to b5e70a9 Compare September 9, 2019 10:37
@rsora rsora requested a review from masci September 10, 2019 12:10
masci
masci suggested changes Sep 10, 2019
View reviewed changes
test/test_new.py Outdated
@@ -0,0 +1,51 @@
# This file is part of arduino-cli.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we name this test_sketch.py for consistency?

masci
masci approved these changes Sep 10, 2019
View reviewed changes
Copy link
Contributor

@masci masci left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

Roberto Sora added 16 commits September 11, 2019 12:26
poc of required mod for behavior alignment
dbacd62
align current tests to new behaviour
5aae182
align sketch new behaviour in sketch path parameter mangling adding p…
142d440 
…roper testing
align sketch new behaviour in sketch path parameter mangling adding
a16bc66 
proper testing
add test for subpath
0343785
add fix test for subpath
c9f2cd9
add .ino suffix trim
24c337f
move sketch new test in separate test_new.py file
f4b227c
add working_dir fixture to sandboxing sketch creation tests
5a2eca1
add e2e test for .ino strip
f10ccc2
apply test_sketch.py to sketch test module name for consistency
5d2664b
update test lib invoke to 1.3.0 in requirements.txt
c8ecd7f
force pytest temp directory to be created in current dir
865b868
create local tmpdir only for windows-2019 VM
8fb0881
add conditional step to test pipeline to overcome win VM behaviour in…
e21f5f0 
… e2e pytests
skip failing sketch new e2e tests in GA Win VM until we find the issu…
0c75cca 
…e root cause
@rsora rsora force-pushed the rsora/path-sketch-new branch from b221df1 to 0c75cca Compare September 11, 2019 10:54
rsora
rsora commented Sep 11, 2019
View reviewed changes
test/test_sketch.py
from test.common import running_on_ci


@pytest.mark.skipif(running_on_ci() and platform.system() == "Windows",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The e2e testing in the Github CI windows VM is temporarly disabled for the sketch new feature.
We are experiencing an odd behavior for windows VM in the test pipeline that we are not able to reproduce on a physical windows machine. The issue seems to be related to the managing of pytest temporary directories.
The test will remain disabled until a root cause is found and the fix introduced as a separate PR.

@rsora rsora merged commit 032fa05 into master Sep 11, 2019
@rsora rsora deleted the rsora/path-sketch-new branch September 11, 2019 12:16
@masci masci mentioned this pull request Oct 1, 2019
Closed
@rsora rsora mentioned this pull request Oct 2, 2019
Merged
@masci masci mentioned this pull request Dec 4, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@masci masci masci approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.6.0
Development

Successfully merging this pull request may close these issues.

arduino-cli sketch new XXX command should create project at current folder, not ~/Arduino
2 participants
@rsora @masci