Skip to content

test(checks): fix cross-test pollution in test_checks.py (#1741)#1743

Merged
johnrtipton merged 2 commits into
mainfrom
fix/test-checks-pollution-1741
Jun 6, 2026
Merged

test(checks): fix cross-test pollution in test_checks.py (#1741)#1743
johnrtipton merged 2 commits into
mainfrom
fix/test-checks-pollution-1741

Conversation

@johnrtipton

Copy link
Copy Markdown
Contributor

Summary

Fixes #1741 — two tests in python/tests/test_checks.py failed deterministically under cross-test ordering but passed in isolation:

  • TestC003DaphneOrdering::test_c003_daphne_missing_info
  • TestSuppressChecks::test_no_suppress_by_default

Both failed with a polluted C003 assertion (0 == 1 — C003 did not fire despite all firing conditions being met).

Proven polluter + mutated global state

Polluter: python/djust/tests/test_dev_server_watchdog_missing.py — specifically the block_watchdog fixture, exercised by test_check_hot_view_replacement_survives_without_watchdog, which evicts and re-imports djust.checks (and djust.dev_server) to test the no-watchdog import path (#994).

Mutated state (proven, not guessed): a re-import via importlib.import_module("djust.checks") rebinds both sys.modules["djust.checks"] and the parent-package attribute djust.checks. The fixture restored only sys.modules, leaving the two pointing at different module objects.

Diagnostic probe in the failing order showed:

globals_is_module = False      # check_configuration.__globals__ is NOT vars(checks-imported-by-test)
sysmod_is_checks  = False      # sys.modules["djust.checks"] != (from djust import checks)
pkg_attr_is_localimport = True # djust.checks (pkg attr) == (from djust import checks)  -> module A
cc_from_sysmod    = True       # sys.modules["djust.checks"].check_configuration is check_configuration -> module B
globals_has_asgi_result = True # the function under test sees the REAL (unpatched) _has_asgi_server

So the failing test did:

  • from djust import checks → resolves the package attribute → module A (which the fixture restored)
  • from djust.checks import check_configuration → resolves sys.modules → module B (the stale re-imported copy)

monkeypatch.setattr(checks, "_has_asgi_server", lambda: False) patched A, but check_configuration's globals belonged to B, so the patch silently no-op'd → the real _has_asgi_server() returned True → C003 was suppressed → len(c003) == 0.

Reproduced failing order

.venv/bin/python -m pytest python/djust/tests/ python/tests/test_checks.py -p no:randomly -q
# -> 2 failed (the two cited tests), 3294 passed

(Also surfaces intermittently in CI shards / xdist, per #1736/#1739 Stage-11 reviewers.)

Isolation fix

In block_watchdog (function-scoped fixture): snapshot and restore the parent-package attribute (djust.checks, djust.dev_server) alongside sys.modules, so the two stay consistent after teardown and from djust import checks / from djust.checks import X resolve the same object. Test-only change; no runtime behavior change.

Module-cache / registry audit (recurring class)

  • djust check registry (@register("djust") into Django's global registry): the actual tech-debt: fix 2 pre-existing test-pollution failures in test_checks.py + audit module-level caches for test-reset #1741 cause was the module re-import desyncing the registered-check module copies — fixed by the package-attr restore above. ✓
  • _route_map_cache (routing.py:27, has _reset_route_map_cache()): already wired into autouse reset fixtures in both tests that mutate it — test_route_map_derivation.py (autouse _reset_state) and test_client_config_tag.py (autouse _reset_route_map). No gap. ✓
  • checks.py module-globals: all compiled regexes / frozensets (immutable; not mutated by tests). No reset needed. ✓
  • No second polluter found.

Verification

Gate-off (#254): reverted the fix (git stash) → the exact 2 tests fail again; restored → pass. The fix is confirmed to be what resolves the failure.

3-clean-runs gate (pollution-class, mandatory):

  • test_checks.py 3× consecutive → 204 passed each. ✓
  • Cross-dir order python/djust/tests/ python/tests/ 3× consecutive → 5471 passed, 9 skipped each. ✓
  • xdist -n auto cross-dir → 5471 passed, 9 skipped. ✓
  • Exact previously-failing order python/djust/tests/ python/tests/test_checks.py3296 passed. ✓

Closes #1741

🤖 Generated with Claude Code

johnrtipton and others added 2 commits June 6, 2026 17:30
…ttr on module restore (#1741)

The block_watchdog fixture in test_dev_server_watchdog_missing.py evicts
and re-imports djust.checks / djust.dev_server to exercise the no-watchdog
import path (#994). A re-import rebinds BOTH sys.modules["djust.checks"]
AND the parent-package attribute djust.checks. The fixture restored only
sys.modules, leaving the two pointing at different module objects.

A later cross-dir test (python/tests/test_checks.py) then did
`from djust import checks` (resolves the package attribute -> module A)
and `from djust.checks import check_configuration` (resolves sys.modules
-> module B). monkeypatch.setattr(checks, "_has_asgi_server", ...) patched
A, but check_configuration's globals belonged to B, so the patch silently
no-op'd and C003 deterministically failed (0 == 1) under the cited order:
  pytest python/djust/tests/ python/tests/test_checks.py

Fix: snapshot + restore the parent-package attribute alongside sys.modules
so the two stay consistent after teardown.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@johnrtipton

Copy link
Copy Markdown
Contributor Author

Stage 11 Adversarial Review — PR #1743 (fix #1741 cross-test pollution)

Verdict: 🟢 APPROVE. Test-only fix, root cause correctly diagnosed, independently reproduced pre-fix / confirmed post-fix, 9/9 clean pollution-gate runs, restore is complete and order-correct, no second/latent polluter found.


Mandatory pre-review checks

Load-bearing empirical reproduction (ran independently)

  • 🟢 Pre-fix FAILS (cited order, origin/main test file): swapped in the origin/main version of the fixture file, ran pytest python/djust/tests/ python/tests/test_checks.py -p no:randomly -q
    2 failed, 3294 passed — the exact two cited tests:
    TestC003DaphneOrdering::test_c003_daphne_missing_info + TestSuppressChecks::test_no_suppress_by_default.
  • 🟢 Post-fix PASSES (same order, PR HEAD): 3296 passed, 3 skipped, 0 failed. The 2 previously-failing now pass.

3-clean-runs pollution gate (re-ran myself, did not trust the report)

  • 🟢 python/tests/test_checks.py ×3: 204 passed / 204 passed / 204 passed.
  • 🟢 cross-dir python/djust/tests/ python/tests/ (serial, pytest-randomly → 3 different seeds) ×3: 5471 passed, 9 skipped each.
  • 🟢 cross-dir -n auto (xdist, mimics CI shard) ×3: 5471 passed, 9 skipped each.
  • 9 clean runs total, zero failures under every ordering. No second polluter trips.

Second-/latent-polluter probe

  • Grepped the whole test suite for importlib.reload / import_module / del sys.modules / sys.modules[...] writes.
  • Only two other tests use the dangerous evict+reimport pattern: tests/unit/test_hot_view_replacement.py and tests/integration/test_hvr_flow.py. Both evict/reimport temporary top-level modules (hvr_fixture_*), which are not attributes of the djust package, so they cannot create the package-attr/sys.modules split. They clean up with sys.modules.pop (correct for top-level modules). Not same-class.
  • tests/unit/test_ws_compression_config.py uses importlib.reload(djust.config)reload mutates the same module object in place (no new object, no split). Safe.
  • test_theming_register_get_roundtrip_invariant.py uses import_module on already-loaded modules (cache hit, no eviction). Safe.
  • No latent same-class polluter found. 🟢

Restore correctness (#1741 invariant)

  • 🟢 Snapshot is taken in fixture setup, before yield (lines 79-82, before blocker/yield) → captures the original objects, not a post-reimport copy.
  • 🟢 Teardown restores BOTH sys.modules (lines 93-97) AND the parent-package attrs djust.checks + djust.dev_server (lines 101-106) to those original objects, with correct _MISSING/delattr handling for the attr-didn't-exist case.
  • 🟢 Because both paths restore the same original object, the post-teardown invariant sys.modules["djust.checks"] is djust.checks holds — the exact invariant the original fixture violated. Empirically corroborated: gating the fix off (origin/main file) reproduces the failure; restoring the fix clears it.

Module-cache audit completeness (#1741 "stop the class recurring")

  • 🟢 _route_map_cache claim verified: both python/djust/tests/test_client_config_tag.py:38 and python/djust/tests/test_route_map_derivation.py:33 have autouse _reset_route_map_cache fixtures (reset before+after each test). The 9 clean runs empirically confirm no residual route-map pollution.

CHANGELOG

No 🔴, no 🟡. Recommend merge.

@johnrtipton johnrtipton merged commit c92ad8e into main Jun 6, 2026
13 checks passed
@johnrtipton johnrtipton deleted the fix/test-checks-pollution-1741 branch June 6, 2026 21:54
@johnrtipton

Copy link
Copy Markdown
Contributor Author

Retrospective — PR #1743 (pipeline-drain v1.0.2-3)

Task: v1.0.2-3 — #1741 (Action Tracker #289): fix 2 deterministic test_checks.py pollution failures.

Quality: 5/5

Pollution proven by causation (not guessed), narrowest fix, 3-clean-runs gate honored, reviewer independently reproduced both directions.

What went well

  • Root cause proven via a diagnostic probe, not bisect-and-hope. The polluter was test_dev_server_watchdog_missing.py's block_watchdog fixture: re-importing djust.checks rebinds BOTH sys.modules["djust.checks"] and the parent-package attribute djust.checks, but the fixture restored only sys.modules → the two pointed at different module objects. A later monkeypatch.setattr(checks, "_has_asgi_server", ...) patched the package-attr copy while check_configuration's globals lived in the sys.modules copy → patch silently no-op'd → C003 not suppressed → the 2 tests fail. The probe showed globals_is_module / sysmod_is_checks flip with/without the polluter — causation, not correlation.
  • Narrowest robust fix: snapshot+restore the package attr alongside sys.modules in the one offending fixture; test-only, no runtime change. Re-establishes the sys.modules[x] is djust.<attr> invariant the original fixture violated.
  • 3-clean-runs gate (pollution-class) honored and re-verified by the reviewer: 9 clean runs (file ×3, cross-dir serial ×3 across seeds, xdist -n auto ×3). No second polluter.

What didn't

  • Nothing. 0🔴/0🟡.

Process finding

The module re-import / sys.modules desync is a distinct pollution sub-class from the usual "cache not reset": re-importing a SUBMODULE rebinds two references (sys.modules entry + parent-package attribute) and restoring only one leaves them split, so attribute patches land on the wrong object. Any fixture that evicts+reimports a djust.* submodule must restore BOTH. The reviewer's second-polluter probe confirmed the other evict+reimport tests operate on temp top-level modules (not djust.* package attrs), so they're immune — but the pattern is worth watching.

Verified

  • Reviewer independently reproduced pre-fix-fails (2 failed on main, cited order) / post-fix-passes (3296 passed); gate-off non-tautological. _route_map_cache autouse resets confirmed present; checks.py globals immutable. All CI green (both Python shards). Two-commit shape intact.

RETRO_COMPLETE

johnrtipton added a commit that referenced this pull request Jun 6, 2026
PR #1743 (#1741 test pollution) + PR #1744 (#1742 demo dogfood). Guard
hardening tracked in #1745.

Audit-bypass-reason: docs-only status update via pipeline-drain skill (no retro needed)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tech-debt: fix 2 pre-existing test-pollution failures in test_checks.py + audit module-level caches for test-reset

1 participant