Skip to content
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

Tests failing with pytest 7.2.0 #6505

Closed
dstansby opened this issue Oct 25, 2022 · 5 comments
Closed

Tests failing with pytest 7.2.0 #6505

dstansby opened this issue Oct 25, 2022 · 5 comments
Labels
Bug Probably a bug Tests Affects tests in some measure Upstream Fix Required A fix needs to go upstream to another package

Comments

@dstansby
Copy link
Member

Describe the bug

With pytest 7.2.0 the following tests are failing:

 FAILED ../../.tox/py310/lib/python3.10/site-packages/sunpy/map/tests/test_map_factory.py::test_data_filenames
FAILED ../../.tox/py310/lib/python3.10/site-packages/sunpy/io/tests/test_fits.py::test_data_filenames
FAILED ../../.tox/py310/lib/python3.10/site-packages/sunpy/map/sources/tests/test_eit_source.py::test_data_filenames
FAILED ../../.tox/py310/lib/python3.10/site-packages/sunpy/physics/tests/test_differential_rotation.py::test_differential_rotation
FAILED ../../.tox/py310/lib/python3.10/site-packages/sunpy/timeseries/tests/test_timeseries_factory.py::test_data_filenames

with messages like:

which will be an error in a future version of pytest.  Did you mean to use `assert` instead of `return`?

I'm guessing these tests are unintentionally returning something?

I'm going to fix this by pinning pytest<7.2 on the 4.0 branch to get 4.0.6 out, but when a fix is merged to main and backported we should remember to remove that pytest pin on the 4.0 branch.

To Reproduce

import sunpy

sunpy.map.Map(...)
etc

Screenshots

No response

System Details

No response

Installation method

No response

@dstansby
Copy link
Member Author

We're now getting this mysterious warning with the new release of pytest-xdist (pytest-dev/pytest-xdist#828 is the PR where the warning came in I think):

INTERNALERROR>     warnings.warn(warning, stacklevel=stacklevel)
INTERNALERROR> DeprecationWarning: The --rsyncdir command line argument and rsyncdirs config variable are deprecated.
INTERNALERROR> The rsync feature will be removed in pytest-xdist 4.0.

@nabobalis nabobalis added Tests Affects tests in some measure Upstream Fix Required A fix needs to go upstream to another package labels Oct 25, 2022
@ConorMacBride
Copy link
Member

The test_data_filenames errors have been fixed by #6507.

test_differential_rotation needs astropy/pytest-arraydiff#36 to be released, however, I don't think we are actually running pytest-arraydiff. It needs to be enabled with --arraydiff. Do we want to enable it? When I do that the test fails with:

Not equal to tolerance rtol=1e-07, atol=0

Mismatched elements: 10 / 16384 (0.061%)
Max absolute difference: 0.001
Max relative difference: 5.55484577e-06

@nabobalis
Copy link
Contributor

nabobalis commented Oct 25, 2022

Oh did we mark the test and then never run it properly?

We should enable it or refactor the test.

@ayshih
Copy link
Member

ayshih commented Oct 26, 2022

Heh, test_differential_rotation traces back to #3245 (Jun 2019), and then I updated the differential-rotation code in #4979 (Feb 2021). Sometime between those two PRs, we must have stopped enabling pytest-arraydiff in the CI, because the test wasn't updated accordingly in #4979.

@dstansby
Copy link
Member Author

I can't work out where, but at some point I think this was fixed - with pytest 7.2.1 the tests I mentioned in the original issue seem to work fine for me now locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Probably a bug Tests Affects tests in some measure Upstream Fix Required A fix needs to go upstream to another package
Projects
None yet
Development

No branches or pull requests

4 participants