fix(testing): don't change the output artifact location #758
Conversation
When running the `test` command, create the output artifact in the standard output location instead of moving it to the spread subdirectory. Fixes canonical#752 Signed-off-by: Claudio Matsuoka <[email protected]>
| # Output into the spread directory. | ||
| parsed_args.output = pathlib.Path.cwd() / "spread" | ||
| parsed_args.output.mkdir(exist_ok=True) | ||
| parsed_args.fetch_service_policy = None |
There was a problem hiding this comment.
what about this fetch service policy change?
There was a problem hiding this comment.
@lengau Why was this change needed when changing the output directory?
There was a problem hiding this comment.
The policy change was unrelated - we were just setting it so when the pack command checks for it, it would still be set.
| @@ -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" | |||
There was a problem hiding this comment.
This doesn't really address the fact that the testing command doesn't support the output parameter right?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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/ ?
There was a problem hiding this comment.
--output isn't in the argument parser for the test command so that will give a usage error.
When running the
testcommand, create the output artifact in thestandard output location instead of moving it to the spread
subdirectory.
Fixes #752
make lint && make test?docs/reference/changelog.rst)?