Merge pull request #12346 from polkapolka/async-tests-issue-warning

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

changelog/11372.improvement.rst

@@ -0,0 +1 @@
+Async tests will now fail, instead of warning+skipping, if you don't have any suitable plugin installed.

changelog/12346.misc.rst

@@ -0,0 +1 @@
+Tests will now fail, instead of raising a warning, if they return any value other than None.

doc/en/changelog.rst

@@ -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.

doc/en/deprecations.rst

@@ -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:
 

doc/en/reference/reference.rst

@@ -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:
 

src/_pytest/python.py

@@ -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:
@@ -135,36 +133,36 @@ def pytest_configure(config: Config) -> None:
     )
 
 
-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 = (
+        "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)
 
 
 @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)
     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)
     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(
+            (
+                f"Expected None, but test returned {result!r}. "
+                "Did you mean to use `assert` instead of `return`?"
+            ),
+            pytrace=False,
         )
     return True
 

src/_pytest/warning_types.py

@@ -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.
@@ -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.

src/pytest/__init__.py

@@ -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
@@ -142,10 +140,8 @@
     "PytestDeprecationWarning",
     "PytestExperimentalApiWarning",
     "PytestRemovedIn9Warning",
-    "PytestReturnNotNoneWarning",
     "Pytester",
     "PytestPluginManager",
-    "PytestUnhandledCoroutineWarning",
     "PytestUnhandledThreadExceptionWarning",
     "PytestUnknownMarkWarning",
     "PytestUnraisableExceptionWarning",

testing/acceptance_test.py

@@ -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.
@@ -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():
@@ -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:
@@ -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`?*"])