Skip to content

fix(testing): don't change the output artifact location #758

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix(testing): don't change the output artifact location #758

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions craft_application/commands/lifecycle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,10 +482,6 @@ def _run(

if parsed_args.test_path:
testing_service.validate_tests(parsed_args.test_path)
# Output into the spread directory.
parsed_args.output = pathlib.Path.cwd() / "spread"
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't really address the fact that the testing command doesn't support the output parameter right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will need further discussion. We need to test the artifact generated by pack, and this is not possible if output points to somewhere outside the project tree.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't mean outputs outside of the project tree, because we can't pack those anyway. I mean outputs that are subdirectories of the project dir. For example, you removed this hardcoded setting but what if the user does snapcraft test --output=spread/ ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

--output isn't in the argument parser for the test command so that will give a usage error.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah alright then!

parsed_args.output.mkdir(exist_ok=True)
parsed_args.fetch_service_policy = None
Copy link
Contributor

Choose a reason for hiding this comment

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

what about this fetch service policy change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lengau Why was this change needed when changing the output directory?

Copy link
Collaborator

Choose a reason for hiding this comment

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

The policy change was unrelated - we were just setting it so when the pack command checks for it, it would still be set.


# Don't enter a shell during the packing step, but save those values
# for the testing service.
Expand Down
Loading