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

Multiple executors health check triggers with trivial pytest test #3733

Closed
The-Compiler opened this issue Sep 8, 2023 · 6 comments · Fixed by #3734
Closed

Multiple executors health check triggers with trivial pytest test #3733

The-Compiler opened this issue Sep 8, 2023 · 6 comments · Fixed by #3734
Labels
enhancement it's not broken, but we want it to be better legibility make errors helpful and Hypothesis grokable

Comments

@The-Compiler
Copy link
Contributor

After upgrading to 6.83.2, my testsuite started to trigger health checks, with rather boring test cases which can be simplified to:

import pytest

from hypothesis import given
from hypothesis.strategies import text

class TestSomething:

    @given(x=text())
    @pytest.mark.parametrize("i", range(2))
    def test_meth(self, x, i):
        pass

leading to:

__________________________ TestSomething.test_meth[1] __________________________

self = <test.TestSomething object at 0x7efc623890d0>, i = 1

    @given(x=text())
>   @pytest.mark.parametrize("i", range(2))
E   hypothesis.errors.FailedHealthCheck: The method TestSomething.test_meth was called from multiple different executors. This may lead to flaky tests and nonreproducible errors when replaying from database.
E   See https://hypothesis.readthedocs.io/en/latest/healthchecks.html for more information about this. If you want to disable just this health check, add HealthCheck.differing_executors to the suppress_health_check settings for this test.

test.py:9: FailedHealthCheck

After reading through the linked page and the custom function execution page linked from there, I feel like I still have no idea what I'm looking at. This all seems to be far more sophisticated than what I'm doing here.

After reviewing #3720 (the title alone made things clearer), I believe this is triggered because pytest (like unittest) creates a new instance of the test class for every test run? To me, this seems like a very normal usage of Hypothesis and pytest, so I'm not sure if this outcome is intended. If it is, I feel like the documentation is rather lacking for people who are not familiar with the background from Hypothesis' perspective.

cc @jobh

The-Compiler added a commit to qutebrowser/qutebrowser that referenced this issue Sep 8, 2023
See HypothesisWorks/hypothesis#3733
Should fix nightly builds (and the next dependency upgrade).
@Zac-HD
Copy link
Member

Zac-HD commented Sep 8, 2023

Yeah, this does seem awfully noisy for parameterized tests. Maybe we should have our purest plugin disable the check for such tests? That happens late enough that we'll be able to access the class as well, so checking whether we have multiple types of self (ie actually inherited the test).

Wait, no, even better: we deal with the equivalent problem for parameterized tests by having an additional component mixed into the database key - we should make the health check fail only if self and the db key are the same.

Or finally, just disable this healthcheck for parametrized tests where it's expected and we expect it to be fine. Duh.

@Zac-HD Zac-HD added enhancement it's not broken, but we want it to be better legibility make errors helpful and Hypothesis grokable labels Sep 9, 2023
@Zac-HD
Copy link
Member

Zac-HD commented Sep 9, 2023

@The-Compiler if you can confirm whether my PR fixes this for you, I'll ship it in the morning. (or try to fix it, if not!)

@The-Compiler
Copy link
Contributor Author

The-Compiler commented Sep 9, 2023

After removing my own suppression of the health check and a

pip install git+https://github.com/Zac-HD/hypothesis@narrower-healthcheck#subdirectory=hypothesis-python

I do still get the warnings.

It's unclear to me why. I can try to investigate some more, but running out of time for today unfortunately.

@The-Compiler
Copy link
Contributor Author

Hm, I think what I'm seeing now boils down to this:

import pytest

import hypothesis
from hypothesis.strategies import text


@pytest.fixture(params=["a", "b"])
def fixt(request):
    return request.param


class TestSomething:
    @hypothesis.settings(
        suppress_health_check=[hypothesis.HealthCheck.function_scoped_fixture]
    )
    @hypothesis.given(x=text())
    def test_meth(self, x, fixt):
        pass

happening here: https://github.com/qutebrowser/qutebrowser/blob/v3.0.0/tests/unit/config/test_configtypes.py#L173-L215

I suppose I could in theory rewrite those to use a shared @pytest.mark.parametrize instead of a parametrized fixture, but this still seems like a false-positive for this particular health check I think.

@Zac-HD
Copy link
Member

Zac-HD commented Sep 10, 2023

Yeah, I think we can suppress this for tests with parameterized fixtures too.

@The-Compiler
Copy link
Contributor Author

Seems to work smoothly now against qutebrowser - thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement it's not broken, but we want it to be better legibility make errors helpful and Hypothesis grokable
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants