WIP: Surface filter-promoted unraisable warnings directly (#14263)#14499
Draft
paulzuradzki wants to merge 4 commits into
Draft
WIP: Surface filter-promoted unraisable warnings directly (#14263)#14499paulzuradzki wants to merge 4 commits into
paulzuradzki wants to merge 4 commits into
Conversation
d7db869 to
476e4f5
Compare
filterwarnings = error::ResourceWarning does not fail tests that leak resources through a reference cycle. collect_unraisable wraps the captured ResourceWarning in PytestUnraisableExceptionWarning, a class the user has no filter for, so the run exits 0. This test pins that contract: on a refcycle-leaking test with error::ResourceWarning configured, pytest should exit non-zero and the output should show the inner ResourceWarning rather than the wrapping PytestUnraisableExceptionWarning. Fails at this commit; the next commit ships the fix that turns it green. Refs pytest-dev#14263.
When sys.unraisablehook captures a Warning subclass instance and the user has an active ``error::<that class>`` filter, raise the original warning rather than wrapping in PytestUnraisableExceptionWarning. The wrap path remains for any case where no matching error filter is set, so suites that don't use ``error::<warning>`` filters see no change. Filter matching is approximate: category only, not message/module/lineno. The check errs toward false negatives, never false positives. The regression test added in the previous commit now passes. Additional coverage: - test_refcycle_userwarning_filter: locks the contract for a non-builtin Warning subclass. - test_unraisable_warning_without_filter_still_wraps: scope guard. A Warning raised from __del__ without a matching error filter must still be wrapped, not raised directly. - test_unraisable_warning_filter_add_note_dedups: covers the duplicate- note guard in the unwrap path for singleton/cached Warning instances. Tightens the ``errors`` list type from list[Exception] to list[Warning | RuntimeError]. Adds Paul Zuradzki to AUTHORS. Notes in test_create_task_raises_unraisable_warning_filter that the propagated class is now bare RuntimeWarning rather than the wrapping PytestUnraisableExceptionWarning (because -Werror activates the new unwrap path). Closes pytest-dev#14263.
476e4f5 to
4cf46d4
Compare
Register only the hook-restore + stash-cleanup as the config.add_cleanup callback. Move the GC pump and collect_unraisable call into a new pytest_unconfigure(config) hook. pytest_unconfigure fires before _cleanup_stack.close(), so warning filters managed via the cleanup stack (the warnings plugin's catch_warnings context, in particular) are guaranteed active when GC runs. This decouples the unraisable step from plugin registration order in default_plugins. The previous arrangement worked only because of LIFO ordering on _cleanup_stack; pytest-dev#13057 (Dec 2024) reordered default_plugins to make that ordering correct, but the structural fragility remained. No observable behavior change. The 141 existing tests in test_unraisableexception + test_warnings + test_recwarn + test_threadexception still pass. The previous commit's test_refcycle_resource_warning_filter continues to fail on main and pass here. This is the structural side of the issue's two proposed fixes; the user-visible side shipped in the previous commit.
FuzzysTodd
approved these changes
May 20, 2026
FuzzysTodd
left a comment
There was a problem hiding this comment.
pytest_unconfigure fires before _cleanup_stack.close(), so warning
filters managed via the cleanup stack (the warnings plugin's
catch_warnings context, in particular) are guaranteed active when GC
When another plugin's pytest_configure raises (e.g. pytest.UsageError in testing/acceptance_test.py::test_config_error), pluggy skips remaining configure hooks. unraisableexception.pytest_configure never runs, config.stash[unraisable_exceptions] is never set. The previous config.add_cleanup callback wasn't registered in that case either, so cleanup was a no-op. The pytest_unconfigure hook introduced in the previous commit ran unconditionally and hit KeyError on the unset stash, surfacing as INTERNAL_ERROR where pytest should exit with USAGE_ERROR. Guard with a stash-presence check at the top of pytest_unconfigure. test_config_error catches the regression direction.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #14263.
filterwarnings = error::ResourceWarningdoes not fail tests that leak resources through a reference cycle.collect_unraisablewraps the captured warning inPytestUnraisableExceptionWarning, a class the user has not filtered.collect_unraisablenow consultswarnings.filtersbefore wrapping. If an activeerrorfilter matches the inner warning's class, it raises the original warning. Theerror::<warning>filter then fails the test.Scope
The unwrap fires only when an active
errorfilter matches the inner warning's class. A__del__that raises aWarningwithout a matching filter still triggers the wrap path. Suites withouterror::<warning>filters see no change.Filter matching is approximate: category only, not
message/module/lineno. The check errs toward false negatives, never false positives.Relationship to #14273
#14273 attempted approach 2 from the issue (move GC to
pytest_unconfigure). Maintainers closed it unmerged: #13057 (Dec 2024) had already shipped approach 1, so the move-GC change alone produced no observable behavior difference and no fails-then-passes regression test was possible. @Zac-HD asked: "we'd need to see a regression test which demonstrates that the unraisable hook is now subject to warnings."This branch ships that test by addressing a different symptom: the wrap class hides the user's filter. The third commit lands the move-GC hardening from #14273. That commit is a structural refactor with no observable behavior change today; it decouples the GC step from
default_pluginsregistration order, so the timing invariant no longer depends on LIFO_cleanup_stackordering. The regression test stays green either way.Manual testing
Reproducer: my adapted version of the original gist from the issue. The original references an undefined
stderr_linesin itsrun_scenariohelper; the adapted version applies a one-line fix.For a faster check without scaffolding files, write a refcycle-leak test and run
pytest -W error::ResourceWarning test_leak.py. The-Wflag drives the same code path as thepytest.inifilterwarningssetting.main. Check outmain, install (pip install -e .), run the adapted reproducer. Scenario 1 exits 0 (bug confirmed). Scenario 2 exits 1 (the gist's conftest workaround using approach 2 from Session-endgc.collect()inunraisableexceptionplugin runs after warning filters are torn down, silently losingResourceWarnings #14263).ResourceWarningraised. Scenario 2 still exits 1.Warningfrom__del__without a matchingerrorfilter against this branch. The wrap path still fires; finalizers that pass today keep passing.error::DeprecationWarningorerror::UserWarningfilters against this branch. Nothing previously-passing should now fail.AI usage
Yes. I'm new to repo. Taking some time to review changes more slowly and manually testing for learning.
Changelog
changelog/14263.bugfix.rst.