Skip to content

Conversation

@marqh
Copy link
Member

@marqh marqh commented Oct 24, 2016

enable test data path to be overridden at run time, via an environment variable

@marqh marqh added this to the v1.11 milestone Oct 24, 2016
TEST_DATA_DIR = None
override = os.environ.get("override_test_data_repository")
if override:
if override == '1':
Copy link
Member

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?

Copy link
Member Author

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

@DPeterK
Copy link
Member

DPeterK commented Oct 24, 2016

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 👍

@DPeterK DPeterK merged commit 8983e38 into SciTools:master Oct 24, 2016
if override == '1':
TEST_DATA_DIR = None
else:
TEST_DATA_DIR = override
Copy link
Member

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 = None

Also, 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 ... 😉

Copy link
Member

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 ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants