-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
DeprecationWarning if sync test requests async fixture #12930
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
Changes from 4 commits
6728ec5
5c41d50
8e100ea
283db4e
5beab07
2d06ff2
0de5302
1891fed
6b9de2a
94dd153
b19fd52
987904c
70639ef
7256c0c
c98ef2b
cd3eb98
2d9bb86
876cc2a
1a4dfbb
d35e4eb
ef096cd
49c140d
f434c27
7084940
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Synchronous tests that request an asynchronous fixture with ``autouse=True`` will now give a DeprecationWarning. | ||
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Synchronous tests that request asynchronous fixtures will now error, instead of silently accepting an unawaited coroutine object as the fixture value. | ||
jakkdl marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,6 +73,7 @@ | |
from _pytest.scope import _ScopeName | ||
from _pytest.scope import HIGH_SCOPES | ||
from _pytest.scope import Scope | ||
from _pytest.warning_types import PytestRemovedIn9Warning | ||
|
||
|
||
if sys.version_info < (3, 11): | ||
|
@@ -575,6 +576,7 @@ | |
# The are no fixtures with this name applicable for the function. | ||
if not fixturedefs: | ||
raise FixtureLookupError(argname, self) | ||
|
||
# A fixture may override another fixture with the same name, e.g. a | ||
# fixture in a module can override a fixture in a conftest, a fixture in | ||
# a class can override a fixture in the module, and so on. | ||
|
@@ -593,6 +595,39 @@ | |
raise FixtureLookupError(argname, self) | ||
fixturedef = fixturedefs[index] | ||
|
||
# Check for attempted use of an async fixture by a sync test | ||
# `self.scope` here is not the scope of the requested fixture, but the scope of | ||
# the requester. | ||
if ( | ||
self.scope == "function" | ||
and not inspect.iscoroutinefunction(self._pyfuncitem.obj) | ||
and ( | ||
inspect.iscoroutinefunction(fixturedef.func) | ||
or inspect.isasyncgenfunction(fixturedef.func) | ||
) | ||
): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not 100% sure this is the place to do the check, and am also not 100% it can't be triggered by a sync fixture requesting an async fixture. But for the latter I think it's covered by the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is correct, and I also see a test for this, so I guess we are good. |
||
if fixturedef._autouse: | ||
warnings.warn( | ||
PytestRemovedIn9Warning( | ||
"Sync test requested an async fixture with autouse=True. " | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add the test name to "Sync test '{name}'" and the fixture name to "async fixture '{name}'" in the phrase here to help users understand the problem better. We also should add an entry to "deprecations", with the rationale for this and guiding users on how to update their code (installing plugins, changing the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was confused for a second what you meant with "installing plugins". The way the error for async test functions handles it is by printing a long message recommending async test plugins, maybe this message should do the same. Not sure it has much of a place in the deprecations doc - if a user has a test suite that currently works the fix almost surely whouldn't be to install a new async test plugin. |
||
"If you intended to use the fixture you may want to make the " | ||
"test asynchronous. If you did not intend to use it you should " | ||
"restructure your test setup. " | ||
"This will turn into an error in pytest 9." | ||
), | ||
stacklevel=3, | ||
) | ||
else: | ||
raise FixtureLookupError( | ||
Zac-HD marked this conversation as resolved.
Show resolved
Hide resolved
|
||
argname, | ||
self, | ||
( | ||
"ERROR: Sync test requested async fixture. " | ||
"You may want to make the test asynchronous and run it with " | ||
"a suitable async framework test plugin." | ||
), | ||
) | ||
|
||
# Prepare a SubRequest object for calling the fixture. | ||
try: | ||
callspec = self._pyfuncitem.callspec | ||
|
@@ -805,7 +840,7 @@ | |
stack = [self.request._pyfuncitem.obj] | ||
stack.extend(map(lambda x: x.func, self.fixturestack)) | ||
msg = self.msg | ||
if msg is not None: | ||
if msg is not None and len(stack) > 1: | ||
jakkdl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
# The last fixture raise an error, let's present | ||
# it at the requesting side. | ||
stack = stack[:-1] | ||
|
@@ -959,6 +994,8 @@ | |
ids: tuple[object | None, ...] | Callable[[Any], object | None] | None = None, | ||
*, | ||
_ispytest: bool = False, | ||
# only used to emit a deprecationwarning, can be removed in pytest9 | ||
_autouse: bool = False, | ||
) -> None: | ||
check_ispytest(_ispytest) | ||
# The "base" node ID for the fixture. | ||
|
@@ -1005,6 +1042,9 @@ | |
self.cached_result: _FixtureCachedResult[FixtureValue] | None = None | ||
self._finalizers: Final[list[Callable[[], object]]] = [] | ||
|
||
# only used to emit a deprecationwarning, can be removed in pytest9 | ||
self._autouse = _autouse | ||
|
||
@property | ||
def scope(self) -> _ScopeName: | ||
"""Scope string, one of "function", "class", "module", "package", "session".""" | ||
|
@@ -1666,6 +1706,7 @@ | |
params=params, | ||
ids=ids, | ||
_ispytest=True, | ||
_autouse=autouse, | ||
jakkdl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
) | ||
|
||
faclist = self._arg2fixturedefs.setdefault(name, []) | ||
|
Uh oh!
There was an error while loading. Please reload this page.