Skip to content

Fix unittest.addModuleCleanup having no effect under pytest - #14973

Open
ShamikOfficial wants to merge 4 commits into
pytest-dev:mainfrom
ShamikOfficial:fix-14958-module-cleanups
Open

Fix unittest.addModuleCleanup having no effect under pytest#14973
ShamikOfficial wants to merge 4 commits into
pytest-dev:mainfrom
ShamikOfficial:fix-14958-module-cleanups

Conversation

@ShamikOfficial

@ShamikOfficial ShamikOfficial commented Sep 5, 2026

Copy link
Copy Markdown

closes #14958

Summary

unittest.addModuleCleanup / enterModuleContext had no effect under pytest because we wired setUpModule/tearDownModule but never ran module cleanups. A prior attempt (#14959) drained the process-global _module_cleanups list at each module boundary; that is only correct under unittest's contiguous scheduling and is unsafe when pytest interleaves or re-enters modules.

This follows the approach discussed on the issue:

  1. Mark-and-drain in the xunit module fixture (_register_setup_module_fixture): record len(unittest.case._module_cleanups) at setup, LIFO-drain down to that mark after tearDownModule (in a finally, and also when setUpModule fails). Cleanups registered during a module visit therefore stay attributed to that visit.
  2. Session-end backstop: import-time registrations sit below every module mark; they are drained via a session finalizer attached from pytest_runtest_setup once the session is on the SetupState stack.
  3. Multiple cleanup failures raise an ExceptionGroup, matching class-cleanup handling (stdlib keeps only the first).

Deliberate deviations from stdlib (also noted on the issue): import-time registrations run at session end rather than at an arbitrary first module boundary; package/__init__.py parity and unconditional fixture registration for modules without setUpModule/tearDownModule are left for follow-ups (session backstop already covers the no-setup case).

AI disclosure

Implementation was assisted by Cursor. I reviewed the approach against the issue discussion and the closed #14959 feedback, own the change, and will handle review feedback.

Test plan

  • New tests in testing/test_unittest.py covering import-time + setup-time cleanups, enterModuleContext, setup/teardown failure paths, cross-module isolation, ExceptionGroup on multiple cleanup failures, private-API contract, and modules without setUpModule
  • Related existing cleanup / setUpModule tests pass locally
  • CI on the PR

Wire mark-and-drain module cleanups into the xunit module fixture and add a session-end backstop for import-time registrations (closes pytest-dev#14958).

Co-authored-by: Cursor <cursoragent@cursor.com>
@psf-chronographer psf-chronographer Bot added the bot:chronographer:provided (automation) changelog entry is part of PR label Sep 5, 2026
@RonnyPfannschmidt

Copy link
Copy Markdown
Member

@claude investigate in detail the differences this creates compared to unittest as well as investigate the failure modes we would see when test reordering is in effect

Post the report as reply in the pr so we can decide what to document and what needs warnings/ errors

Prepare a table wirh suggestions/ tradeoffs to support that

CI was treating EncodingWarning from open()/read_text() as errors.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ShamikOfficial

Copy link
Copy Markdown
Author

Pushed a small fix for the CI failures. The new event-log tests were opening and reading files without an encoding argument, which raised EncodingWarning under the CI warning settings. They now pass encoding="utf-8".

Still happy to follow up on the unittest vs reordering tradeoffs once that report is in.

Co-authored-by: Cursor <cursoragent@cursor.com>
@RonnyPfannschmidt

Copy link
Copy Markdown
Member

please check the ci again

Co-authored-by: Cursor <cursoragent@cursor.com>
@ShamikOfficial

Copy link
Copy Markdown
Author

Checked CI again.

Status after the latest push

  • All build (...) jobs were already green on the previous head (including Windows).
  • Remaining red: pre-commit.ci mypy on _unittest_module_cleanups (list missing type args / Any return). Just pushed a typing fix for that.
  • codecov/patch is at ~98% (1 miss / 1 partial). Happy to cover those lines if you want them green too.

On the unittest vs pytest / reordering report you asked for (Claude did not post one, so here is a concise version):

Behavior vs stdlib unittest

Topic unittest this PR
When visit-scoped cleanups run After tearDownModule, or when setUpModule fails Same, via mark-and-drain in the xunit module fixture finally
Import-time addModuleCleanup Fired at the first module boundary that drains the global list (loader order) Left below every module mark; drained at session end
Process-global list Contiguous module schedule makes a full doModuleCleanups() at each boundary "work" Full drain at each boundary is unsafe under reordering/re-entry; we only drain down to a per-visit mark
Cleanup errors First exception only (doModuleCleanups) Visit drain: ExceptionGroup if several fail (matches class cleanups). Session backstop still uses public doModuleCleanups()
Modules with no setUpModule/tearDownModule Suite still hits module teardown machinery No xunit fixture, so visit mark-and-drain does not run; import-time cleanups still hit session backstop

Reordering / re-entry failure modes

Scenario Risk if we naively called doModuleCleanups() per module With mark-and-drain + session backstop
A then B (normal) OK OK
Collect A+B, run A only A's teardown would also run B's import-time cleanups A's visit drain only pops above its mark; B's leftovers wait for session end
A, B, A again (re-enter A) First A teardown can steal B's still-queued cleanups Each visit drains only what it registered during that visit
Package __init__ + inner module Inner teardown can eat package-level cleanups Still a known gap (called out on the issue); not handled here
Parallel / xdist Shared list is process-local only Same as today; no new cross-process story

Suggestions / tradeoffs

Option Pros Cons Proposal
A. Current PR (mark-and-drain + session backstop) Fixes the reported no-op; safe under reorder; small surface Import-time cleanups run later than unittest; docs must say so Keep
B. Full doModuleCleanups() at every module boundary Matches unittest on contiguous runs Wrong under reorder/re-entry (closed #14959) Reject
C. Also register fixture when only cleanups exist (no setUp/tearDown) Earlier drain for cleanup-only modules Extra autouse fixture cost; needs a separate change Follow-up
D. Package/__init__.py parity Closer to suite nesting Separate design; easy to get wrong Follow-up
E. Warn when import-time cleanups are deferred to session end Makes the deviation visible Noise for common patterns Optional docs/warning later
F. Document deviations only (changelog + unittest page) Low churn Users still surprised if they assume exact unittest timing Do with A

I can add a short docs note for (F) in a follow-up commit on this PR if you want that before merge.

@RonnyPfannschmidt

Copy link
Copy Markdown
Member

This comment was written and posted by an AI agent (Claude Fable 5.1 via Claude Code) on @RonnyPfannschmidt's instruction. He prompted the investigation (his @claude comment above reached an unrelated GitHub user, not a bot); the experiments, the repros and the text are the agent's work and he has not edited them.


Comparison of PR head 084c895 against python -m unittest on Python 3.14.3, using the same modules under both runners with an event log. Modules a/b each register a cleanup at import time, in setUpModule, in setUpClass and inside test_1. Scenario labels below refer to those runs.

1. Behaviour differences vs. unittest

Situation python -m unittest This PR Verdict
Cleanups registered during a module visit (setUpModule, setUpClass, test body), module has setUpModule/tearDownModule after tearDownModule, LIFO same match
Re-entered module (a::t1 b::t1 a::t2, i.e. any cross-module reordering) setUpModule runs per visit; visit-scoped cleanups drain per visit same match
setUpModule raises Exception / SkipTest cleanups drained same match
tearDownModule raises cleanups still drained same (finally) match
Several cleanups fail first exception only, rest swallowed ExceptionGroup deliberate deviation, consistent with class-cleanup path; document
Import-time registrations (addModuleCleanup/enterModuleContext at module top level) drained at the first module boundary reached, whoever owns them: in a two-module run, a's teardown ran b's import-time cleanup before b's tests drained at session end, after session-scoped fixture teardown deliberate deviation, arguably saner than upstream; document
Module with neither setUpModule nor tearDownModule still a module boundary: its cleanups run right after its last test no fixture is registered, so cleanups from its setUpClass and tests stay pending until session end and run after other modules' tests gap, see §2
Failure in an import-time cleanup reported as a tearDownModule error, FAILED (errors=1) exception escapes pytest_sessionfinish: raw traceback out of the console entry point, no summary line, exit code 1 defect
Nested in-process session (pytester.runpytest_inprocess, pytest.main()) n/a the inner session's backstop calls doModuleCleanups(), which drains the outer session's pending cleanups; the outer module's later mark-drain then finds len < mark and silently does nothing defect
--collect-only, or all tests deselected (exit 5) nothing runs (no module boundary is reached) import-time cleanups run at session end acceptable: the matching enterModuleContext enter already ran at import; document
xdist n/a every worker imports every module, so every worker runs every module's import-time cleanups at its own session end, including modules whose tests it never ran (verified with -n 2 and --dist loadfile) same pairing argument as above; document
Package __init__.py setUpModule registering cleanups n/a registered via Package.setup, below every module mark, so they run at session end after unrelated modules, not after the package's tearDownModule known follow-up; document until then
Cleanup registered by another autouse module-scoped fixture (e.g. conftest) that happens to run before the xunit fixture n/a sits below the mark, runs at session end edge case; document "register from setUpModule/tests"
Output printed by session-end cleanups n/a not captured, appears raw in the terminal after the progress line cosmetic

Repro for the nested-session defect (this is exactly the shape of a plugin test suite, and of pytest's own testing/test_unittest.py):

import unittest, unittest.case as uc
pytest_plugins = ["pytester"]

def setUpModule():
    unittest.addModuleCleanup(print, "outer cleanup")

def test_inner_run(pytester):
    print("pending before:", len(uc._module_cleanups))   # 1
    pytester.makepyfile(test_inner="import unittest\nclass T(unittest.TestCase):\n    def test(self): pass")
    pytester.runpytest_inprocess().assert_outcomes(passed=1)  # prints "outer cleanup" here
    print("pending after:", len(uc._module_cleanups))    # 0

2. Failure modes under test reordering

Reordering here means anything that breaks unittest's "all tests of a module are contiguous" invariant: explicit node ids, pytest-randomly / pytest-random-order, --lf/--ff, xdist scheduling.

  1. Modules with setUpModule/tearDownModule: no new failure mode. Mark-and-drain follows pytest's setup stack, so each visit gets its own setUpModule, tearDownModule and cleanup drain. This is also what unittest does when handed an interleaved list of ids, so it is the one scheme that survives reordering.

  2. Modules without setUpModule/tearDownModule leak across module boundaries. Their cleanups are only reached by the session-end backstop, so under reordering they run after an arbitrary set of other modules' tests. The dangerous case is a cleanup that undoes global state: enterModuleContext(mock.patch(...)) from setUpClass or a test in such a module leaves the patch active for every later module. Under unittest the patch would be undone at the module boundary. This is silent today.

  3. Stale marks. The mark is a list length. Anything that pops entries between mark and drain without going through the fixture makes the mark wrong: a nested in-process session (above), or user code calling doModuleCleanups() directly. The result is len < mark, which the drain treats as "nothing to do", so cleanups either already ran at the wrong time or are lost silently. There is no diagnostic for this.

  4. Import-time cleanups always run once per process at session end, regardless of order and regardless of whether the module's tests ran. That is stable under reordering and does not depend on which module happens to finish first (unittest's behaviour does).

  5. Session-end failure crashes the run rather than producing an error report (see table). Under -x, --maxfail, or a KeyboardInterrupt the backstop still runs and can still crash the exit path.

3. Suggestions and trade-offs

# Suggestion Fixes Cost / trade-off Assessment
1 Backstop drains only down to a mark taken at pytest_sessionstart, never doModuleCleanups() nested-session drain; stale marks caused by inner sessions trivial; the private-API dependency already exists required before merge
2 Report backstop failures as a teardown error instead of letting them escape pytest_sessionfinish: e.g. attach the drain as a finalizer on the session node from the first pytest_runtest_setup (which is what the PR description already says, but the code uses pytest_sessionfinish) raw-traceback crash; also gives proper capture of cleanup output uses SetupState; with no tests run the drain never happens, which matches unittest more closely but changes the --collect-only row above required before merge
3 Emit a warning when a drain finds len < mark makes stale-mark bugs visible instead of silent one warnings.warn; can only fire when something external tampered recommended, cheap
4 Register the xunit module fixture for modules that contain unittest.TestCase classes even without setUpModule/tearDownModule (lazily from UnitTestCase.collect, so pytest-style modules pay nothing) cross-module leak of setUpClass/test-time cleanups; the mock.patch hazard one more autouse module fixture per unittest module; fixture-closure cost is a known hotspot (#14877), needs a number attached recommended follow-up before release, or at minimum a documented limitation
5 Docs (unittest integration page) stating the deviations: import-time → session end, once per process (so once per xdist worker), per-visit semantics, ExceptionGroup, no-setUpModule limitation user expectations none required
6 Optional warning at import time when a module's import grows _module_cleanups ("import-time module cleanups run at session end under pytest") surprise about timing, runs at the one deterministic point (Module._importtestmodule) fires on a pattern the stdlib docs endorse and that the issue reporter uses; would need a filter/opt-out not by default; docs suffice
7 Package __init__.py parity: mark-and-drain in Package.setup package-level cleanups timing small, separate PR follow-up, as already planned
8 Fix PR description to match the code (finalizer vs pytest_sessionfinish) once #2 is decided reviewer confusion none before merge

4. CI status

  • pre-commit.ci fails on mypy only, reproduced locally with pre-commit run -a:
    src/_pytest/unittest.py:53: error: Missing type arguments for generic type "list"  [type-arg]
    src/_pytest/unittest.py:61: error: Returning Any from function declared to return "list[Any]"  [no-any-return]
    
    Give _unittest_module_cleanups a concrete return type (a list[tuple[Callable[..., object], tuple[object, ...], dict[str, object]]] alias) and cast the getattr result.
  • codecov/patch is at 94.2% (6 missed lines), almost certainly the case is None / not cleanups early returns in drain_remaining_module_cleanups, which suggestions 1 and 2 would remove anyway.
  • All build jobs, docs and the changelog check pass.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided (automation) changelog entry is part of PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

unittest.addModuleCleanup has no effect

2 participants