-
Notifications
You must be signed in to change notification settings - Fork 17
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
Added additional information to error when data submodule is not present #20
Conversation
scico/test/test_data.py
Outdated
if len(os.listdir(os.path.abspath("./data"))) == 0: | ||
raise IOError( | ||
"\nThe data submodule must be cloned and initialized. If already cloned use the following in the root directory:\n\tgit submodule update --init --recursive\nOtherwise, make sure to clone using:\n\tgit clone --recurse-submodules [email protected]:lanl/scico.git\nAnd after cloning run:\n\tgit submodule init && git submodule update." | ||
) | ||
x = data.kodim23() |
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.
Perhaps cleaner to make this a separate test rather than include it in both existing tests?
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 is, but this way, the other two tests don't run and give the other error which has no information as to what's happening. Maybe I can expand a bit on the error message if I use your approach and mention the rest of the cases might not run? I'm currently looking at a way to stop this particular test class completely if the data module isnt available, and throw the appropiate error to pytest. We can leave it on hold for now, but this can be a good starting point.
In that case, I assume I should consider this PR to be on hold for now? |
I managed to make a check that skips the test module entirely if the data submodule is not present, If a dev wants to see the information/reason of skip (itll show the reason for every test), run the test case using pytest -rs [path]. |
scico/test/test_data.py
Outdated
from scico import data | ||
|
||
pytestmark = pytest.mark.skipif( | ||
len(os.listdir(os.path.abspath("./data"))) == 0, | ||
reason="\nThe data submodule must be cloned and initialized. If the main repository is already cloned, use the following in the root directory to get the data submodule:\n\tgit submodule update --init --recursive\nOtherwise, make sure to clone using:\n\tgit clone --recurse-submodules [email protected]:lanl/scico.git\nAnd after cloning run:\n\tgit submodule init && git submodule update.\n", |
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.
Maximum line length exceeded.
Resolves #1 |
Added individually to each method but looking to make a modularized version using either fixtures or something similar. At first glance, that idea did not seem to work, but I'll check other alternatives out.