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

Hypothesis fixture scope warning does not detect if fixture is overridden #2375

Closed
simonfagerholm opened this issue Mar 23, 2020 · 6 comments

Comments

@simonfagerholm
Copy link
Contributor

simonfagerholm commented Mar 23, 2020

In #2356 a warning was added if @given() is used together with function scoped fixtures.
However the warning functionality does not discover if a fixture is overridden by a "closer" definition, e.g. if a fixture with the same name is defined in the test file. See image below of how fx_defs looks with multiple definitions.
image
This was discovered while fixing pytest-dev/pytest-asyncio#145.

@Zac-HD: Do you have any ideas?

@Zac-HD
Copy link
Member

Zac-HD commented Mar 23, 2020

This might be as simple as reversing our iteration order, or we might need to do something a little fancier...

We also need to add tests for cases where a function-scoped fixture is overridden with a safe scope, and vice-versa!

@simonfagerholm
Copy link
Contributor Author

simonfagerholm commented Mar 23, 2020

@Zac-HD: I did a quick test and this approach seems to work at least in this case:

for fx in fx_defs:
    fxture = item._request._get_active_fixturedef(fx.argname)
    if fxture.scope == "function":
        note_deprecation(
            "%s uses the %r fixture, but function-scoped fixtures "
            "should not be used with @given(...) tests, because "
            "fixtures are not reset between generated examples!"
            % (item.nodeid, fx.argname),
            since="2020-02-29",
        )

I used the internal _get_active_fixturedef to get the same fixture that pytest is using for this item. I figured this might be better than guessing pytests approach to order fixtures, but it might have other problems.

@simonfagerholm
Copy link
Contributor Author

Maybe this is better to prevent initializing fixtures that are not being used:

if fx.argname in argnames:
    fxture = item._request._get_active_fixturedef(
        fx.argname)
    if fxture.scope == "function":
        note_deprecation(
            "%s uses the %r fixture, but function-scoped"
            " fixtures should not be used with @given(...)"
            " tests, because fixtures are not reset "
            "between generated examples!"
            % (item.nodeid, fx.argname),
            since="2020-02-29",
        )

@Zac-HD
Copy link
Member

Zac-HD commented Mar 23, 2020

That looks great to me - want to open a PR?

@simonfagerholm
Copy link
Contributor Author

@Zac-HD Sure, I will get on it

DRMacIver added a commit that referenced this issue Mar 23, 2020
Added support and unittest for overridden fixtures, solving #2375.
@Zac-HD
Copy link
Member

Zac-HD commented Mar 23, 2020

Closed by #2376.

@Zac-HD Zac-HD closed this as completed Mar 23, 2020
Tinche pushed a commit to pytest-dev/pytest-asyncio that referenced this issue Mar 29, 2020
clrpackages pushed a commit to clearlinux-pkgs/pytest-asyncio that referenced this issue Apr 22, 2020
…oop provided by pytest-asyncio instead of creating a new loop.

Albert Tugushev (1):
      Replace yield_fixture() by fixture()

Andrew Svetlov (1):
      Test on Python 3.8, drop 3.3 and 3.4

Jim Crist (1):
      Handle BaseExceptions from loop.run_until_complete (#126)

Michael Seifert (3):
      Sync wrapper for hypothesis tests uses the event loop provided by pytest-asyncio instead of creating a new loop.
      Removed a layer of indirection when checking for markers.
      Simplified fixture setup by relying on the fact that get_event_loop returns the loop provided by the event_loop fixture of pytest-asyncio.

Orion Poplawski (1):
      Fix required pytest version

Romain Létendart (1):
      plugin: Use pytest 5.4.0 new Function API

Timothy Fitz (1):
      Add test for hypothesis event_loop reuse.

Tin Tvrtkovic (2):
      0.11 open for business.
      0.11.0

simonfagerholm (4):
      Enable test_subprocess to be run on win, by changing to ProactorEventLoop in those tests.
      Change event_loop to module scope in hypothesis tests, fixing #145.
      Add max supported pytest version to < 5.4.0 to prevent fails until #141 is fixed.
      Added min hypothesis version so that bugfix for HypothesisWorks/hypothesis#2375 is installed, solving build issues.
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

2 participants