diff --git a/AUTHORS b/AUTHORS index 106d9b8f7f2..27c0b3ac408 100644 --- a/AUTHORS +++ b/AUTHORS @@ -370,6 +370,7 @@ Patrick Hayes Patrick Lannigan Paul Müller Paul Reece +Paul Zuradzki Pauli Virtanen Pavel Karateev Pavel Zhukov diff --git a/changelog/14263.bugfix.rst b/changelog/14263.bugfix.rst new file mode 100644 index 00000000000..dba08435b18 --- /dev/null +++ b/changelog/14263.bugfix.rst @@ -0,0 +1 @@ +Unraisable exceptions from finalizers are now collected during ``pytest_unconfigure``, before pytest tears down the warning filters installed for the session. Previously the collection ran from a cleanup callback whose order relative to other plugins' cleanups was not guaranteed, so an active ``error`` filter could be removed before the exception surfaced and a late resource leak would pass silently. A ``-W error`` filter, or any filter matching :class:`pytest.PytestUnraisableExceptionWarning`, now promotes these exceptions to failures regardless of plugin cleanup order. diff --git a/src/_pytest/unraisableexception.py b/src/_pytest/unraisableexception.py index 259f0d8e7f4..6c092fb6bd3 100644 --- a/src/_pytest/unraisableexception.py +++ b/src/_pytest/unraisableexception.py @@ -153,6 +153,26 @@ def pytest_configure(config: Config) -> None: sys.unraisablehook = functools.partial(unraisable_hook, append=deque.append) +def pytest_unconfigure(config: Config) -> None: + # Runs before ``_cleanup_stack.close()``, so warning filters from + # cleanup-stack-managed contexts (notably the ``warnings`` plugin's + # ``catch_warnings``) are still installed when garbage-collected + # finalizers fire. A ``config.add_cleanup`` callback would instead + # couple correctness to LIFO pop order across plugins' cleanups. + if unraisable_exceptions not in config.stash: + # ``pytest_configure`` did not complete (e.g. a usage error raised + # in another plugin's configure), so the queue stash was never set. + return + # PyPy can resurrect objects in __del__, so it needs several GC passes + # (5, per the Trio project); CPython frees cycles in one pass. See #14441. + _default_gc_collect_iterations = 5 if sys.implementation.name == "pypy" else 1 + gc_collect_iterations = config.stash.get( + gc_collect_iterations_key, _default_gc_collect_iterations + ) + gc_collect_harder(gc_collect_iterations) + collect_unraisable(config) + + @pytest.hookimpl(trylast=True) def pytest_runtest_setup(item: Item) -> None: collect_unraisable(item.config) diff --git a/testing/test_unraisableexception.py b/testing/test_unraisableexception.py index a6a4d6f35e8..acc098a8ff8 100644 --- a/testing/test_unraisableexception.py +++ b/testing/test_unraisableexception.py @@ -317,6 +317,8 @@ def test_scheduler_must_be_created_within_running_loop() -> None: # TODO: Should be a test failure or error. Currently the exception # propagates all the way to the top resulting in exit code 1. + # -Werror matches the wrapping PytestUnraisableExceptionWarning, so the + # wrapper propagates; its message embeds the original RuntimeWarning. assert result.ret == 1 result.assert_outcomes(passed=1) @@ -359,6 +361,138 @@ def test_it(): result.stderr.fnmatch_lines("ValueError: del is broken") +def test_unraisable_warning_without_filter_still_wraps(pytester: Pytester) -> None: + # A Warning raised from ``__del__`` gets wrapped in + # PytestUnraisableExceptionWarning like any other unraisable exception. + # pytest does not unwrap it to honor a filter on the inner class, so with + # no filter matching the wrapper, pytest logs the warning and the run + # passes. This guards against re-introducing the dropped unwrap path. + pytester.makepyfile( + test_it=""" + class RaisingDel: + def __del__(self): + raise UserWarning("oops") + + def test_it(): + obj = RaisingDel() + del obj + """ + ) + + # Subprocess so we don't inherit the outer pytest's ``error`` filter from + # pyproject.toml, which would promote the wrapper and fail the run. + # ``-Wdefault::pytest.PytestUnraisableExceptionWarning`` makes the wrapping + # warning visible on stderr whether ``__del__`` fires inside the test or + # during later GC (PyPy). + result = pytester.runpytest_subprocess( + "-Wdefault::pytest.PytestUnraisableExceptionWarning" + ) + + assert result.ret == 0 + result.assert_outcomes(passed=1) + # The unraisable hook emitted PytestUnraisableExceptionWarning. + # The warning lands in the warnings-summary section of stdout on + # CPython (where ``__del__`` fires inside the test) and on stderr on + # PyPy (where it fires during later GC). Check both rather than the + # outcomes counter, which is timing-dependent. + combined = "\n".join(result.outlines + result.errlines) + assert "PytestUnraisableExceptionWarning" in combined + + +@pytest.mark.skipif(PYPY, reason="garbage-collection differences make this flaky") +def test_unraisable_decouples_from_cleanup_stack_order(pytester: Pytester) -> None: + # Regression test for the structural fix. The garbage-collection step + # that surfaces queued unraisables must run before _cleanup_stack.close() + # so warning filters installed via cleanup-stack-managed contexts are + # still in effect when finalizers fire. Otherwise correctness depends + # on the order in which plugins register their cleanups under LIFO. + # + # The conftest uses ``@hookimpl(trylast=True)`` so its pytest_configure + # runs after all built-in pytest_configures. Its + # ``warnings.resetwarnings`` cleanup then lands on the cleanup stack + # last and pops first under LIFO. Under the pre-fix layout, where + # garbage collection runs inside unraisableexception's own cleanup + # callback, that pop clears the user's + # ``error::pytest.PytestUnraisableExceptionWarning`` filter before the + # finalizer raises; pytest logs the wrapped warning instead of + # promoting it, and the suite exits 0. Under the post-fix layout, + # ``pytest_unconfigure`` runs the garbage collection and queue processing + # before the cleanup stack starts closing, so the conftest's reset has no + # effect. + pytester.makeini( + """ + [pytest] + filterwarnings = + error::pytest.PytestUnraisableExceptionWarning + """ + ) + pytester.makeconftest( + """ + import warnings + import pytest + + @pytest.hookimpl(trylast=True) + def pytest_configure(config): + config.add_cleanup(warnings.resetwarnings) + """ + ) + pytester.makepyfile( + test_it=""" + import gc; gc.disable() + + class BrokenDel: + def __init__(self): + self.self = self # cycle survives until session-end gc + + def __del__(self): + raise ValueError("del is broken") + + def test_it(): + BrokenDel() + """ + ) + + result = pytester.runpytest_subprocess() + + assert result.ret == 1, ( + "wrapper filter is still installed at GC time, so the unraisable " + "exits non-zero regardless of cleanup-stack pop order" + ) + result.assert_outcomes(passed=1) + result.stderr.fnmatch_lines("*ValueError: del is broken*") + + +def test_pytest_unconfigure_survives_failed_pytest_configure( + pytester: Pytester, +) -> None: + # Regression test for the guard against an unset stash key. When + # another plugin's pytest_configure raises pytest.UsageError, pluggy + # stops calling the remaining configure hooks; the unraisable plugin's + # pytest_configure never runs and config.stash[unraisable_exceptions] + # stays unset. pytest_unconfigure still fires for partially-configured + # runs, so without a presence check the unraisable plugin's + # pytest_unconfigure raises KeyError and pytest reports INTERNALERROR + # instead of USAGE_ERROR. + pytester.makeconftest( + """ + import pytest + + def pytest_configure(config): + raise pytest.UsageError("simulated bad config") + """ + ) + pytester.makepyfile(test_it="def test_it(): pass") + + result = pytester.runpytest_subprocess() + + assert result.ret == pytest.ExitCode.USAGE_ERROR, ( + "the UsageError must surface as USAGE_ERROR, not a KeyError INTERNALERROR" + ) + result.stderr.fnmatch_lines("*ERROR: simulated bad config*") + result.stderr.no_fnmatch_line("*INTERNALERROR*") + result.stderr.no_fnmatch_line("*KeyError*") + + @pytest.mark.filterwarnings("error::pytest.PytestUnraisableExceptionWarning") def test_possibly_none_excinfo(pytester: Pytester) -> None: pytester.makepyfile(