Skip to content

Commit 757ebee

Browse files
committed
Use consistent approach to processing user-provided paths
Paths from user input are converted to absolute paths as early as possible, then those absolute paths are used from there on. If possible, the path is converted to a user-friendly path relative to the root of the repository (GITHUB_WORKSPACE) before output to the log or sketches report. This is now more important due to the addition of support for ~, which requires processing through the absolute_path() function to work.
1 parent 887acd6 commit 757ebee

File tree

2 files changed

+37
-40
lines changed

2 files changed

+37
-40
lines changed

compilesketches/compilesketches.py

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,8 @@ def __init__(self, cli_version, fqbn_arg, platforms, libraries, sketch_paths, ve
111111

112112
# Save the space-separated list of paths as a Python list
113113
sketch_paths = parse_list_input(sketch_paths)
114-
sketch_paths = [pathlib.Path(sketch_path) for sketch_path in sketch_paths]
115-
self.sketch_paths = sketch_paths
114+
absolute_sketch_paths = [absolute_path(path=sketch_path) for sketch_path in sketch_paths]
115+
self.sketch_paths = absolute_sketch_paths
116116

117117
self.verbose = parse_boolean_input(boolean_input=verbose)
118118

@@ -383,10 +383,10 @@ def install_platforms_from_path(self, platform_list):
383383
"""
384384
for platform in platform_list:
385385
source_path = absolute_path(platform[self.dependency_source_path_key])
386-
self.verbose_print("Installing platform from path:", platform[self.dependency_source_path_key])
386+
self.verbose_print("Installing platform from path:", path_relative_to_workspace(path=source_path))
387387

388388
if not source_path.exists():
389-
print("::error::Platform source path:", platform[self.dependency_source_path_key], "doesn't exist")
389+
print("::error::Platform source path:", path_relative_to_workspace(path=source_path), "doesn't exist")
390390
sys.exit(1)
391391

392392
platform_installation_path = self.get_platform_installation_path(platform=platform)
@@ -583,7 +583,7 @@ def install_libraries(self):
583583

584584
# The original behavior of the action was to assume the root of the repo is a library to be installed, so
585585
# that behavior is retained when using the old input syntax
586-
library_list.path = [{self.dependency_source_path_key: pathlib.Path(os.environ["GITHUB_WORKSPACE"])}]
586+
library_list.path = [{self.dependency_source_path_key: os.environ["GITHUB_WORKSPACE"]}]
587587

588588
if len(library_list.manager) > 0:
589589
self.install_libraries_from_library_manager(library_list=library_list.manager)
@@ -700,36 +700,34 @@ def find_sketches(self):
700700
sketch_list = []
701701
self.verbose_print("Finding sketches under paths:", list_to_string(self.sketch_paths))
702702
for sketch_path in self.sketch_paths:
703-
# The absolute path is used in the code, the sketch path provided by the user is used in the log output
704-
absolute_sketch_path = absolute_path(sketch_path)
705703
sketch_path_sketch_list = []
706-
if not absolute_sketch_path.exists():
707-
print("::error::Sketch path:", sketch_path, "doesn't exist")
704+
if not sketch_path.exists():
705+
print("::error::Sketch path:", path_relative_to_workspace(path=sketch_path), "doesn't exist")
708706
sys.exit(1)
709707

710708
# Check if the specified path is a sketch (as opposed to containing sketches in subfolders)
711-
if absolute_sketch_path.is_file():
712-
if path_is_sketch(path=absolute_sketch_path):
709+
if sketch_path.is_file():
710+
if path_is_sketch(path=sketch_path):
713711
# The path directly to a sketch file was provided, so don't search recursively
714-
sketch_list.append(absolute_sketch_path.parent)
712+
sketch_list.append(sketch_path.parent)
715713
continue
716714
else:
717-
print("::error::Sketch path:", sketch_path, "is not a sketch")
715+
print("::error::Sketch path:", path_relative_to_workspace(path=sketch_path), "is not a sketch")
718716
sys.exit(1)
719717
else:
720718
# Path is a directory
721-
if path_is_sketch(path=absolute_sketch_path):
719+
if path_is_sketch(path=sketch_path):
722720
# Add sketch to list, but also search the path recursively for more sketches
723-
sketch_path_sketch_list.append(absolute_sketch_path)
721+
sketch_path_sketch_list.append(sketch_path)
724722

725723
# Search the sketch path recursively for sketches
726-
for sketch in sorted(absolute_sketch_path.rglob("*")):
724+
for sketch in sorted(sketch_path.rglob("*")):
727725
if sketch.is_dir() and path_is_sketch(path=sketch):
728726
sketch_path_sketch_list.append(sketch)
729727

730728
if len(sketch_path_sketch_list) == 0:
731729
# If a path provided via the sketch-paths input doesn't contain sketches, that indicates user error
732-
print("::error::No sketches were found in", sketch_path)
730+
print("::error::No sketches were found in", path_relative_to_workspace(path=sketch_path))
733731
sys.exit(1)
734732

735733
sketch_list.extend(sketch_path_sketch_list)
@@ -751,12 +749,12 @@ def compile_sketch(self, sketch_path):
751749
command=compilation_command, enable_output=self.RunCommandOutput.NONE, exit_on_failure=False)
752750
# Group compilation output to make the log easy to read
753751
# https://github.com/actions/toolkit/blob/master/docs/commands.md#group-and-ungroup-log-lines
754-
print("::group::Compiling sketch:", path_relative_to_workspace(sketch_path))
752+
print("::group::Compiling sketch:", path_relative_to_workspace(path=sketch_path))
755753
print(compilation_data.stdout)
756754
print("::endgroup::")
757755

758756
class CompilationResult:
759-
sketch = path_relative_to_workspace(sketch_path)
757+
sketch = sketch_path
760758
success = compilation_data.returncode == 0
761759
output = compilation_data.stdout
762760

@@ -795,7 +793,7 @@ def get_sketch_report(self, compilation_result):
795793

796794
# Compile the sketch again
797795
print("Compiling previous version of sketch to determine memory usage change")
798-
previous_compilation_result = self.compile_sketch(sketch_path=absolute_path(path=compilation_result.sketch))
796+
previous_compilation_result = self.compile_sketch(sketch_path=compilation_result.sketch)
799797

800798
# git checkout the PR's head ref to return the repository to its previous state
801799
repository.git.checkout(original_git_ref)
@@ -824,7 +822,7 @@ def get_sketch_report_from_output(self, compilation_result):
824822
ram_regex = r"Global variables use [0-9]+ bytes .*of dynamic memory"
825823

826824
# Set default report values
827-
sketch_report = {self.report_sketch_key: str(compilation_result.sketch),
825+
sketch_report = {self.report_sketch_key: str(path_relative_to_workspace(path=compilation_result.sketch)),
828826
self.report_compilation_success_key: compilation_result.success,
829827
self.report_flash_key: self.not_applicable_indicator,
830828
self.report_ram_key: self.not_applicable_indicator}

compilesketches/tests/test_compilesketches.py

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -153,8 +153,8 @@ def test_compilesketches():
153153
platforms = unittest.mock.sentinel.platforms
154154
libraries = unittest.mock.sentinel.libraries
155155
sketch_paths = "examples/FooSketchPath examples/BarSketchPath"
156-
expected_sketch_paths_list = [pathlib.PurePath("examples/FooSketchPath"),
157-
pathlib.PurePath("examples/BarSketchPath")]
156+
expected_sketch_paths_list = [compilesketches.absolute_path(path="examples/FooSketchPath"),
157+
compilesketches.absolute_path(path="examples/BarSketchPath")]
158158
verbose = "false"
159159
github_token = "fooGitHubToken"
160160
report_sketch = unittest.mock.sentinel.report_sketch
@@ -771,19 +771,19 @@ def __init__(self):
771771
"libraries, expected_manager, expected_path, expected_repository, expected_download",
772772
[("",
773773
[],
774-
[{compilesketches.CompileSketches.dependency_source_path_key: pathlib.PurePath(os.environ["GITHUB_WORKSPACE"])}],
774+
[{compilesketches.CompileSketches.dependency_source_path_key: os.environ["GITHUB_WORKSPACE"]}],
775775
[],
776776
[]),
777777
("foo bar",
778778
[{compilesketches.CompileSketches.dependency_name_key: "foo"},
779779
{compilesketches.CompileSketches.dependency_name_key: "bar"}],
780-
[{compilesketches.CompileSketches.dependency_source_path_key: pathlib.PurePath(os.environ["GITHUB_WORKSPACE"])}],
780+
[{compilesketches.CompileSketches.dependency_source_path_key: os.environ["GITHUB_WORKSPACE"]}],
781781
[],
782782
[]),
783783
("\"foo\" \"bar\"",
784784
[{compilesketches.CompileSketches.dependency_name_key: "foo"},
785785
{compilesketches.CompileSketches.dependency_name_key: "bar"}],
786-
[{compilesketches.CompileSketches.dependency_source_path_key: pathlib.PurePath(os.environ["GITHUB_WORKSPACE"])}],
786+
[{compilesketches.CompileSketches.dependency_source_path_key: os.environ["GITHUB_WORKSPACE"]}],
787787
[],
788788
[]),
789789
("-",
@@ -925,8 +925,7 @@ def test_install_libraries_from_path(capsys, monkeypatch, mocker, path_exists, l
925925
symlink_to_calls.append(
926926
unittest.mock.call(symlink_source_path,
927927
target=compilesketches.absolute_path(
928-
library[compilesketches.CompileSketches.dependency_source_path_key]
929-
),
928+
library[compilesketches.CompileSketches.dependency_source_path_key]),
930929
target_is_directory=True))
931930

932931
# noinspection PyUnresolvedReferences
@@ -1011,8 +1010,11 @@ def test_find_sketches(capsys):
10111010
)
10121011
with pytest.raises(expected_exception=SystemExit, match="1"):
10131012
compile_sketches.find_sketches()
1014-
assert capsys.readouterr().out.strip() == ("::error::Sketch path: " + str(pathlib.PurePath(nonexistent_sketch_path))
1015-
+ " doesn't exist")
1013+
assert capsys.readouterr().out.strip() == (
1014+
"::error::Sketch path: "
1015+
+ str(compilesketches.path_relative_to_workspace(path=nonexistent_sketch_path))
1016+
+ " doesn't exist"
1017+
)
10161018

10171019
# Test sketch path is a sketch file
10181020
compile_sketches = get_compilesketches_object(
@@ -1080,7 +1082,7 @@ def test_path_is_sketch():
10801082
(0, True)])
10811083
def test_compile_sketch(capsys, mocker, returncode, expected_success):
10821084
stdout = unittest.mock.sentinel.stdout
1083-
relative_sketch_path = pathlib.PurePath("FooSketch", "FooSketch.ino")
1085+
sketch_path = pathlib.Path("FooSketch", "FooSketch.ino").resolve()
10841086

10851087
# Stub
10861088
class CompilationData:
@@ -1095,19 +1097,19 @@ class CompilationData:
10951097
return_value=CompilationData())
10961098

10971099
compilation_result = compile_sketches.compile_sketch(
1098-
sketch_path=pathlib.PurePath(os.environ["GITHUB_WORKSPACE"]).joinpath(relative_sketch_path)
1100+
sketch_path=sketch_path
10991101
)
11001102

11011103
expected_stdout = (
1102-
"::group::Compiling sketch: " + str(relative_sketch_path) + "\n"
1104+
"::group::Compiling sketch: " + str(compilesketches.path_relative_to_workspace(path=sketch_path)) + "\n"
11031105
+ str(stdout) + "\n"
11041106
+ "::endgroup::"
11051107
)
11061108
if not expected_success:
11071109
expected_stdout += "\n::error::Compilation failed"
11081110
assert capsys.readouterr().out.strip() == expected_stdout
11091111

1110-
assert compilation_result.sketch == relative_sketch_path
1112+
assert compilation_result.sketch == sketch_path
11111113
assert compilation_result.success == expected_success
11121114
assert compilation_result.output == stdout
11131115

@@ -1124,7 +1126,6 @@ def __init__(self, sketch_input):
11241126

11251127
compilation_result = CompilationResult(sketch_input=sketch)
11261128

1127-
sketch_absolute_path = unittest.mock.sentinel.sketch_absolute_path
11281129
previous_compilation_result = unittest.mock.sentinel.previous_compilation_result
11291130
sketch_size = unittest.mock.sentinel.sketch_size
11301131

@@ -1148,7 +1149,6 @@ def checkout(self):
11481149
return_value=do_size_deltas_report)
11491150
mocker.patch("git.Repo", autospec=True, return_value=Repo())
11501151
mocker.patch("compilesketches.CompileSketches.checkout_pull_request_base_ref", autospec=True)
1151-
mocker.patch("compilesketches.absolute_path", autospec=True, return_value=sketch_absolute_path)
11521152
mocker.patch("compilesketches.CompileSketches.compile_sketch", autospec=True,
11531153
return_value=previous_compilation_result)
11541154
mocker.patch.object(Repo, "checkout")
@@ -1162,8 +1162,7 @@ def checkout(self):
11621162
if do_size_deltas_report:
11631163
git.Repo.assert_called_once_with(path=os.environ["GITHUB_WORKSPACE"])
11641164
compile_sketches.checkout_pull_request_base_ref.assert_called_once()
1165-
compilesketches.absolute_path.assert_called_once_with(path=compilation_result.sketch)
1166-
compile_sketches.compile_sketch.assert_called_once_with(compile_sketches, sketch_path=sketch_absolute_path)
1165+
compile_sketches.compile_sketch.assert_called_once_with(compile_sketches, sketch_path=compilation_result.sketch)
11671166
Repo.checkout.assert_called_once_with(original_git_ref)
11681167
get_sketch_size_from_output_calls.append(
11691168
unittest.mock.call(compile_sketches, compilation_result=previous_compilation_result))
@@ -1269,11 +1268,11 @@ def checkout(self):
12691268
get_compilesketches_object().not_applicable_indicator)]
12701269
)
12711270
def test_get_sketch_report_from_output(compilation_success, compilation_output, flash, ram):
1272-
sketch_path = unittest.mock.sentinel.sketch
1271+
sketch_path = pathlib.PurePath("foo/bar")
12731272
compilation_output = compilation_output.format(flash=str(flash), ram=str(ram))
12741273
compile_sketches = get_compilesketches_object()
12751274
compilation_result = type("CompilationResult", (),
1276-
{compile_sketches.report_sketch_key: sketch_path,
1275+
{"sketch": sketch_path,
12771276
"success": compilation_success,
12781277
"output": compilation_output})
12791278

0 commit comments

Comments
 (0)