Migrate more test examples to use pytest parametrization#1278
Conversation
conda-bot
added
the
cla-signed
| return Path("~").expanduser() / calculate_install_dir(input_dir / config_filename) | ||
| elif installer.suffix == ".msi": | ||
| return calculate_msi_install_path(input_dir / config_filename) | ||
| elif short: |
There was a problem hiding this comment.
One of the examples reached maximum path length with the new pytest parametrization. pytest automatically creates unique tmp directories for each test parameter run (like /tmp/pytest-of-<username>/pytest-<run_number>/<test_module_name>/<test_function_name><[parameter_id]>/), because of this, one of the examples reached the max path limit (it was very close to the limit already). Therefore this new arg short simply forces it into a shorter name.
| name: Signed_AzureSignTool | ||
| version: 1.0.0 | ||
| installer_type: [exe, msi] | ||
| installer_type: [exe, msi] # [win] |
There was a problem hiding this comment.
The entire installer should only run on Windows, so I don't think we need to add that selector.
There was a problem hiding this comment.
Originally I did not add the selector but I found a bug which resulted in me adding it. Without # [win] the regex matches [exe, msi] as a selector because the regex looks for any [...] at the end of a line. It tries to evaluate exe, msi as a Python expression, which fails because exe and msi aren't defined variables.
The # [win] selector is needed to prevent the YAML list value from being misinterpreted as a platform selector.
I am planning to fix it but I propose to do it in a separate PR to minimize the changes in this PR since it's not a new bug.
There was a problem hiding this comment.
Then why not format it as YAML?
installer_type:
- exe
- msi
There was a problem hiding this comment.
Good call, I was so locked in on the formats already in our code-base and forgot this was an option. See 3e648b4.
There was a problem hiding this comment.
Actually I think the selector is needed:
INTERNALERROR> SystemExit: Error: invalid installer type 'exe' for linux; allowed: sh
There was a problem hiding this comment.
This is happening during the unit tests. I think there is something going on during the import step when the tests are collected that the selector may be masking.
There was a problem hiding this comment.
I did a little more changes, with some minor refactoring to be able to filter out this scenario fc7df2e. The refactoring allows installer_types_for_example() to handle platform-incompatible examples by catching InvalidInstallerTypeError instead of SystemExit, so tests are properly skipped without needing platform specific selectors in the YAML file or manual skipif decorators.
3 participants
Description
This is a follow-up of #1266, since that PR added the foundation, this PR migrates about half of the tests to the new layout (to simplify review). I am planning to a third (last) PR with the remaining tests.
Checklist - did you ...
newsdirectory (using the template) for the next release's release notes?