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

Open
wants to merge 21 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 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
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` hook that resolves 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 using async pytest plugins, 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 <https://anyio.readthedocs.io/en/stable/testing.html>`_ 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
27 changes: 27 additions & 0 deletions 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 @@ -959,6 +961,8 @@
ids: tuple[object | None, ...] | Callable[[Any], object | None] | None = None,
*,
_ispytest: bool = False,
# only used in a deprecationwarning msg, 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 +1009,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 +1143,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 1150 in src/_pytest/fixtures.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/fixtures.py#L1150

Added line #L1150 was not covered by tests

warnings.warn(

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
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 +1692,7 @@
params=params,
ids=ids,
_ispytest=True,
_autouse=autouse,
jakkdl marked this conversation as resolved.
Show resolved Hide resolved
)

faclist = self._arg2fixturedefs.setdefault(name, [])
Expand Down
100 changes: 100 additions & 0 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1286,6 +1286,106 @@ 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()
Zac-HD marked this conversation as resolved.
Show resolved Hide resolved
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):
# async gens don't emit unawaited-coroutine
...
"""
)
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