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

Pluggy 1.4.0 now warns about deprecated hook wrapper style #221

Closed
neutrinoceros opened this issue Jan 24, 2024 · 8 comments
Closed

Pluggy 1.4.0 now warns about deprecated hook wrapper style #221

neutrinoceros opened this issue Jan 24, 2024 · 8 comments

Comments

@neutrinoceros
Copy link
Contributor

Pluggy 1.4.0 was released a couple hours ago.
It nows warns about old-style hook wrappers:

@pytest.hookimpl(hookwrapper=True)
def pytest_runtest_call(self, item): # noqa

@pytest.hookimpl(hookwrapper=True)
def pytest_runtest_call(self, item):

See the release notes and docs.

@pllim
Copy link
Contributor

pllim commented Jan 24, 2024

Hmm, so the force_exception added in #219 didn't fix this problem? Does that mean #216 is needed sooner than later?

@neutrinoceros
Copy link
Contributor Author

#219 fixed an incompatibility with pytest 8.1, but the new warning is coming from pluggy itself.

I supposed this could be closed as a dupe of #216 actually. Sorry about the noise !

@pllim
Copy link
Contributor

pllim commented Jan 24, 2024

@neutrinoceros , are you sure #219 didn't fix the problem in astropy ? CircleCI is also green with dev version of pytest-mpl (astropy/astropy#15942). Maybe instead of implementing #216 , all we need is a new release of pytest-mpl (which would be faster to do and is needed anyway since the last release was 2 years ago).

@pllim
Copy link
Contributor

pllim commented Jan 24, 2024

Now I am confused, see astropy/astropy#15929 (comment)

@neutrinoceros
Copy link
Contributor Author

Well I'm sure that #216 needs to happen to fix the problem I reported here, but I'm as confused as you are regarding the apparent flakiness of CI.
I also don't get how #219 is supposed to fix astropy's CI if it's not released yet.

@pllim
Copy link
Contributor

pllim commented Jan 24, 2024

is supposed to fix astropy's CI if it's not released yet

It cannot officially "fix the CI" but would indicate a new patch here isn't necessary, just a new release. But again, maybe I was just lucky that it passed using dev version of pytest-mpl if the failures don't pop up all the time. 🤷‍♀️

@ConorMacBride
Copy link
Member

If the only error you are seeing is PluggyTeardownRaisedWarning, that should be fixed on the the main branch of pytest-mpl. The pluggy release notes links the new warning to pytest-dev/pluggy#463, which was motivated by me seeing a misleading error message in pytest-dev/pytest#11714.

When I was seeing that error before (i.e. error raised in teardown) it was very flaky. It appeared in different tests every time I ran pytest. If you temporarily replace image: pytest-mpl with image: git+https://github.com/matplotlib/pytest-mpl.git in /tox.ini in astropy/astropy#15929 and allow pluggy 1.4, the image tests will hopefully run without any issues.

There's just one PR that needs reviewed and merged before a new pytest-mpl release can be published.

@neutrinoceros
Copy link
Contributor Author

Thanks a lot for the clarification @ConorMacBride !

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

No branches or pull requests

3 participants