-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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.
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
Refactor test subsets in CI workflows #4788
Conversation
b1e4b12
to
c3d6046
Compare
Compared to the previous regex-based approach, this can distinguish by OS and floatX setting, allowing for more informative outputs.
0910372
to
879adf4
Compare
bb4f676
to
ad188fe
Compare
Note: I can't reproduce the latest failures of the CI job by running this tests case individually. Yeah, I can reproduce the error locally only by running the entire testfile, but not when running the tests individually. |
But don't run tests_distributions.py on Windows because it runs into aesara-devs/aesara#485
@@ -36,6 +36,7 @@ repos: | |||
- repo: local | |||
hooks: | |||
- id: check-no-tests-are-ignored | |||
additional_dependencies: [pandas,pyyaml] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no objection here, but pandas is a pretty heavy dependency for a pre-commit hook. I'll have a look at this later, perhaps it can be done without it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It certainly can be done with a bunch of dictionaries, but then the summarization doesn't work with one-liners and we'd have to do our own table printing. Not sure if that's worth the effort?
Thanks @michaelosthege! |
Initially I wanted to re-use test subsets with anchors (#4517), but GitHub actions don't support YAML anchors :(
The Windows workflow was merged into the
pytest.yml
and thepytest
job renamed toubuntu
.Previously the Windows jobs ran with
bash.exe
as the shell, but this caused lots of problems related to maximum path lenghts. One option might have been (I didn't try) to change git options, but I decided to switch the shell to the standard Windowscmd
. To deal with multilinerun
options I changed|
to>-
and&&
. The environment variable is now resolved by%TEST_SUBSET%
instead of$TEST_SUBSET
.The
scipts\check_all_tests_are_covered.py
(which currently just asserts that tests don't run multiple times, but only warns about tests that are ignored) was refactored to analyze the workflows based on their YAML job matrix.This way it now prints a table of how often the tests are run with different OSes and floatX settings: