-
-
Notifications
You must be signed in to change notification settings - Fork 283
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
Use pytest.importorskip in tests #492
Conversation
def skip_test_env_var(name): | ||
""" Checks for environment variables indicating whether tests requiring services should be run | ||
""" | ||
value = os.environ.get(name, '0') |
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.
Should we handle empty strings? If so, this might help.
value = os.environ.get(name, '0') | |
value = os.environ.get(name, '') or '0' |
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.
Hmm I'm not sure. Is an empty string a common pattern for disabling things with an env var? If so, then it's probably worth supporting here. No strong opinion from me one way or the other
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's more so that it is not a common pattern for enabling things. So the concern would be someone accidentally enabling these.
That said, let's leave it since it was already this way and we can follow-up in another issue/PR.
Thanks for working on this James! This looks much better.
|
Thanks @jrbourbeau! 😄 |
This PR replaces (almost all) occurrences of
try
/except
blocks to determine if a library is available withpytest.importorskip
. I wasn't able to replace thisLZMA
importzarr-python/zarr/tests/test_core.py
Lines 1944 to 1947 in 55ae9eb
because it's used in other conditional statements in
test_core.py
.Also added a
skip_test_env_var
utility function tozarr/tests/util.py
to replace some of the boilerplate code related to skipping tests based on environment variables (e.g.ZARR_TEST_ABS
)Closes #229
TODO:
tox -e docs
)