Skip to content

Mysterious logwarning warnings only happen with xdist under Pytest 4.1 #406

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

Closed
Zac-HD opened this issue Jan 28, 2019 · 8 comments
Closed

Comments

@Zac-HD
Copy link
Member

Zac-HD commented Jan 28, 2019

Here's a CI log from HypothesisWorks/hypothesis#1782. To reproduce this problem, clone Hypothesis (and for posterity, check out the current master d1b0df5).

pip install -r requirements/test.txt
pip install -e hypothesis-python/
pip install pytest==4.1
pytest hypothesis-python/tests/cover/test_testdecorators.py       # quick subset, passes
pytest hypothesis-python/tests/cover/test_testdecorators.py -n 2  # fails thanks to -Werror

So somewhere, our dependencies must be using a pytest_logwarning, right? Except that I can't find such a location in any of them, and we're not using any pytest plugins aside from xdist itself!

Do you have any idea what's happening here, or how I might find out? It's been blocking our move to Pytest 4.1 for weeks now 😞

@RonnyPfannschmidt
Copy link
Member

@Zac-HD as a workaround i propose ignoring that specific warning

but i notice that -Werror is quite a pain as it hides passed on location information

@Zac-HD
Copy link
Member Author

Zac-HD commented Jan 28, 2019

Thanks for tracking that down! I'd seen both chunks of code but assumed that using the latest versions of each would not emit warnings on the default configuration.

Is there a way to handle this upstream in pytest-xdist? I'd prefer to fix it for everyone and minimize the number of workarounds we have to maintain in Hypothesis.

@RonnyPfannschmidt
Copy link
Member

pytest-xdist already conditionally imports the hook, i believe simply not implementing it when the warning is there suffices

https://github.com/pytest-dev/pluggy/blob/master/pluggy/hooks.py#L37 suggests if we not only check for the existence, but also for (untested) pytest_spec.get("warn_on_impl") things should be fine

@nicoddemus
Copy link
Member

I think we can simply stop implementing it, given that we will remove it anyway in the future.

@RonnyPfannschmidt
Copy link
Member

@nicoddemus we should take a look at the support matrix then just in case

@nicoddemus
Copy link
Member

@RonnyPfannschmidt went ahead with your suggestion given that it is really simple anyway. 👍

@Zac-HD
Copy link
Member Author

Zac-HD commented Jan 28, 2019

Two hours from report to merged fix... you guys are legends 😍

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