Warn when test functions return other than None (#9956)

Closes #7337
This commit is contained in:
Cheuk Ting Ho 2022-05-25 06:48:02 -06:00 committed by GitHub
parent 31f9e5bcdd
commit c988e49af6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 75 additions and 0 deletions

View File

@ -63,6 +63,7 @@ Ceridwen
Charles Cloud Charles Cloud
Charles Machalow Charles Machalow
Charnjit SiNGH (CCSJ) Charnjit SiNGH (CCSJ)
Cheuk Ting Ho
Chris Lamb Chris Lamb
Chris NeJame Chris NeJame
Chris Rose Chris Rose

View File

@ -0,0 +1 @@
A warning 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` (for example `return foo(a, b) == result`) would cause a test to pass or fail, instead of using `assert`.

View File

@ -252,6 +252,47 @@ or ``pytest.warns(Warning)``.
See :ref:`warns use cases` for examples. See :ref:`warns use cases` for examples.
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`.
This prevents a common mistake among beginners that expect that returning a `bool` would cause a test to pass or fail, for example:
.. code-block:: python
@pytest.mark.parametrize(
["a", "b", "result"],
[
[1, 2, 5],
[2, 3, 8],
[5, 3, 18],
],
)
def test_foo(a, b, result):
return foo(a, b) == result
Given that pytest ignores the return value, this might be surprising that it will never fail.
The proper fix is to change the `return` to an `assert`:
.. code-block:: python
@pytest.mark.parametrize(
["a", "b", "result"],
[
[1, 2, 5],
[2, 3, 8],
[5, 3, 18],
],
)
def test_foo(a, b, result):
assert foo(a, b) == result
The ``--strict`` command-line option The ``--strict`` command-line option
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

View File

@ -1130,6 +1130,9 @@ Custom warnings generated in some situations such as improper usage or deprecate
.. autoclass:: pytest.PytestExperimentalApiWarning .. autoclass:: pytest.PytestExperimentalApiWarning
:show-inheritance: :show-inheritance:
.. autoclass:: pytest.PytestReturnNotNoneWarning
:show-inheritance:
.. autoclass:: pytest.PytestUnhandledCoroutineWarning .. autoclass:: pytest.PytestUnhandledCoroutineWarning
:show-inheritance: :show-inheritance:

View File

@ -77,10 +77,12 @@ from _pytest.pathlib import parts
from _pytest.pathlib import visit from _pytest.pathlib import visit
from _pytest.scope import Scope from _pytest.scope import Scope
from _pytest.warning_types import PytestCollectionWarning from _pytest.warning_types import PytestCollectionWarning
from _pytest.warning_types import PytestReturnNotNoneWarning
from _pytest.warning_types import PytestUnhandledCoroutineWarning from _pytest.warning_types import PytestUnhandledCoroutineWarning
if TYPE_CHECKING: if TYPE_CHECKING:
from typing_extensions import Literal from typing_extensions import Literal
from _pytest.scope import _ScopeName from _pytest.scope import _ScopeName
@ -192,6 +194,13 @@ def pytest_pyfunc_call(pyfuncitem: "Function") -> Optional[object]:
result = testfunction(**testargs) result = testfunction(**testargs)
if hasattr(result, "__await__") or hasattr(result, "__aiter__"): if hasattr(result, "__await__") or hasattr(result, "__aiter__"):
async_warn_and_skip(pyfuncitem.nodeid) async_warn_and_skip(pyfuncitem.nodeid)
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`?"
)
)
return True return True

View File

@ -55,6 +55,13 @@ class PytestRemovedIn8Warning(PytestDeprecationWarning):
__module__ = "pytest" __module__ = "pytest"
@final
class PytestReturnNotNoneWarning(PytestDeprecationWarning):
"""Warning emitted when a test function is returning value other than None."""
__module__ = "pytest"
@final @final
class PytestExperimentalApiWarning(PytestWarning, FutureWarning): class PytestExperimentalApiWarning(PytestWarning, FutureWarning):
"""Warning category used to denote experiments in pytest. """Warning category used to denote experiments in pytest.

View File

@ -69,6 +69,7 @@ from _pytest.warning_types import PytestConfigWarning
from _pytest.warning_types import PytestDeprecationWarning from _pytest.warning_types import PytestDeprecationWarning
from _pytest.warning_types import PytestExperimentalApiWarning from _pytest.warning_types import PytestExperimentalApiWarning
from _pytest.warning_types import PytestRemovedIn8Warning from _pytest.warning_types import PytestRemovedIn8Warning
from _pytest.warning_types import PytestReturnNotNoneWarning
from _pytest.warning_types import PytestUnhandledCoroutineWarning from _pytest.warning_types import PytestUnhandledCoroutineWarning
from _pytest.warning_types import PytestUnhandledThreadExceptionWarning from _pytest.warning_types import PytestUnhandledThreadExceptionWarning
from _pytest.warning_types import PytestUnknownMarkWarning from _pytest.warning_types import PytestUnknownMarkWarning
@ -127,6 +128,7 @@ __all__ = [
"PytestDeprecationWarning", "PytestDeprecationWarning",
"PytestExperimentalApiWarning", "PytestExperimentalApiWarning",
"PytestRemovedIn8Warning", "PytestRemovedIn8Warning",
"PytestReturnNotNoneWarning",
"Pytester", "Pytester",
"PytestPluginManager", "PytestPluginManager",
"PytestUnhandledCoroutineWarning", "PytestUnhandledCoroutineWarning",

View File

@ -1292,3 +1292,14 @@ def test_no_brokenpipeerror_message(pytester: Pytester) -> None:
# Cleanup. # Cleanup.
popen.stderr.close() popen.stderr.close()
def test_function_return_non_none_warning(testdir) -> None:
testdir.makepyfile(
"""
def test_stuff():
return "something"
"""
)
res = testdir.runpytest()
res.stdout.fnmatch_lines(["*Did you mean to use `assert` instead of `return`?*"])