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.

Sign up for GitHub

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

Jump to bottom

CI Failures Associated with Pytest v8.0.0 Release #8681

Closed
andersy005 opened this issue Jan 29, 2024 · 2 comments · Fixed by #8686
Closed

CI Failures Associated with Pytest v8.0.0 Release #8681

andersy005 opened this issue Jan 29, 2024 · 2 comments · Fixed by #8686
Labels
CI Continuous Integration tools topic-testing

Comments

@andersy005
Copy link
Member

andersy005 commented Jan 29, 2024 •
edited
Loading

What is your issue?

A recent release of pytest (v8.0.0) appears to have broken our CI.

pytest                    8.0.0              pyhd8ed1ab_0    conda-forge
pytest-cov                4.1.0              pyhd8ed1ab_0    conda-forge
pytest-env                1.1.3              pyhd8ed1ab_0    conda-forge
pytest-github-actions-annotate-failures 0.2.0                    pypi_0    pypi
pytest-timeout            2.2.0              pyhd8ed1ab_0    conda-forge
pytest-xdist              3.5.0              pyhd8ed1ab_0    conda-forge

Strangely, the issue doesn't seem to occur when using previous versions (e.g. v7.4.4). our last successful CI run used pytest v7.4.4

pytest                    7.4.4              pyhd8ed1ab_0    conda-forge
pytest-cov                4.1.0              pyhd8ed1ab_0    conda-forge
pytest-env                1.1.3              pyhd8ed1ab_0    conda-forge
pytest-github-actions-annotate-failures 0.2.0                    pypi_0    pypi
pytest-timeout            2.2.0              pyhd8ed1ab_0    conda-forge
pytest-xdist              3.5.0              pyhd8ed1ab_0    conda-forge

i recreated the environment and successfully ran tests locally. the CI failures appear to be connected to the latest release of pytest. i haven't had a chance to do an in-depth exploration of the changes from pytest which could be influencing this disruption. so, i wanted to open an issue to track what is going on. in the meantime, i'm going to pin pytest to an earlier version.

any insights, especially from those familiar with changes in the pytest v8.0.0 update, are warmly welcomed.
@andersy005 andersy005 added needs triage Issue that has not been reviewed by xarray team member CI Continuous Integration tools and removed needs triage Issue that has not been reviewed by xarray team member labels Jan 29, 2024
@andersy005 andersy005 mentioned this issue Jan 29, 2024
Merged
4 tasks
@ksunden
Copy link
Contributor

ksunden commented Jan 30, 2024

The biggest change which seems to be affecting the tests here is that now pytest.warns does not capture all warnings, where previously it did.

We had similar changes needed in mpl: matplotlib/matplotlib#27624

In particular, nesting pytest.raises and pytest.warns in pytest 7 would cause things to not fail even if one of those conditions were not met (e.g. the exception is raised before the warning would have been issued) so in pytest 8 this now (correctly I think) tells you when you thought you were testing that a warning was issued that is not, in fact, actually issued.

A quick scan of the failing tests looked like all of them had that pattern of nested raises and warns. The fix is either to ensure that expected warnings are issued before exceptions interrupt control flow or to not test for warnings that are not issued.

@mgorny
Copy link
Contributor

mgorny commented Jan 31, 2024

I'm going to try making a pull request for this.

mgorny added a commit to mgorny/xarray that referenced this issue Jan 31, 2024
@mgorny
test_groupby: Clear recorded warnings for pytest-8 compatibility
3bc5abf 
Clear the warnings recorded during the `pytest.warns()` use
in `test_groupby_dims_property`, to fix test failures with pytest-8.0.0.
Prior to this version, `pytest.warns()` invocation used to capture all
warnings.  Now it only captures the warnings that match the arguments,
and the remaining warnings are re-emitted and therefore caught by
`recwarn` fixture.  To provide compatibility with both versions of
pytest, clear the recorded warnings immediately after `pytest.warns()`.

Fixes pydata#8681
@mgorny mgorny mentioned this issue Jan 31, 2024
Merged
4 tasks
dcherian pushed a commit that referenced this issue Jan 31, 2024
@mgorny
Fixes for pytest-8.0.0 compatibility (#8686)
ca10531 
* test_dataset: remove incorrect pytest.warns() to fix pytest-8

Remove two incorrect `pytest.warns()` assertions to fix test failures
with pytest-8.0.0.  Prior to this version, an exception raised would
cause `pytest.warns()` to be ignored.  This way fixed in 8.0.0, and now
warnings must actually be emitted prior to the exception.

In `test_drop_index_labels()`, the exception is raised at the very
beginning of the function, prior to the deprecation warning.

In `test_rename_multiindex()`, the warning is not emitted at all (it is
not applicable to the call in question).

* test_groupby: Clear recorded warnings for pytest-8 compatibility

Clear the warnings recorded during the `pytest.warns()` use
in `test_groupby_dims_property`, to fix test failures with pytest-8.0.0.
Prior to this version, `pytest.warns()` invocation used to capture all
warnings.  Now it only captures the warnings that match the arguments,
and the remaining warnings are re-emitted and therefore caught by
`recwarn` fixture.  To provide compatibility with both versions of
pytest, clear the recorded warnings immediately after `pytest.warns()`.

Fixes #8681

* Revert "Fix CI: temporary pin pytest version to 7.4.* (#8682)"

This reverts commit b0b5b2f.
The tests should be fixed now.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
CI Continuous Integration tools topic-testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fixes for pytest-8.0.0 compatibility mgorny/xarray
4 participants
@mgorny @ksunden @andersy005 @TomNicholas