diff --git a/AUTHORS b/AUTHORS index 86a814a137b..6cecb0d56f8 100644 --- a/AUTHORS +++ b/AUTHORS @@ -63,6 +63,7 @@ Ceridwen Charles Cloud Charles Machalow Charnjit SiNGH (CCSJ) +Cheuk Ting Ho Chris Lamb Chris NeJame Chris Rose diff --git a/changelog/7337.improvement.rst b/changelog/7337.improvement.rst new file mode 100644 index 00000000000..74d98d9b6c9 --- /dev/null +++ b/changelog/7337.improvement.rst @@ -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`. diff --git a/doc/en/deprecations.rst b/doc/en/deprecations.rst index 91944b758f9..3bbd29bb509 100644 --- a/doc/en/deprecations.rst +++ b/doc/en/deprecations.rst @@ -252,6 +252,47 @@ or ``pytest.warns(Warning)``. 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 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/doc/en/reference/reference.rst b/doc/en/reference/reference.rst index d082697258c..69e8690e73e 100644 --- a/doc/en/reference/reference.rst +++ b/doc/en/reference/reference.rst @@ -1130,6 +1130,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.PytestUnhandledCoroutineWarning :show-inheritance: diff --git a/src/_pytest/python.py b/src/_pytest/python.py index cd951939e7a..4d1f722e474 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -77,10 +77,12 @@ from _pytest.pathlib import visit from _pytest.scope import Scope from _pytest.warning_types import PytestCollectionWarning +from _pytest.warning_types import PytestReturnNotNoneWarning from _pytest.warning_types import PytestUnhandledCoroutineWarning if TYPE_CHECKING: from typing_extensions import Literal + from _pytest.scope import _ScopeName @@ -192,6 +194,13 @@ def pytest_pyfunc_call(pyfuncitem: "Function") -> Optional[object]: result = testfunction(**testargs) if hasattr(result, "__await__") or hasattr(result, "__aiter__"): 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 diff --git a/src/_pytest/warning_types.py b/src/_pytest/warning_types.py index ac79bb53acc..ebfeb87d842 100644 --- a/src/_pytest/warning_types.py +++ b/src/_pytest/warning_types.py @@ -55,6 +55,13 @@ class PytestRemovedIn8Warning(PytestDeprecationWarning): __module__ = "pytest" +@final +class PytestReturnNotNoneWarning(PytestDeprecationWarning): + """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. diff --git a/src/pytest/__init__.py b/src/pytest/__init__.py index 777d3774064..c1634e296bc 100644 --- a/src/pytest/__init__.py +++ b/src/pytest/__init__.py @@ -69,6 +69,7 @@ from _pytest.warning_types import PytestDeprecationWarning from _pytest.warning_types import PytestExperimentalApiWarning from _pytest.warning_types import PytestRemovedIn8Warning +from _pytest.warning_types import PytestReturnNotNoneWarning from _pytest.warning_types import PytestUnhandledCoroutineWarning from _pytest.warning_types import PytestUnhandledThreadExceptionWarning from _pytest.warning_types import PytestUnknownMarkWarning @@ -127,6 +128,7 @@ "PytestDeprecationWarning", "PytestExperimentalApiWarning", "PytestRemovedIn8Warning", + "PytestReturnNotNoneWarning", "Pytester", "PytestPluginManager", "PytestUnhandledCoroutineWarning", diff --git a/testing/acceptance_test.py b/testing/acceptance_test.py index 92661b0f23a..c7139b538b2 100644 --- a/testing/acceptance_test.py +++ b/testing/acceptance_test.py @@ -1292,3 +1292,14 @@ def test_no_brokenpipeerror_message(pytester: Pytester) -> None: # Cleanup. 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`?*"])