Skip to content

make async tests and tests that return non-None fail instead of warn #12346

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

Merged
merged 6 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/11372.improvement.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Async tests will now fail, instead of warning+skipping, if you don't have any suitable plugin installed.
Copy link
Member

Choose a reason for hiding this comment

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

Traditionally we have marked changelog entries that turned warnings into errors as breaking, to stand out more.

Copy link
Member

Choose a reason for hiding this comment

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

good to know, feel free to rename the file :)

Copy link
Member

Choose a reason for hiding this comment

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

Done in #12942.

1 change: 1 addition & 0 deletions changelog/12346.misc.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Tests will now fail, instead of raising a warning, if they return any value other than None.
2 changes: 1 addition & 1 deletion doc/en/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1462,7 +1462,7 @@ pytest 7.2.0 (2022-10-23)
Deprecations
------------

- `#10012 <https://github.com/pytest-dev/pytest/issues/10012>`_: Update :class:`pytest.PytestUnhandledCoroutineWarning` to a deprecation; it will raise an error in pytest 8.
- `#10012 <https://github.com/pytest-dev/pytest/issues/10012>`_: Update ``pytest.PytestUnhandledCoroutineWarning`` to a deprecation; it will raise an error in pytest 8.


- `#10396 <https://github.com/pytest-dev/pytest/issues/10396>`_: pytest no longer depends on the ``py`` library. ``pytest`` provides a vendored copy of ``py.error`` and ``py.path`` modules but will use the ``py`` library if it is installed. If you need other ``py.*`` modules, continue to install the deprecated ``py`` library separately, otherwise it can usually be removed as a dependency.
Expand Down
2 changes: 1 addition & 1 deletion doc/en/deprecations.rst
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ Returning non-None value in test functions

.. deprecated:: 7.2

A :class:`pytest.PytestReturnNotNoneWarning` is now emitted if a test function returns something other than `None`.
A ``pytest.PytestReturnNotNoneWarning`` is now emitted if a test function returns something other than `None`.

This prevents a common mistake among beginners that expect that returning a `bool` would cause a test to pass or fail, for example:

Expand Down
6 changes: 0 additions & 6 deletions doc/en/reference/reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1229,15 +1229,9 @@ Custom warnings generated in some situations such as improper usage or deprecate
.. autoclass:: pytest.PytestExperimentalApiWarning
:show-inheritance:

.. autoclass:: pytest.PytestReturnNotNoneWarning
:show-inheritance:

.. autoclass:: pytest.PytestRemovedIn9Warning
:show-inheritance:

.. autoclass:: pytest.PytestUnhandledCoroutineWarning
:show-inheritance:

.. autoclass:: pytest.PytestUnknownMarkWarning
:show-inheritance:

Expand Down
36 changes: 17 additions & 19 deletions src/_pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,6 @@
from _pytest.scope import Scope
from _pytest.stash import StashKey
from _pytest.warning_types import PytestCollectionWarning
from _pytest.warning_types import PytestReturnNotNoneWarning
from _pytest.warning_types import PytestUnhandledCoroutineWarning


if TYPE_CHECKING:
Expand Down Expand Up @@ -135,36 +133,36 @@
)


