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

Prevent double wrapping of inherited Hypothesis tests #239

Conversation

seifertm
Copy link
Contributor

@seifertm seifertm commented Jan 6, 2022

When looking at #231 I thought that something is wrong with the setup and teardown of the event_loop fixture. Running pytest with --show-setup proved that this is not the case, though:

tests/hypothesis/test_inherited_test.py 
        SETUP    F event_loop
        tests/hypothesis/test_inherited_test.py::TestOne::test_hypothesis (fixtures used: event_loop).
        TEARDOWN F event_loop
        SETUP    F event_loop
        tests/hypothesis/test_inherited_test.py::TestTwo::test_hypothesis (fixtures used: event_loop).
        TEARDOWN F event_loop

I dug further and noticed that regular async tests are executed just fine. Only tests that were using Hypothesis raised errors when they were executed in a subclass. Eventually, it turned out that subclassing caused multiple synchronous function wrappers to be layered around inherited Hypothesis tests. Here's why:

Pytest-asyncio identifies Hypothesis test cases by their is_hypothesis_test attribute. When setting up an async Hypothesis test pytest-asyncio replaces the function's hypothesis.inner_test attribute. The the top level function never changes.

When a Hypothesis test case is defined in a base class and inherited by subclasses, the test is collected in each subclass. Since the top-level Hypothesis test never changes, its inner test will be wrapped multiple times.

Double wrapping leads to execution errors caused by stale (closed) event loops in all test executions after the first. This also explains why a module scoped fixture could serve as a workaround for the OP in #231.

This change adds an original_test_function attribute to the async function wrapper, in order to keep track of the original Hypothesis test. When re-wrapping would occur in subclasses pytest-asyncio wraps the original test function rather than the wrapper function.

Closes #231

… and renamed it to "test_base".

This allows us to add more tests for the Hypothesis integration while at the same time keep different tests tidy by using different files.

Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
@seifertm seifertm force-pushed the prevent_double_wrapping_of_inherited_hypothesis_test branch from 5639a37 to 927f376 Compare January 7, 2022 07:31
Pytest-asyncio identifies Hypothesis test cases by their `is_hypothesis_test` flag. When setting up an async Hypothesis test pytest-asyncio replaces the function's `hypothesis.inner_test` attribute. The the top level function never changes.

When a Hypothesis test case is defined in a base class and inherited by subclasses, the test is collected in each subclass. Since the top-level Hypothesis test never changes, its inner test will be wrapped multiple times.

Double wrapping leads to execution errors caused by stale (closed) event loops in all test executions after the first.

This change adds an `original_test_function` attribute to the async function wrapper, in order to keep track of the original Hypothesis test. When re-wrapping would occur in subclasses pytest-asyncio wraps the original test function rather than the wrapper function.

Closes pytest-dev#231

Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
This is an important information for downstream packagers.

Signed-off-by: Michael Seifert <m.seifert@digitalernachschub.de>
@seifertm seifertm force-pushed the prevent_double_wrapping_of_inherited_hypothesis_test branch from 927f376 to dc74a20 Compare January 7, 2022 07:38
@codecov-commenter
Copy link

codecov-commenter commented Jan 7, 2022

Codecov Report

Merging #239 (dc74a20) into master (57ccbc7) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #239   +/-   ##
=======================================
  Coverage   95.83%   95.83%           
=======================================
  Files           2        2           
  Lines         144      144           
=======================================
  Hits          138      138           
  Misses          6        6           
Impacted Files Coverage Δ
pytest_asyncio/plugin.py 95.80% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57ccbc7...dc74a20. Read the comment docs.

@seifertm
Copy link
Contributor Author

seifertm commented Jan 7, 2022

Rebased to master. #220 addresses the same issue in a slightly different way. I adjusted the PR accordingly.

The PR now:

  • Fixes a linting issue
  • Links the fixes issues in the changelog
  • Mentions flaky as an added test dependency in the changelog. This is an important information for downstream packagers.

Copy link
Contributor

@asvetlov asvetlov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Tinche Tinche merged commit 430d69a into pytest-dev:master Jan 7, 2022
@seifertm seifertm deleted the prevent_double_wrapping_of_inherited_hypothesis_test branch October 23, 2023 06:14
@seifertm seifertm restored the prevent_double_wrapping_of_inherited_hypothesis_test branch October 23, 2023 08:17
@seifertm seifertm deleted the prevent_double_wrapping_of_inherited_hypothesis_test branch October 23, 2023 08:35
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.

Shared tests with hypothesis throws an Event loop is closed error
4 participants