-
Notifications
You must be signed in to change notification settings - Fork 300
override test data path #2211
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
override test data path #2211
Conversation
| TEST_DATA_DIR = None | ||
| override = os.environ.get("override_test_data_repository") | ||
| if override: | ||
| if override == '1': |
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.
What if the passed value is neither '1' nor a proper path to the test data?
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.
then the test data directory will not be found, and the tests will fall back to the 'no data' approach of skipping
|
Other than the single comment above I think this is a useful side-step for missing test data in an environment that Iris is deployed to 👍 |
| if override == '1': | ||
| TEST_DATA_DIR = None | ||
| else: | ||
| TEST_DATA_DIR = override |
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.
@marqh We may want to be a wee bit more general here, see how the iris.tests.runner._runner.finalize_options sets this environment variable.
How about:
if os.path.isdir(override):
TEST_DATA_DIR = override
else:
TEST_DATA_DIR = NoneAlso, I think it's pretty bad style to have lower case environment variables ... now might be an opportunity to change that (if you care) since we're in this space ... 😉
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.
@dkillick @marqh I think that we need to fix this given the comment above. The finalize_options sets the override_test_data_repository = 'true' which means the above merged code won't behave as intended ...
enable test data path to be overridden at run time, via an environment variable