def async_warn_and_skip(nodeid: str) -> None:
msg = "async def functions are not natively supported and have been skipped.\n"
msg += (
def async_fail(nodeid: str) -> None:
msg = (

Check warning on line 137 in src/_pytest/python.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/python.py#L137

Added line #L137 was not covered by tests
"async def functions are not natively supported.\n"
"You need to install a suitable plugin for your async framework, for example:\n"
" - anyio\n"
" - pytest-asyncio\n"
" - pytest-tornasync\n"
" - pytest-trio\n"
" - pytest-twisted"
)
msg += " - anyio\n"
msg += " - pytest-asyncio\n"
msg += " - pytest-tornasync\n"
msg += " - pytest-trio\n"
msg += " - pytest-twisted"
warnings.warn(PytestUnhandledCoroutineWarning(msg.format(nodeid)))
skip(reason="async def function and no async plugin installed (see warnings)")
fail(msg, pytrace=False)

Check warning on line 146 in src/_pytest/python.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/python.py#L146

Added line #L146 was not covered by tests


@hookimpl(trylast=True)
def pytest_pyfunc_call(pyfuncitem: Function) -> object | None:
testfunction = pyfuncitem.obj
if is_async_function(testfunction):
async_warn_and_skip(pyfuncitem.nodeid)
async_fail(pyfuncitem.nodeid)

Check warning on line 153 in src/_pytest/python.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/python.py#L153

Added line #L153 was not covered by tests
funcargs = pyfuncitem.funcargs
testargs = {arg: funcargs[arg] for arg in pyfuncitem._fixtureinfo.argnames}
result = testfunction(**testargs)
if hasattr(result, "__await__") or hasattr(result, "__aiter__"):
async_warn_and_skip(pyfuncitem.nodeid)
async_fail(pyfuncitem.nodeid)

Check warning on line 158 in src/_pytest/python.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/python.py#L158

Added line #L158 was not covered by tests
elif result is not None:
warnings.warn(
PytestReturnNotNoneWarning(
f"Expected None, but {pyfuncitem.nodeid} returned {result!r}, which will be an error in a "
"future version of pytest. Did you mean to use `assert` instead of `return`?"
)
fail(

Check warning on line 160 in src/_pytest/python.py

View check run for this annotation

Codecov / codecov/patch

src/_pytest/python.py#L160

Added line #L160 was not covered by tests
(
f"Expected None, but test returned {result!r}. "
"Did you mean to use `assert` instead of `return`?"
),
pytrace=False,
)
return True

Expand Down
18 changes: 0 additions & 18 deletions src/_pytest/warning_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,6 @@ class PytestRemovedIn9Warning(PytestDeprecationWarning):
__module__ = "pytest"


class PytestReturnNotNoneWarning(PytestWarning):
"""Warning emitted when a test function is returning value other than None."""

__module__ = "pytest"


@final
class PytestExperimentalApiWarning(PytestWarning, FutureWarning):
"""Warning category used to denote experiments in pytest.
Expand All @@ -77,18 +71,6 @@ def simple(cls, apiname: str) -> PytestExperimentalApiWarning:
return cls(f"{apiname} is an experimental api that may change over time")


@final
class PytestUnhandledCoroutineWarning(PytestReturnNotNoneWarning):
"""Warning emitted for an unhandled coroutine.

A coroutine was encountered when collecting test functions, but was not
handled by any async-aware plugin.
Coroutine test functions are not natively supported.
"""

__module__ = "pytest"


@final
class PytestUnknownMarkWarning(PytestWarning):
"""Warning emitted on use of unknown markers.
Expand Down
4 changes: 0 additions & 4 deletions src/pytest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,6 @@
from _pytest.warning_types import PytestDeprecationWarning
from _pytest.warning_types import PytestExperimentalApiWarning
from _pytest.warning_types import PytestRemovedIn9Warning
from _pytest.warning_types import PytestReturnNotNoneWarning
from _pytest.warning_types import PytestUnhandledCoroutineWarning
from _pytest.warning_types import PytestUnhandledThreadExceptionWarning
from _pytest.warning_types import PytestUnknownMarkWarning
from _pytest.warning_types import PytestUnraisableExceptionWarning
Expand Down Expand Up @@ -142,10 +140,8 @@
"PytestDeprecationWarning",
"PytestExperimentalApiWarning",
"PytestRemovedIn9Warning",
"PytestReturnNotNoneWarning",
"Pytester",
"PytestPluginManager",
"PytestUnhandledCoroutineWarning",
"PytestUnhandledThreadExceptionWarning",
"PytestUnknownMarkWarning",
"PytestUnraisableExceptionWarning",
Expand Down
35 changes: 14 additions & 21 deletions testing/acceptance_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1235,7 +1235,7 @@ def test_usage_error_code(pytester: Pytester) -> None:
assert result.ret == ExitCode.USAGE_ERROR


def test_warn_on_async_function(pytester: Pytester) -> None:
def test_error_on_async_function(pytester: Pytester) -> None:
# In the below we .close() the coroutine only to avoid
# "RuntimeWarning: coroutine 'test_2' was never awaited"
# which messes with other tests.
Expand All @@ -1251,23 +1251,19 @@ def test_3():
return coro
"""
)
result = pytester.runpytest("-Wdefault")
result = pytester.runpytest()
result.stdout.fnmatch_lines(
[
"test_async.py::test_1",
"test_async.py::test_2",
"test_async.py::test_3",
"*async def functions are not natively supported*",
"*3 skipped, 3 warnings in*",
"*test_async.py::test_1*",
"*test_async.py::test_2*",
"*test_async.py::test_3*",
]
)
# ensure our warning message appears only once
assert (
result.stdout.str().count("async def functions are not natively supported") == 1
)
result.assert_outcomes(failed=3)


def test_warn_on_async_gen_function(pytester: Pytester) -> None:
def test_error_on_async_gen_function(pytester: Pytester) -> None:
pytester.makepyfile(
test_async="""
async def test_1():
Expand All @@ -1278,20 +1274,16 @@ def test_3():
return test_2()
"""
)
result = pytester.runpytest("-Wdefault")
result = pytester.runpytest()
result.stdout.fnmatch_lines(
[
"test_async.py::test_1",
"test_async.py::test_2",
"test_async.py::test_3",
"*async def functions are not natively supported*",
"*3 skipped, 3 warnings in*",
"*test_async.py::test_1*",
"*test_async.py::test_2*",
"*test_async.py::test_3*",
]
)
# ensure our warning message appears only once
assert (
result.stdout.str().count("async def functions are not natively supported") == 1
)
result.assert_outcomes(failed=3)


def test_pdb_can_be_rewritten(pytester: Pytester) -> None:
Expand Down Expand Up @@ -1377,14 +1369,15 @@ def test_no_brokenpipeerror_message(pytester: Pytester) -> None:
popen.stderr.close()


def test_function_return_non_none_warning(pytester: Pytester) -> None:
def test_function_return_non_none_error(pytester: Pytester) -> None:
pytester.makepyfile(
"""
def test_stuff():
return "something"
"""
)
res = pytester.runpytest()
res.assert_outcomes(failed=1)
res.stdout.fnmatch_lines(["*Did you mean to use `assert` instead of `return`?*"])


Expand Down
Loading