Skip to content

refactor(tests): split conftest.py under the 300-line cap (boy-scout follow-up) - #310

Open
cdeust wants to merge 1 commit into
mainfrom
split-conftest-size-cap-boyscout
Open

refactor(tests): split conftest.py under the 300-line cap (boy-scout follow-up)#310
cdeust wants to merge 1 commit into
mainfrom
split-conftest-size-cap-boyscout

Conversation

@cdeust

@cdeust cdeust commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Summary

tests_py/conftest.py was 501 lines — over both the repo's local 300-line
cap and the global 500-line cap (coding-standards.md §4.1) — discovered
while fixing #287, whose own PR (#308) touches this exact file (a small,
additive-only guard-wiring change). Per §14 (Boy-scout), a defect seen in
touched material must be fixed in the same PR or deferred to its OWN PR
with that PR cited in the report — this is that PR, cited in #308's Boy-scout
section.

What changed — behavior-preserving split

Split along the file's own pre-existing section-comment boundaries into
three new modules, zero behavior change: closures over conftest.py's
module globals became explicit-parameter functions (an incidental DI
improvement, not the goal — these are independently unit-testable now,
which the file's own prior state was not).

New module What moved there
tests_py/_pg_throwaway_db.py Per-process throwaway PG database lifecycle: create_isolated_test_database, drop_isolated_database, drop_dead_orphaned_databases, maintenance_url
tests_py/_pg_safety_guards.py The two fail-closed data-safety guards: guard_against_populated_db (anti-537k-row-deletion, incident 2026-06-10) and guard_against_real_data_roots (issue #219), plus looks_like_test_db and _resolved_real_data_roots
tests_py/_domain_mapping_isolation.py The dev-root filesystem-scan isolation from issue #196

conftest.py drops from 501 to 281 lines. The pytest hooks
(pytest_sessionfinish, the autouse _test_isolation fixture) and the
module-level bootstrap sequence stay in place — pytest hooks/fixtures must
be defined in a conftest.py itself to be auto-discovered — now delegating
to the extracted functions in the exact same order as before.

A real regression this split would have caused, and how it was avoided

Two existing regression-guard test files hard-coded the guard's prior
location/signature — one scanned conftest.py's raw source text and
exec'd the extracted lines into a hand-built namespace; the other called
a zero-arg closure directly — and both broke outright on the move
(StopIteration / AssertionError on a stale structural check). Fixed to
import the real, now-standalone modules directly, which also let them
drop the old exec-from-source-text workaround entirely (that workaround
existed only because the functions used to be closures inside a file that
ALSO did heavy bootstrapping at import time; the extracted modules import
only os/pytest, so a direct import is safe).

Those fixes, plus the new unit tests written for the newly-extracted
functions (previously exercised only implicitly by the full test-session
bootstrap), then pushed both of those test files over the same
300-line cap (test_conftest_guard.py: 496 lines pre-existing on
origin/main; test_store_isolation.py: 350 pre-existing) — rather than
leave a second, freshly-introduced-by-me violation, each was split along
its natural per-function boundary:

  • test_conftest_guard.pytest_looks_like_test_db.py +
    test_guard_against_populated_db.py + test_guard_blocks_populated_db.py
    (the anti-537k branch, kept separate as the single most safety-critical
    piece) + test_guard_against_populated_db_structural.py
  • test_store_isolation.pytest_store_isolation.py (trimmed) +
    test_real_data_root_guard.py

Completion Ledger

Path / requirement Evidence
_pg_throwaway_db.maintenance_url — basic + query-string-with-embedded-slash edge case test_pg_throwaway_db.py::TestMaintenanceUrl (3 tests)
_pg_throwaway_db._orphan_pid — well-formed parse, non-numeric suffix, domain-scoped split/rsplit/maxsplit equivalence documented test_pg_throwaway_db.py::TestOrphanPid (2 tests + in-source equivalence doc)
_pg_throwaway_db._pid_is_definitely_dead — dead/alive/permission-denied, exact os.kill call args (mocked — see safety note below) test_pg_throwaway_db.py::TestPidIsDefinitelyDead (3 tests)
_pg_throwaway_db.drop_dead_orphaned_databases — exact SELECT SQL, dict-row vs tuple-row, dead-pid drop with exact terminate+drop SQL/args, alive-pid no-op, continue-not-break on both guard clauses, selection-failure swallowed test_pg_throwaway_db.py::TestDropDeadOrphanedDatabases (7 tests)
_pg_throwaway_db.create_isolated_test_database / drop_isolated_database — unreachable-server None, exact connect args, orphan-reap on the same connection, db-name format regex, exact CREATE/TERMINATE/DROP SQL test_pg_throwaway_db.py::TestCreateAndDropIsolatedDatabase (7 tests)
_pg_safety_guards.looks_like_test_db — basic true/false, query-string stripped, host-vs-name trap (kills the rsplit-vs-split mutation), double-? edge case, no-slash edge case, remaining split/rsplit/maxsplit survivors documented equivalent test_looks_like_test_db.py (6 tests)
_pg_safety_guards.guard_against_populated_db — override bypass, url-name trust, PG-unreachable, empty-DB, exact connect/execute args test_guard_against_populated_db.py (8 tests)
_pg_safety_guards.guard_against_populated_db — the anti-537k block branch, exact exit message test_guard_blocks_populated_db.py (5 tests)
_pg_safety_guards.guard_against_populated_db — structural (definition present, module-level call, returncode=2, incident date) test_guard_against_populated_db_structural.py (4 tests)
_pg_safety_guards._resolved_real_data_roots — exact 4-tuple list test_store_isolation.py::TestResolvedRealDataRoots (1 test)
_pg_safety_guards.guard_against_real_data_roots — fires/silent, exact message with 2+ offenders (exercises the join separator), no-bypass-env-var test_real_data_root_guard.py::TestIsolationGuard (4 tests)
_pg_safety_guards.guard_against_real_data_roots — structural (redirect + guard call present at module level, ordering before first mcp_server import) test_real_data_root_guard.py::TestGuardStructuralIntegrity (3 tests)
_domain_mapping_isolation.isolate_dev_root_scan — stub replacement, cache cleared test_domain_mapping_isolation.py (2 tests)
conftest.py's wiring itself (imports + calls survive) test_guard_against_populated_db_structural.py, test_real_data_root_guard.py::TestGuardStructuralIntegrity

59 tests total in the touched/new files; full local suite 6854 passed, 5 skipped, 116 subtests passed (up from the pre-split 6826 baseline collected on this same origin/main commit — the +33 delta is exactly the new coverage this PR adds for previously-untested extracted code).

Gate output (quoted, run against this exact head)

§4 AST size check — every touched/new file, clean:

tests_py/conftest.py: 281 lines
tests_py/invariants/test_store_isolation.py: 241 lines
tests_py/_domain_mapping_isolation.py: 61 lines
tests_py/_pg_safety_guards.py: 125 lines
tests_py/_pg_throwaway_db.py: 146 lines
tests_py/invariants/test_domain_mapping_isolation.py: 42 lines
tests_py/invariants/test_guard_against_populated_db.py: 270 lines
tests_py/invariants/test_guard_against_populated_db_structural.py: 95 lines
tests_py/invariants/test_guard_blocks_populated_db.py: 160 lines
tests_py/invariants/test_looks_like_test_db.py: 65 lines
tests_py/invariants/test_pg_throwaway_db.py: 289 lines
tests_py/invariants/test_real_data_root_guard.py: 213 lines
(no FILE/FN OVER CAP lines printed = clean, including no function over 40 lines)

ruff:

$ ruff check tests_py/
All checks passed!
$ ruff format --check tests_py/
478 files already formatted

Doc claims / badges (unaffected — issue #294 already removed the
suite-size claim from prose/JSON docs):

$ python scripts/check_doc_claims.py --test-count 6859
doc claims OK (1 declared not-a-claim exemption(s))
$ python scripts/generate_repo_badges.py --check --test-count 6859
badges OK (5 checked)

§12 Mutation testing (scoped run via a hand-adapted mutation_check.sh
invocation, CORTEX_TEST_ISOLATE_DB=0 to keep the live per-process
throwaway-DB bootstrap itself out of the mutated call graph — mutating a
function pytest_sessionfinish calls conflicts with mutmut's own
verification trampoline otherwise):

211 mutants generated, 204 killed, 7 survived (0 suspicious after fixes)

All 7 survivors are provable-equivalent mutants, documented at each use
site:

  • looks_like_test_db (4): split/rsplit/maxsplit choices — _PIN_RE-style
    reasoning generalizes here too ([0] from split(sep,N) and [-1] from
    rsplit(sep,N) are invariant to N); verified by a 300k-case property fuzz
    against arbitrary strings (structural equivalence, not domain-scoped).
  • _orphan_pid (3): split/rsplit/maxsplit choices, but this one IS
    domain-scoped — equivalent only because every name the function is ever
    called against (cortex_test_pw<pid>_<8-hex>) has exactly one "" in the
    parsed segment (the hex suffix's alphabet excludes "
    "); verified by a
    200k-case fuzz over the REAL format (an unrestricted fuzz does NOT agree,
    confirming this is domain-scoped, not structural).

One additional finding along the way: a mutation to _pid_is_definitely_dead's
hardcoded signal number (os.kill(pid, 0)os.kill(pid, 1)/SIGHUP) was
initially exercised by an UNMOCKED real-syscall test
(_pid_is_definitely_dead(os.getpid())), which under that mutant would send
a real SIGHUP to the pytest process running the mutation sub-run — mutmut
flagged it "suspicious" rather than cleanly killed. Removed that unmocked
test (redundant with the mocked, exact-call-args test that already covers
the same contract) rather than leave a test whose mutation-testing behavior
is a real safety hazard.

No conflict markers; .bestpractices.json parses — confirmed.

Boy-scout check (coding-standards.md §14)

Rules compliance (coding-standards.md, evaluated against the version

quoted in this session's system prompt)

Rule Status Evidence
§1 SOLID (SRP) pass Each extracted module has one cohesive concern (throwaway-DB lifecycle / data-safety guards / filesystem-scan isolation); each split test file targets one function or one branch of one function
§4 Size limits pass AST-measured above, every file and function
§5 DI pass Extracted functions take explicit parameters instead of closing over conftest.py's module globals
§7 Local reasoning pass No new globals/monkey-patching/reflection
§8 Source discipline pass No invented constants; every SQL/message string traces to the code being extracted verbatim
§9 Anti-patterns pass No dead code left behind; unused _CONFTEST_PATH/_GUARDS_PATH constants removed from test_store_isolation.py once their sole consumers moved out
§12 Mutation testing pass 204/211 killed; 7 documented equivalent (see above)
§13 Completion Ledger pass table above
§14 Boy-scout pass this PR IS the boy-scout fix; see above

Hand-offs

None.

Not tied to a tracked GitHub issue by itself — this is the boy-scout
follow-up PR cited from PR #308 (issue #287), per this session's
"no new issues, cite the PR instead" instruction.

… issue #276/#287)

tests_py/conftest.py was 501 lines — over both the repo's local 300-line
cap and the global 500-line cap (coding-standards.md §4.1) — discovered
while fixing #287 (a small, additive-only touch there would have left the
pre-existing violation unaddressed and unfiled, which §14 forbids without
a citing PR; this is that PR).

Split along the file's own pre-existing section-comment boundaries, into
three new modules with zero behavior change — closures over conftest.py's
module globals became explicit-parameter functions (an incidental DI
improvement, not the goal):

- tests_py/_pg_throwaway_db.py — per-process throwaway PG database
  creation/cleanup (create_isolated_test_database, drop_isolated_database,
  drop_dead_orphaned_databases, maintenance_url).
- tests_py/_pg_safety_guards.py — the two fail-closed data-safety guards
  (guard_against_populated_db, the anti-537k-row-deletion guard from
  incident 2026-06-10; guard_against_real_data_roots, from issue #219) plus
  looks_like_test_db and _resolved_real_data_roots.
- tests_py/_domain_mapping_isolation.py — the dev-root filesystem-scan
  isolation from issue #196.

conftest.py itself drops from 501 to 281 lines: the pytest hooks
(pytest_sessionfinish, the autouse _test_isolation fixture) and the
module-level bootstrap sequence stay in place, now delegating to the
extracted functions in the exact same order.

Two existing regression-guard test files hard-coded the guard's prior
location/signature (scanning conftest.py's raw source, or calling a
zero-arg closure) and broke on the move — fixed to import the real,
now-standalone modules directly, which also let them drop the old
exec-from-source-text workaround entirely. Both, plus the new tests added
for the extracted modules, then pushed test_conftest_guard.py (496 lines
pre-existing) and test_store_isolation.py (350 pre-existing) over the
same cap; split each along its natural per-function boundary rather than
leave a second freshly-introduced violation:
  test_conftest_guard.py -> test_looks_like_test_db.py +
    test_guard_against_populated_db.py + test_guard_blocks_populated_db.py +
    test_guard_against_populated_db_structural.py
  test_store_isolation.py -> test_store_isolation.py (trimmed) +
    test_real_data_root_guard.py

Mutation-tested (scoped, CORTEX_TEST_ISOLATE_DB=0 to keep the per-process
throwaway-DB bootstrap itself out of the mutated call graph): 204/211
mutants killed; 7 survivors are provable-equivalent split/rsplit/maxsplit
choices (documented at each use site, verified by construction and by
property fuzzing — one is domain-scoped to the exact
cortex_test_pw<pid>_<hex> format the function is only ever called
against, not a general claim).

Full local suite: 6854 passed, 5 skipped, 116 subtests passed — no
regressions.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012Yu6EnWspTfqHoGkExyS6u
@cdeust
cdeust force-pushed the split-conftest-size-cap-boyscout branch from 8614744 to 6249f58 Compare July 30, 2026 20:16
@cdeust

cdeust commented Jul 30, 2026

Copy link
Copy Markdown
Owner Author

Rebased onto current main (e275362)

Rebased split-conftest-size-cap-boyscout from its old base onto
origin/main at e275362 (post #308/#309/#311). Force-pushed with
--force-with-lease; new head 6249f58.

Conflict — tests_py/conftest.py, one hunk

Sibling PR #308 (fix(tests): guard local venv from silently under-collecting vs. CI (#287), merged before this rebase) added a new
inline guard, _guard_against_venv_lock_drift(), and a call to it right
next to this PR's own call-site edit (replacing the two zero-arg guard
calls with the module-based _pg_safety_guards.guard_against_populated_db(_TEST_DB_URL)
/ guard_against_real_data_roots(_TEST_CLAUDE_DIR)). Git flagged the
3-line call-site block as conflicting because both sides touched
adjacent/overlapping lines.

Resolution — kept both sides' assertions, nothing dropped:

_pg_safety_guards.guard_against_populated_db(_TEST_DB_URL)
_pg_safety_guards.guard_against_real_data_roots(_TEST_CLAUDE_DIR)
_guard_against_venv_lock_drift()
_USE_PG = _pg_available()

No other file in this PR's diff conflicted or required editing — verified
every non-conftest.py file in HEAD is byte-identical to the original
PR commit (8614744) via git diff 8614744:<file> HEAD:<file> on all 12
other touched files (all empty diffs). tests_py/invariants/test_conftest_guard.py
(the file this PR deletes) was untouched by any commit on main since
this PR's original base — confirmed no sibling PR modified it, so the
wholesale replacement loses no content.

Re-verification against the rebased head

CI — every check green per-job on 6249f58

gh pr checks 310 — 18/18 required + informational checks pass
(Fuzz (scheduled batch) reports skipping, which is its designed
behavior on a PR — it only runs on the cron schedule, not per-PR):
Analyze (actions/js-ts/python) ×3, Build Package, CodeQL, Docker Build
(runtime + devcontainer), Docker Smoke, Fuzz (PR batch) ×2, Lint, Test
(Python 3.10/3.11/3.12/3.13), Test (SQLite backend), Test (Windows,
SQLite backend), Type Check. mergeStateStatus: CLEAN, mergeable: MERGEABLE.

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.

1 participant