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

TST: Update for pytest 7 #83

Merged
merged 9 commits into from
Nov 2, 2023
Merged

TST: Update for pytest 7 #83

merged 9 commits into from
Nov 2, 2023

Conversation

StefRe
Copy link
Contributor

@StefRe StefRe commented Oct 25, 2023

Remove 36 PytestRemovedIn8Warning and PytestReturnNotNoneWarning warnings.

Remove 36 PytestRemovedIn8Warning and PytestReturnNotNoneWarning warnings.
@StefRe
Copy link
Contributor Author

StefRe commented Oct 25, 2023

Python 3.5 and 3.6 are no longer supported by ubuntu-latest (22.04) on Github workflows.

@phobson
Copy link
Member

phobson commented Oct 30, 2023

@StefRe thanks for the update! I can take a look at this soon

@phobson phobson self-assigned this Oct 30, 2023
@phobson
Copy link
Member

phobson commented Oct 30, 2023

@StefRe -- anything else you think should go in here?

@StefRe
Copy link
Contributor Author

StefRe commented Oct 31, 2023

Looking at the CI test output, there are still 16 warnings. I think they are all due to missing pytest plugins:

  • PytestConfigWarning: Unknown config option: pep8ignore
    --> maybe we can remove this from setup as the tests files are checked with black anyway, so there shouldn't be any PEP8 violations in there
  • PytestReturnNotNoneWarning for all tests decorated with mpl_image_compare. I guess this is because the pytest-mpl plugin is not properly installed. I don't see these warnings locally with pytest-mpl version 0.16.1 installed. If I'm not mistaken it turns out that CI doesn't test anything for all these tests and this went unnoticed with pytest versions < 7.

@StefRe
Copy link
Contributor Author

StefRe commented Nov 1, 2023

I only now saw that all the tests are run twice: first in Basic unit tests and then in Image comparison tests. The latter installs pytest-mpl and hence runs without warnings. Both workflows run, however, the complete test suite. What is actually the idea behind splitting the tests into two workflows?

@phobson
Copy link
Member

phobson commented Nov 1, 2023

What is actually the idea behind splitting the tests into two workflows?

I'm sure I had a reason at the time. Maybe the img tests were flaky and allowed to fail? In any case, I can't think of a good reason now.

@phobson
Copy link
Member

phobson commented Nov 1, 2023

Oh also the img tests are only run on the "latest" version of python, not e.g., 3.8, 3.9, etc. Maybe that should change

@StefRe
Copy link
Contributor Author

StefRe commented Nov 1, 2023

My point was that Basic unit tests and Image comparison tests do the same thing, i.e. all tests are run twice (even more so now that the python version matrices are identical). So if we want to run the image comparison tests separately then maybe it's clearer to add -m "not mpl_image_compare" to python check_probscale.py --doctest-modules in python-runtests-basic.yml and -m mpl_image_compare to python check_probscale.py --mpl --doctest-modules in python-runtests-img-comp.yml, respectively.

@phobson
Copy link
Member

phobson commented Nov 1, 2023

Yeah I agree. Since all python versions in the img-comp matrix are passing, I'd be inclined to scrap the "vanilla" tests altogether.

@phobson phobson merged commit 60e0c3c into matplotlib:master Nov 2, 2023
7 checks passed
@StefRe StefRe deleted the fix/pytest branch November 3, 2023 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants