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

DeprecationWarning if sync test requests async fixture #12930

Merged
merged 24 commits into from
Nov 17, 2024
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6728ec5
Raise error if sync test relies on async fixture, and warn if the fix…
jakkdl Oct 31, 2024
5c41d50
Merge remote-tracking branch 'origin/main' into sync_test_async_fixture
jakkdl Nov 1, 2024
8e100ea
fix tests
jakkdl Nov 1, 2024
283db4e
add changelog
jakkdl Nov 1, 2024
5beab07
improve warning message. Make it warn regardless of autouse or not. A…
jakkdl Nov 6, 2024
2d06ff2
Merge branch 'main' into sync_test_async_fixture
jakkdl Nov 6, 2024
0de5302
fix test
jakkdl Nov 6, 2024
1891fed
Rename changelog entries to 'breaking' (#12942)
nicoddemus Nov 6, 2024
6b9de2a
[pre-commit.ci] pre-commit autoupdate
pre-commit-ci[bot] Oct 21, 2024
94dd153
Upgrade pylint version, activate all extensions
Pierre-Sassoulas Oct 29, 2024
b19fd52
[pylint dict-init-mutate] Initialize a dict right off for speed
Pierre-Sassoulas Oct 29, 2024
987904c
[automated] Update plugin list (#12950)
github-actions[bot] Nov 10, 2024
70639ef
build(deps): Bump django in /testing/plugins_integration (#12951)
dependabot[bot] Nov 11, 2024
7256c0c
build(deps): Bump pypa/gh-action-pypi-publish from 1.10.3 to 1.12.2 (…
dependabot[bot] Nov 11, 2024
c98ef2b
change implementation so the check happens in pytest_fixture_setup af…
jakkdl Nov 11, 2024
cd3eb98
fix test
jakkdl Nov 11, 2024
2d9bb86
Merge branch 'main' into sync_test_async_fixture
jakkdl Nov 11, 2024
876cc2a
update docs/changelog
jakkdl Nov 11, 2024
1a4dfbb
remove incorrect comments, add link
jakkdl Nov 14, 2024
d35e4eb
revert now unrelated fix
jakkdl Nov 14, 2024
ef096cd
small wording changes
jakkdl Nov 14, 2024
49c140d
Apply suggestions from code review
jakkdl Nov 17, 2024
f434c27
be more explicit about the downside of async autouse fixtures
jakkdl Nov 17, 2024
7084940
fix tests after message change
jakkdl Nov 17, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/10839.deprecation.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Requesting an asynchronous fixture without a `pytest_fixture_setup` to handle it will now give a DeprecationWarning. This most commonly happens if a sync test requests an async fixture. This should have no effect on a majority of users with async tests or fixtures, but may affect non-standard hook setups or ``autouse=True``. For guidance on how to work around this warning see :ref:`sync-test-async-fixture`.
59 changes: 59 additions & 0 deletions doc/en/deprecations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,65 @@ Below is a complete list of all pytest features which are considered deprecated.
:class:`~pytest.PytestWarning` or subclasses, which can be filtered using :ref:`standard warning filters <warnings>`.


.. _sync-test-async-fixture:

sync test depending on async fixture
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. deprecated:: 8.4

Pytest has for a long time given an error when encountering an asynchronous test function, prompting the user to install
a plugin that can handle it. It has not given any errors if you have an asynchronous fixture that's depended on by a
synchronous test. If the fixture was an async function you did get an "unawaited coroutine" warning, but for async yield fixtures you didn't even get that.
This is a problem even if you do have a plugin installed for handling async tests, as they may require
special decorators for async fixtures to be handled, and some may not robustly handle if a user accidentally requests an
async fixture from their sync tests. Fixture values being cached can make this even more unintuitive, where everything will
"work" if the fixture is first requested by an async test, and then requested by a synchronous test.

Unfortunately there is no 100% reliable method of identifying when a user has made a mistake, versus when they expect an
unawaited object from their fixture that they will handle on their own. To suppress this warning
when you in fact did intend to handle this you can wrap your async fixture in a synchronous fixture:

.. code-block:: python

import asyncio
import pytest


@pytest.fixture
async def unawaited_fixture():
return 1


def test_foo(unawaited_fixture):
assert 1 == asyncio.run(unawaited_fixture)

should be changed to


.. code-block:: python

import asyncio
import pytest


@pytest.fixture
def unawaited_fixture():
async def inner_fixture():
return 1

return inner_fixture()


def test_foo(unawaited_fixture):
assert 1 == asyncio.run(unawaited_fixture)


You can also make use of `pytest_fixture_setup` to handle the coroutine/asyncgen before pytest sees it - this is the way current async pytest plugins handle it.

If a user has an async fixture with ``autouse=True`` in their ``conftest.py``, or in a file where they also have synchronous tests, they will also get this warning. We strongly recommend against this practice, and they should restructure their testing infrastructure so the fixture is synchronous or to separate the fixture from their synchronous tests. Note that the anyio pytest plugin has some support for sync test + async fixtures currently.


.. _import-or-skip-import-error:

``pytest.importorskip`` default behavior regarding :class:`ImportError`
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ disable = [
]

[tool.codespell]
ignore-words-list = "afile,asser,assertio,feld,hove,ned,noes,notin,paramete,parth,socio-economic,tesults,varius,wil"
ignore-words-list = "afile,asend,asser,assertio,feld,hove,ned,noes,notin,paramete,parth,socio-economic,tesults,varius,wil"
skip = "*/plugin_list.rst"
write-changes = true

Expand Down
1 change: 1 addition & 0 deletions src/_pytest/compat.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ class NotSetType(enum.Enum):


def is_generator(func: object) -> bool:
# note: this also returns true for async generator functions
genfunc = inspect.isgeneratorfunction(func)
return genfunc and not iscoroutinefunction(func)

Expand Down
31 changes: 30 additions & 1 deletion src/_pytest/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -805,7 +807,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:
# The last fixture raise an error, let's present
# it at the requesting side.
stack = stack[:-1]
Expand Down Expand Up @@ -883,6 +885,8 @@
fixturefunc: _FixtureFunc[FixtureValue], request: FixtureRequest, kwargs
) -> FixtureValue:
if is_generator(fixturefunc):
# note: this also triggers on async generators, suppressing 'unawaited coroutine'
# warning.
fixturefunc = cast(
Callable[..., Generator[FixtureValue, None, None]], fixturefunc
)
Expand Down Expand Up @@ -959,6 +963,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.
Expand Down Expand Up @@ -1005,6 +1011,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"."""
Expand Down Expand Up @@ -1136,6 +1145,25 @@

fixturefunc = resolve_fixture_function(fixturedef, request)
my_cache_key = fixturedef.cache_key(request)

if inspect.isasyncgenfunction(fixturefunc) or inspect.iscoroutinefunction(
fixturefunc
):
auto_str = " with autouse=True" if fixturedef._autouse else ""

Check warning on line 1152 in src/_pytest/fixtures.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L1152

Added line #L1152 was not covered by tests

warnings.warn(

Check warning on line 1154 in src/_pytest/fixtures.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L1154

Added line #L1154 was not covered by tests
PytestRemovedIn9Warning(
f"{request.node.name!r} requested an async fixture "
f"{request.fixturename!r}{auto_str}, with no plugin or hook that "
"handled it. This is usually an error, as pytest does not natively "
"support it. If this is intentional, consider making the fixture "
"sync and return a coroutine/asyncgen. "
"This will turn into an error in pytest 9."
),
# no stacklevel will point at users code, so we just point here
stacklevel=1,
)

try:
result = call_fixture_func(fixturefunc, request, kwargs)
except TEST_OUTCOME as e:
Expand Down Expand Up @@ -1666,6 +1694,7 @@
params=params,
ids=ids,
_ispytest=True,
_autouse=autouse,
)

faclist = self._arg2fixturedefs.setdefault(name, [])
Expand Down
101 changes: 101 additions & 0 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1286,6 +1286,107 @@ def test_3():
result.assert_outcomes(failed=3)


def test_warning_on_sync_test_async_fixture(pytester: Pytester) -> None:
pytester.makepyfile(
test_sync="""
import pytest

@pytest.fixture
async def async_fixture():
...

def test_foo(async_fixture):
# suppress unawaited coroutine warning
try:
async_fixture.send(None)
except StopIteration:
pass
"""
)
result = pytester.runpytest()
result.stdout.fnmatch_lines(
[
"*== warnings summary ==*",
(
"*PytestRemovedIn9Warning: 'test_foo' requested an async "
"fixture 'async_fixture', with no plugin or hook that handled it. "
"This is usually an error, as pytest does not natively support it. "
"If this is intentional, consider making the fixture sync and return "
"a coroutine/asyncgen. "
"This will turn into an error in pytest 9."
),
]
)
result.assert_outcomes(passed=1, warnings=1)


def test_warning_on_sync_test_async_fixture_gen(pytester: Pytester) -> None:
pytester.makepyfile(
test_sync="""
import pytest

@pytest.fixture
async def async_fixture():
yield

def test_foo(async_fixture):
# we don't need to suppress RuntimeWarning for unawaited coroutine
# as pytest internals accidentally do so already for async gens
...
"""
)
result = pytester.runpytest()
result.stdout.fnmatch_lines(
[
"*== warnings summary ==*",
(
"*PytestRemovedIn9Warning: 'test_foo' requested an async "
"fixture 'async_fixture', with no plugin or hook that handled it. "
"This is usually an error, as pytest does not natively support it. "
"If this is intentional, consider making the fixture sync and return "
"a coroutine/asyncgen. "
"This will turn into an error in pytest 9."
),
]
)
result.assert_outcomes(passed=1, warnings=1)


def test_warning_on_sync_test_async_autouse_fixture(pytester: Pytester) -> None:
pytester.makepyfile(
test_sync="""
import pytest

@pytest.fixture(autouse=True)
async def async_fixture():
...

# We explicitly request the fixture to be able to
# suppress the RuntimeWarning for unawaited coroutine.
def test_foo(async_fixture):
try:
async_fixture.send(None)
except StopIteration:
pass
"""
)
result = pytester.runpytest()
result.stdout.fnmatch_lines(
[
"*== warnings summary ==*",
(
"*PytestRemovedIn9Warning: 'test_foo' requested an async "
"fixture 'async_fixture' with autouse=True, with no plugin or hook "
"that handled it. This is usually an error, as pytest does not "
"natively support it. If this is intentional, consider making the "
"fixture sync and return a coroutine/asyncgen. "
"This will turn into an error in pytest 9."
),
]
)
result.assert_outcomes(passed=1, warnings=1)


def test_pdb_can_be_rewritten(pytester: Pytester) -> None:
pytester.makepyfile(
**{
Expand Down
Loading