Skip to content

Conversation

@embray
Copy link
Member

@embray embray commented Nov 23, 2013

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.

Copy link
Contributor

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

@embray
Copy link
Member Author

embray commented Feb 14, 2014

Need to rebase this and see how it goes

@astrofrog
Copy link
Member

@embray - how are you identifying these by the way?

@embray
Copy link
Member Author

embray commented Feb 17, 2014

It's simply a matter of opening the Python interpreter and running astropy.test() twice in a row. It should succeed no matter how many times it is run. I rebased locally, and I think there is just one test that is still breaking things....

@embray embray modified the milestones: v0.3.2, v0.3.1 Feb 25, 2014
@embray embray self-assigned this Apr 18, 2014
…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.
@embray
Copy link
Member Author

embray commented Apr 21, 2014

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

@cdeil
Copy link
Member

cdeil commented Apr 22, 2014

@embray You said that this can reveal such problems?

python -c 'import astropy; astropy.test(); astropy.test()'

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?

@cdeil
Copy link
Member

cdeil commented Apr 22, 2014

Actually I do see a test error in test_validate in astropy/io/votable/tests/vo_test.py:812 when running

python -c 'import astropy; astropy.test()

that doesn't show up when running

python setup.py test

https://gist.github.com/cdeil/11171946
cc @mdboom

@mdboom
Copy link
Contributor

mdboom commented Apr 22, 2014

@cdeil: It looks like astropy.test() is not ignoring astropy.cfg like it should. I'll file a PR shortly.

@embray
Copy link
Member Author

embray commented Apr 23, 2014

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

@astrofrog
Copy link
Member

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.

embray added a commit that referenced this pull request Apr 23, 2014
Fix tests that violate isolation
@embray embray merged commit fce71d4 into astropy:master Apr 23, 2014
@embray embray deleted the test-isolation branch April 23, 2014 15:17
embray added a commit that referenced this pull request Apr 23, 2014
Fix tests that violate isolation
Conflicts:

	astropy/modeling/tests/test_models.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants