-
Notifications
You must be signed in to change notification settings - Fork 45
Skip extension tests if the respective extension is not found or is passed via --disable-extensions
#29
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
Conversation
a17ad61
to
a0b5f3a
Compare
Is it possible to by default test an extension if it appears to be installed (if |
a0b5f3a
to
d7e0ddc
Compare
--extensions ...
option--disable-extensions
Sure thing. So now this PR skips extension tests if they're not found, otherwise testing them by default. If the user wants to purposely skip extension tests, they can use something like |
57525e5
to
44e6d59
Compare
For future reference—the current NumPy workflow conflicts with a |
44e6d59
to
54f3a35
Compare
I forgot to undraft this last week heh. Happy for a review. |
Looks like this has conflicts now. |
b6b5f84
to
544d4fc
Compare
@asmeurer fixed, fortunately just a rebase did the trick. |
The GitHub actions workflow should be appending to conftest (it uses If you have any suggestions for a better way to do it, let me know. I didn't get any good answers at https://stackoverflow.com/questions/68764967/xfail-pytest-tests-from-the-command-line. Maybe we should open a feature request for pytest to make something like this easier. Ideally you could just do |
Let's document this in the README. |
I keep getting errors like this
The problem, which always takes me some time to figure out, is that I accidentally did |
Just pushed a change to master that should make the tests pass here once you merge it. |
544d4fc
to
0f628db
Compare
Now the user can purposely test an extension via I see the issue with workflows is tricky. I tried a few things, but I think I like this the most: have a |
0f628db
to
d5ba0c6
Compare
You might as well also update the XFAILs. Some ones need to be added (I'm not sure why floor_divide and to_device were not already listed), and some can be removed now that numpy/numpy#19937 is merged. |
b78734a
to
b50e45b
Compare
Updated the README appropriately. Also updated the xfails—the NumPy workflow still fails, but that should be fixed in #31. |
Should be skipped by new `xp_extension()` mark
And merges both `conftest.py` files together
b50e45b
to
6e452fe
Compare
Should resolve #25. My current idea is to add a pytest options
--extensions
, where the user can specify any extensions they want to test alongside the normal tests (i.e. by default, no extensions are tested).This works by catching a custom marker
xp_extension(<ext>)
in pytest's collection hook—if the user has has passed the respective<ext>
then the test is run, otherwise it's skipped. For parameters it can just be passed in themarks
kwarg e.g.pytest.param(..., marks=xp_extension("linalg"))
, and for module level it can be defined in a file's top-level variablepytestmark
e.g.:I think I understand how
test_signatures.py
works now, so I tomorrow I should have a ready solution to easily opt-in thelinalg
tests there and intest_linalg.py
.