-
Notifications
You must be signed in to change notification settings - Fork 47
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
MNT: Compat with pytest 8.1 #219
Conversation
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.
Could you also revert bd7b5c6, so this could be tested?
(and suggestion for the maintainers: add a job for testing with the 8.0.x branch (RC) just as well with pytestdev)
Good catch; reverted. Thanks! |
I have no idea what this test failure is in pytest-dev job
|
That commit I reverted was from #215 by @ConorMacBride . I have no idea what he meant by "Skip pytestdev until hook wrapper issue is fixed" as there is no further explanation in the PR nor any linked issue. |
The hook wrapper issue is #216 |
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.
When I tested a fix for #216 last month, the tests passed with the pytest main branch installed. I did not see any mention of this issue, which seems strange as I would have expected the deprecation warning to be raised as an exception. Have you seen this pytest-mpl hook raise an exception?
We didn't see deprecation warnings in other plugins either and similar failures popped up when they became actual errors. I haven't dug deep enough to get a full understanding but the plugins/pytest doesn't always behave the same predictable way as a normal test dependency library. |
and fix up the other pytest 8 compat problem with old-style hook wrapper. Update flake8 setting to reflect what is actually in the code.
and remove unnecessary pytest version check I added earlier.
pytestdev is green but the log has traceback, is this expected? I don't maintain this plugin so I don't know what the test is supposed to do. https://github.com/matplotlib/pytest-mpl/actions/runs/7464235520/job/20310772695?pr=219 |
Would be nice to get this merged sooner than later if this patch is acceptable. astropy devinfra job is failing without this. |
@ConorMacBride , does this look good to you now? Can we merge this? I don't have write access to this repo but astropy depends on it. |
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.
To avoid this in pytest 8.1.dev:
See pytest-dev/pytest#11779 (comment)
Fix #216 , xref pytest-dev/pytest#11714 (comment)