apply warnings filter as soon as possible, and remove it as late as possible (#13057)

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

Author: graingert

.github/workflows/test.yml

@@ -123,6 +123,7 @@ jobs:
             python: "3.9"
             os: ubuntu-latest
             tox_env: "py39-lsof-numpy-pexpect"
+            use_coverage: true
 
           - name: "ubuntu-py39-pluggy"
             python: "3.9"

changelog/10404.bugfix.rst

@@ -0,0 +1,7 @@
+Apply filterwarnings from config/cli as soon as possible, and revert them as late as possible
+so that warnings as errors are collected throughout the pytest run and before the
+unraisable and threadexcept hooks are removed.
+
+This allows very late warnings and unraisable/threadexcept exceptions to fail the test suite.
+
+This also changes the warning that the lsof plugin issues from PytestWarning to the new warning PytestFDWarning so it can be more easily filtered.

pyproject.toml

@@ -403,6 +403,9 @@ filterwarnings = [
     "ignore:VendorImporter\\.find_spec\\(\\) not found; falling back to find_module\\(\\):ImportWarning",
     # https://github.com/pytest-dev/execnet/pull/127
     "ignore:isSet\\(\\) is deprecated, use is_set\\(\\) instead:DeprecationWarning",
+    # https://github.com/pytest-dev/pytest/issues/2366
+    # https://github.com/pytest-dev/pytest/pull/13057
+    "default::pytest.PytestFDWarning",
 ]
 pytester_example_dir = "testing/example_scripts"
 markers = [

src/_pytest/config/__init__.py

@@ -264,11 +264,11 @@ def directory_arg(path: str, optname: str) -> str:
     "setuponly",
     "setupplan",
     "stepwise",
+    "unraisableexception",
+    "threadexception",
     "warnings",
     "logging",
     "reports",
-    "unraisableexception",
-    "threadexception",
     "faulthandler",
 )
 
@@ -1112,9 +1112,7 @@ def add_cleanup(self, func: Callable[[], None]) -> None:
     def _do_configure(self) -> None:
         assert not self._configured
         self._configured = True
-        with warnings.catch_warnings():
-            warnings.simplefilter("default")
-            self.hook.pytest_configure.call_historic(kwargs=dict(config=self))
+        self.hook.pytest_configure.call_historic(kwargs=dict(config=self))
 
     def _ensure_unconfigure(self) -> None:
         try:

src/_pytest/pytester.py

@@ -65,7 +65,7 @@
 from _pytest.reports import CollectReport
 from _pytest.reports import TestReport
 from _pytest.tmpdir import TempPathFactory
-from _pytest.warning_types import PytestWarning
+from _pytest.warning_types import PytestFDWarning
 
 
 if TYPE_CHECKING:
@@ -188,7 +188,7 @@ def pytest_runtest_protocol(self, item: Item) -> Generator[None, object, object]
                     "*** function {}:{}: {} ".format(*item.location),
                     "See issue #2366",
                 ]
-                item.warn(PytestWarning("\n".join(error)))
+                item.warn(PytestFDWarning("\n".join(error)))
 
 
 # used at least by pytest-xdist plugin

src/_pytest/warning_types.py

@@ -123,6 +123,13 @@ def format(self, **kwargs: Any) -> _W:
         return self.category(self.template.format(**kwargs))
 
 
+@final
+class PytestFDWarning(PytestWarning):
+    """When the lsof plugin finds leaked fds."""
+
+    __module__ = "pytest"
+
+
 def warn_explicit_for(method: FunctionType, message: PytestWarning) -> None:
     """
     Issue the warning :param:`message` for the definition of the given :param:`method`

src/_pytest/warnings.py

@@ -3,6 +3,7 @@
 
 from collections.abc import Generator
 from contextlib import contextmanager
+from contextlib import ExitStack
 import sys
 from typing import Literal
 import warnings
@@ -17,20 +18,14 @@
 import pytest
 
 
-def pytest_configure(config: Config) -> None:
-    config.addinivalue_line(
-        "markers",
-        "filterwarnings(warning): add a warning filter to the given test. "
-        "see https://docs.pytest.org/en/stable/how-to/capture-warnings.html#pytest-mark-filterwarnings ",
-    )
-
-
 @contextmanager
 def catch_warnings_for_item(
     config: Config,
     ihook,
     when: Literal["config", "collect", "runtest"],
     item: Item | None,
+    *,
+    record: bool = True,
 ) -> Generator[None]:
     """Context manager that catches warnings generated in the contained execution block.
 
