Run examples using pytest + optimize CI setup
#675
Conversation
conda-bot
added
the
cla-signed
.github/workflows/main.yml
Outdated
| git diff --exit-code | ||
| package-name: ${{ github.event.repository.name }} | ||
| subdir: ${{ matrix.subdir }} | ||
| anaconda-org-channel: ctools |
There was a problem hiding this comment.
Should we change to conda-canary here? xref #612
| @@ -314,7 +314,7 @@ def main(): | |||
| logger.info("Got the following cli arguments: '%s'", args) | |||
|
|
|||
| if args.verbose or args.debug: | |||
| logger.setLevel(logging.DEBUG) | |||
| logging.getLogger("constructor").setLevel(logging.DEBUG) | |||
There was a problem hiding this comment.
This fixes a few logging issues (because the level was only set to constructor.main, other modules didn't have the right level set).
| @@ -20,6 +21,17 @@ | |||
| logger = logging.getLogger(__name__) | |||
|
|
|||
|
|
|||
| def calculate_install_dir(yaml_file, subdir=None): | |||
There was a problem hiding this comment.
Utility function needed in tests.
|
This PR is now ready for review. Hopefully it makes testing and debugging simpler / less surprising. Adding new examples is now a matter of adding a cc @conda/constructor - thanks (and apologies) in advance, this one is a bit large 😬 |
.github/workflows/main.yml
Outdated
| git diff --exit-code | ||
| package-name: ${{ github.event.repository.name }} | ||
| subdir: ${{ matrix.subdir }} | ||
| anaconda-org-channel: ctools |
| _run_installer(input_path, installer, install_dir, request=request) | ||
|
|
||
|
|
||
| def test_example_shortcuts(tmp_path, request): |
There was a problem hiding this comment.
For these I would have expected a parametrize over the _example_paths. Assuming that function returns filenames quickly it seems fine to refactor this way and would give more complete failure reporting for example if a change breaks multiple examples. It also would allow selecting tests by the example filename because that would show up in the resulting test name
There was a problem hiding this comment.
I thought about it but I would expect a future where different examples have different tests in pytest instead of those post_install tests we have right now (which are a bit hacky imo).
Some examples are not tested either, so we would have needed to explicitly write the example list (instead of the very attractive os.listdir), or implement the allow-list / deny-list we have in the current scripts/run_examples.py. I hope it's ok we leave it as it is now, even if a bit boilerplate-y 😬
I tried to use the example names in the function name precisely to allow easy selection but perhaps I missed some?
There was a problem hiding this comment.
I've been going back and forth on whether to change the boiler-plate code into paramatrized tests.
However, I think Jaime has a point that there is potential that the tests become more involved, especially if there are plans to replace the test_install.* scripts.
It's a good first step. However, I think my comment should be implemented before merging.
Description
Closes #641
Checklist - did you ...
newsdirectory (using the template) for the next release's release notes?