-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Fix tests that violate isolation #1822
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
astropy/units/tests/test_units.py
Outdated
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.
Sorry I'm just noticing this for the first time now. 👍
|
Need to rebase this and see how it goes |
|
@embray - how are you identifying these by the way? |
|
It's simply a matter of opening the Python interpreter and running |
…s the use of parameters.values() in a context where the order of the parameter values *does* matter. The fact that it was working before was only by luck. The other problem was that it modified global state, causing subsequent test executions to break.
…this should be redone in the context of astropy#2347
|
Rebased and added one additional fix. This fixes all cases I could find currently where some test didn't reset global state (at least in such a way that it affected subsequent test runs). |
|
@embray You said that this can reveal such problems? Would it be possible to add a travis-ci build for this to make sure this problem doesn't get re-introduced in the future? |
|
Actually I do see a test error in that doesn't show up when running https://gist.github.com/cdeil/11171946 |
|
@cdeil: It looks like |
|
Otherwise I'll go ahead and merge this. I'm not entirely sure it's worth adding yet another test configuration for this case, which is really a corner case. It's just worth checking every now and then... I'm open to suggestions though. Maybe if not on Travis then on Jenkins would be fine... |
|
I think we should just merge for now. Agree we don't want to add another test configuration. I could add one on the Mac Jenkins instance. |
Fix tests that violate isolation
Fix tests that violate isolation Conflicts: astropy/modeling/tests/test_models.py
The purpose of this PR is to fix a handful of tests that modify global state in some way, either unnecessarily, or necessarily but without restoring the previous state. This causes tests to fail if they are run multiple times within the same Python session (mainly, multiple subsequent runs of
astropy.test()).This PR is not complete--there are still a few more to knock out. But I'm putting this up now so I don't forget about it.