@@ -40,10 +35,7 @@ def catch_warnings_for_item(
     """
     config_filters = config.getini("filterwarnings")
     cmdline_filters = config.known_args_namespace.pythonwarnings or []
-    with warnings.catch_warnings(record=True) as log:
-        # mypy can't infer that record=True means log is not None; help it.
-        assert log is not None
-
+    with warnings.catch_warnings(record=record) as log:
         if not sys.warnoptions:
             # If user is not explicitly configuring warning filters, show deprecation warnings by default (#2908).
             warnings.filterwarnings("always", category=DeprecationWarning)
@@ -64,15 +56,19 @@ def catch_warnings_for_item(
         try:
             yield
         finally:
-            for warning_message in log:
-                ihook.pytest_warning_recorded.call_historic(
-                    kwargs=dict(
-                        warning_message=warning_message,
-                        nodeid=nodeid,
-                        when=when,
-                        location=None,
+            if record:
+                # mypy can't infer that record=True means log is not None; help it.
+                assert log is not None
+
+                for warning_message in log:
+                    ihook.pytest_warning_recorded.call_historic(
+                        kwargs=dict(
+                            warning_message=warning_message,
+                            nodeid=nodeid,
+                            when=when,
+                            location=None,
+                        )
                     )
-                )
 
 
 def warning_record_to_str(warning_message: warnings.WarningMessage) -> str:
@@ -131,3 +127,26 @@ def pytest_load_initial_conftests(
         config=early_config, ihook=early_config.hook, when="config", item=None
     ):
         return (yield)
+
+
+def pytest_configure(config: Config) -> None:
+    with ExitStack() as stack:
+        stack.enter_context(
+            catch_warnings_for_item(
+                config=config,
+                ihook=config.hook,
+                when="config",
+                item=None,
+                # this disables recording because the terminalreporter has
+                # finished by the time it comes to reporting logged warnings
+                # from the end of config cleanup. So for now, this is only
+                # useful for setting a warning filter with an 'error' action.
+                record=False,
+            )
+        )
+        config.addinivalue_line(
+            "markers",
+            "filterwarnings(warning): add a warning filter to the given test. "
+            "see https://docs.pytest.org/en/stable/how-to/capture-warnings.html#pytest-mark-filterwarnings ",
+        )
+        config.add_cleanup(stack.pop_all().close)

src/pytest/__init__.py

@@ -77,6 +77,7 @@
 from _pytest.warning_types import PytestConfigWarning
 from _pytest.warning_types import PytestDeprecationWarning
 from _pytest.warning_types import PytestExperimentalApiWarning
+from _pytest.warning_types import PytestFDWarning
 from _pytest.warning_types import PytestRemovedIn9Warning
 from _pytest.warning_types import PytestUnhandledThreadExceptionWarning
 from _pytest.warning_types import PytestUnknownMarkWarning
@@ -124,6 +125,7 @@
     "PytestConfigWarning",
     "PytestDeprecationWarning",
     "PytestExperimentalApiWarning",
+    "PytestFDWarning",
     "PytestPluginManager",
     "PytestRemovedIn9Warning",
     "PytestUnhandledThreadExceptionWarning",

testing/test_unraisableexception.py

@@ -1,5 +1,7 @@
 from __future__ import annotations
 
+from collections.abc import Generator
+import contextlib
 import gc
 import sys
 from unittest import mock
@@ -203,7 +205,25 @@ class MyError(BaseException):
     )
 
 
-def test_create_task_unraisable(pytester: Pytester) -> None:
+def _set_gc_state(enabled: bool) -> bool:
+    was_enabled = gc.isenabled()
+    if enabled:
+        gc.enable()
+    else:
+        gc.disable()
+    return was_enabled
+
+
+@contextlib.contextmanager
+def _disable_gc() -> Generator[None]:
+    was_enabled = _set_gc_state(enabled=False)
+    try:
+        yield
+    finally:
+        _set_gc_state(enabled=was_enabled)
+
+
+def test_refcycle_unraisable(pytester: Pytester) -> None:
     # see: https://github.com/pytest-dev/pytest/issues/10404
     pytester.makepyfile(
         test_it="""
@@ -221,13 +241,8 @@ def test_it():
         """
     )
 
-    was_enabled = gc.isenabled()
-    gc.disable()
-    try:
+    with _disable_gc():
         result = pytester.runpytest()
-    finally:
-        if was_enabled:
-            gc.enable()
 
     # TODO: should be a test failure or error
     assert result.ret == pytest.ExitCode.INTERNAL_ERROR
@@ -236,6 +251,101 @@ def test_it():
     result.stderr.fnmatch_lines("ValueError: del is broken")
 
 
+@pytest.mark.filterwarnings("default::pytest.PytestUnraisableExceptionWarning")
+def test_refcycle_unraisable_warning_filter(pytester: Pytester) -> None:
+    # note that the host pytest warning filter is disabled and the pytester
+    # warning filter applies during config teardown of unraisablehook.
+    # see: https://github.com/pytest-dev/pytest/issues/10404
+    pytester.makepyfile(
+        test_it="""
+        import pytest
+
+        class BrokenDel:
+            def __init__(self):
+                self.self = self  # make a reference cycle
+
+            def __del__(self):
+                raise ValueError("del is broken")
+
+        def test_it():
+            BrokenDel()
+        """
+    )
+
+    with _disable_gc():
+        result = pytester.runpytest("-Werror")
+
+    # TODO: should be a test failure or error
+    assert result.ret == pytest.ExitCode.INTERNAL_ERROR
+
+    result.assert_outcomes(passed=1)
+    result.stderr.fnmatch_lines("ValueError: del is broken")
+
+
+@pytest.mark.filterwarnings("default::pytest.PytestUnraisableExceptionWarning")
+def test_create_task_raises_unraisable_warning_filter(pytester: Pytester) -> None:
+    # note that the host pytest warning filter is disabled and the pytester
+    # warning filter applies during config teardown of unraisablehook.
+    # see: https://github.com/pytest-dev/pytest/issues/10404
+    # This is a dupe of the above test, but using the exact reproducer from
+    # the issue
+    pytester.makepyfile(
+        test_it="""
+        import asyncio
+        import pytest
+
+        async def my_task():
+            pass
+
+        def test_scheduler_must_be_created_within_running_loop() -> None:
+            with pytest.raises(RuntimeError) as _:
+                asyncio.create_task(my_task())
+        """
+    )
+
+    with _disable_gc():
+        result = pytester.runpytest("-Werror")
+
+    # TODO: should be a test failure or error
+    assert result.ret == pytest.ExitCode.INTERNAL_ERROR
+
+    result.assert_outcomes(passed=1)
+    result.stderr.fnmatch_lines("RuntimeWarning: coroutine 'my_task' was never awaited")
+
+
+def test_refcycle_unraisable_warning_filter_default(pytester: Pytester) -> None:
+    # note this time we use a default warning filter for pytester
+    # and run it in a subprocess, because the warning can only go to the
+    # sys.stdout rather than the terminal reporter, which has already
+    # finished.
+    # see: https://github.com/pytest-dev/pytest/pull/13057#discussion_r1888396126
+    pytester.makepyfile(
+        test_it="""
+        import pytest
+
+        class BrokenDel:
+            def __init__(self):
+                self.self = self  # make a reference cycle
+
+            def __del__(self):
+                raise ValueError("del is broken")
+
+        def test_it():
+            BrokenDel()
+        """
+    )
+
+    with _disable_gc():
+        result = pytester.runpytest_subprocess("-Wdefault")
+
+    assert result.ret == pytest.ExitCode.OK
+
+    # TODO: should be warnings=1, but the outcome has already come out
+    # by the time the warning triggers
+    result.assert_outcomes(passed=1)
+    result.stderr.fnmatch_lines("ValueError: del is broken")
+
+
 @pytest.mark.filterwarnings("error::pytest.PytestUnraisableExceptionWarning")
 def test_possibly_none_excinfo(pytester: Pytester) -> None:
     pytester.makepyfile(