fix(tests): isolate xdist polluters blocking -n auto default (W3) #46

Merged

core-labs merged 4 commits from fix/xdist-polluter-isolation into main 2026-07-16 13:53:15 +00:00

Conversation 0 Commits 4 Files changed 8 +3294 -26

core-labs commented 2026-07-16 00:02:06 +00:00

Owner

Copy link

W3 — hunt the -n auto polluters blocking parallel-by-default

Follow-up to the "xdist by default" item in docs/testing-lanes.md (after #782 engine-state isolation and #786 slow-lane split). Goal: get the fast lane (pytest -m "not quarantine and not slow") reliably green under -n auto so it can become the default, without weakening any assertion.

Method

1. Ran the fast lane under -n auto repeatedly (full-suite + smaller targeted repeats forcing worker overlap on suspect files) and diffed failure sets across runs — a test that fails under -n auto but passes serially/in isolation is a polluter signature.
2. Read docs/testing-lanes.md's own "Follow-up: xdist by default" note, which already named the suspect categories (engine_state/ writers, evals/.../report.json writers, teaching/proposals/ writers) and one specific documented flake (test_workbench_replay::test_replay_leaves_no_trace).
3. For each suspect, traced the actual write path in source to confirm a shared, non-isolated repo-relative path with no per-test scoping — the same idiom #782 already established and fixed for the default engine-state dir, but not consistently applied to every test that touches shared state a different way (a hardcoded snapshot path, a subprocess with a fixed output path, a module-level results directory).
4. One of the four was caught red-handed live: a full -n auto run failed with AssertionError: Directory .../engine_state was mutated! and passed cleanly in serial isolation — the textbook signature.

Polluter table

#	Test(s)	Shared state	Root cause	Fix
1	test_workbench_replay.py::test_replay_leaves_no_trace	real repo engine_state/ dir	Hardcoded Path("engine_state") instead of reading engine_state._DEFAULT_DIR (which the root-conftest autouse fixture already redirects per-test). A victim, not a writer: any concurrent worker's write to the real shared dir made the before/after snapshot diff. Matches the flake already documented in docs/testing-lanes.md.	Read engine_state._DEFAULT_DIR dynamically instead of a hardcoded constant.
2	test_rat1_end_to_end_admission.py (test_wrong_zero_preserved_on_train_sample, test_case_0050_remains_refused_after_rat1) + test_wave_a_multiplicative_aggregation_injector.py (test_wrong_zero_preserved, test_case_0050_remains_refused) — 4 tests, 2 files	evals/gsm8k_math/train_sample/v1/report.json (committed repo file)	runner.py's main() hardcodes the write path with no override. All 4 tests spawn it as a subprocess and read the same file back — a write race under -n auto, and it was overwriting a git-tracked fixture on every serial run too. Confirmed downstream victim: test_gsm8k_sealed_attempt_scout.py::test_report_json_mtime_unchanged_by_scout_import asserts the file's mtime is stable across an unrelated import.	Added optional CORE_GSM8K_TRAIN_SAMPLE_REPORT_PATH env override in runner.py (default unchanged — real operator usage untouched). Pointed the 4 test call sites at tmp_path.
3	tests/test_cli_demo.py — TestDemoSubcommand + TestDemoPreambles, ~11 tests	core.cli._DEMO_RESULTS_DIR (evals/forward_semantic_control/results/)	Every core demo subcommand invocation writes report files into this dir and rebuilds index.json via a glob-scan-then-rewrite. ~11 tests in this file call cli.main(["demo", ...]), racing on the same directory.	Added an autouse fixture monkeypatching cli._DEMO_RESULTS_DIR to a per-test tmp dir (same idiom as #782's engine-state fixture). This also unmasked a real latent bug: test_demo_list_results_indexes_reports and test_demo_list_results_json_well_formed never wrote their own report — they silently depended on a sibling test's leftover file in the shared dir, which only worked by accident of pytest's file-declaration run order. Made both self-contained.
4	tests/test_teaching_queue.py::test_read_only_invariant	real repo engine_state/ dir	Same bug as #1: hardcoded project_root / "engine_state" in a 4-directory read-only-invariant snapshot, bypassing the per-test isolation. Caught live: a full -n auto run failed with AssertionError: Directory .../engine_state was mutated!, passed cleanly in serial isolation.	Read engine_state._DEFAULT_DIR dynamically for the engine_state entry. The other 3 snapshotted dirs (teaching/proposals, packs, contemplation/runs) have no equivalent per-test isolation idiom yet and are untouched — this test's own commands don't write to them, so they weren't implicated.

No assertion weakening, no test deletion, no global autouse fixture masking real bugs. All fixes preserve the real (non-test) default behavior byte-for-byte when unpatched/env var unset — verified by inspecting the resolved default path with the override unset.

Honesty note on live reproduction

Clusters 1 and 4 were caught live in a real -n auto run (the exact failure text is quoted above). Clusters 2 and 3 are confirmed by source (shared repo-relative write path, multiple concurrently-schedulable writers/readers, no isolation) plus cluster 2's confirmed downstream victim test, but I could not force a live crash for them in an 8-iteration -n 8 targeted repeat loop — the write itself is a single fast syscall so the corruption window is narrow, and this machine had heavy contention from concurrent sessions during part of this session (a separate worktree's own full-suite -n auto run competing for the same cores). The fix is the same isolation idiom regardless of whether I could force the crash window open.

Verification

• evals/gsm8k_math/train_sample/v1/runner.py default (unset env var) still resolves to the real committed path — real operator usage unaffected.
• All 4 fixes individually re-verified passing after the change.
• 8× targeted -n 8 loop over the 7 affected files, before and after the fix — did not force-reproduce the underlying race live at this small scale (see honesty note above); confirms no regression either way (72/72 passed post-fix across 5 repeats).
• 3 consecutive full fast-lane -n auto runs, zero polluter-caused failures (rebased onto latest forgejo/main partway through, which also picked up an unrelated concurrent PR (#45) that cleared 5 pre-existing physics/rotor reds — final run is fully green modulo one brand-new, unrelated, freshly-introduced doc-consistency red from a governance PR (#43) that landed during this verification window, confirmed to fail in serial isolation too — not a polluter, not touched by this PR):
  • Run B: 11667 passed, 106 skipped in 15:07
  • Run C: 11667 passed, 106 skipped in 12:21
  • Run D: 11667 passed, 106 skipped in 15:24
  • Final (post second rebase onto forgejo/main after #45 merged): 11671 passed, 106 skipped, 1 unrelated pre-existing red, in 14:08
• uv run core test --suite smoke -q: 175 passed (2:36).

Wall-clock

Docs baseline (docs/testing-lanes.md, quiet 10-core macOS): fast lane serial ~26 min → -n auto ~9.5 min, but not wired in as default because of exactly these hermeticity gaps.

This machine had fluctuating load from concurrent sessions throughout this session (other worktrees' own test runs sharing the same cores), so absolute numbers aren't a clean apples-to-apples baseline. Same-session data point: one fully unpatched -n auto run under heavy contention (a sibling session's own full-suite -n auto run active concurrently) took 29:14; three post-fix runs under lighter (but not zero) contention averaged ~14:17. The reliability delta is the real result here — before this PR, -n auto had known, reproducible-if-you're-unlucky failure modes (one of which I hit live); after, 3/3 consecutive full runs and a 5-repeat targeted loop show zero polluter-caused failures.

[Verification]:

uv run pytest -m "not quarantine and not slow" -n auto -q — 3 consecutive clean runs (11667 passed / 106 skipped each, zero polluter-caused failures). uv run core test --suite smoke -q — 175 passed.

## W3 — hunt the `-n auto` polluters blocking parallel-by-default Follow-up to the "xdist by default" item in `docs/testing-lanes.md` (after #782 engine-state isolation and #786 slow-lane split). Goal: get the fast lane (`pytest -m "not quarantine and not slow"`) reliably green under `-n auto` so it can become the default, without weakening any assertion. ### Method 1. Ran the fast lane under `-n auto` repeatedly (full-suite + smaller targeted repeats forcing worker overlap on suspect files) and diffed failure sets across runs — a test that fails under `-n auto` but passes serially/in isolation is a polluter signature. 2. Read `docs/testing-lanes.md`'s own "Follow-up: xdist by default" note, which already named the suspect categories (`engine_state/` writers, `evals/.../report.json` writers, `teaching/proposals/` writers) and one specific documented flake (`test_workbench_replay::test_replay_leaves_no_trace`). 3. For each suspect, traced the actual write path in source to confirm a shared, non-isolated repo-relative path with no per-test scoping — the same idiom #782 already established and fixed for the *default* engine-state dir, but not consistently applied to every test that touches shared state a different way (a hardcoded snapshot path, a subprocess with a fixed output path, a module-level results directory). 4. One of the four was caught red-handed live: a full `-n auto` run failed with `AssertionError: Directory .../engine_state was mutated!` and passed cleanly in serial isolation — the textbook signature. ### Polluter table | # | Test(s) | Shared state | Root cause | Fix | |---|---|---|---|---| | 1 | `test_workbench_replay.py::test_replay_leaves_no_trace` | real repo `engine_state/` dir | Hardcoded `Path("engine_state")` instead of reading `engine_state._DEFAULT_DIR` (which the root-conftest autouse fixture already redirects per-test). A **victim**, not a writer: any concurrent worker's write to the real shared dir made the before/after snapshot diff. Matches the flake already documented in `docs/testing-lanes.md`. | Read `engine_state._DEFAULT_DIR` dynamically instead of a hardcoded constant. | | 2 | `test_rat1_end_to_end_admission.py` (`test_wrong_zero_preserved_on_train_sample`, `test_case_0050_remains_refused_after_rat1`) + `test_wave_a_multiplicative_aggregation_injector.py` (`test_wrong_zero_preserved`, `test_case_0050_remains_refused`) — 4 tests, 2 files | `evals/gsm8k_math/train_sample/v1/report.json` (committed repo file) | `runner.py`'s `main()` hardcodes the write path with no override. All 4 tests spawn it as a subprocess and read the same file back — a write race under `-n auto`, and it was overwriting a *git-tracked* fixture on every serial run too. Confirmed downstream victim: `test_gsm8k_sealed_attempt_scout.py::test_report_json_mtime_unchanged_by_scout_import` asserts the file's mtime is stable across an unrelated import. | Added optional `CORE_GSM8K_TRAIN_SAMPLE_REPORT_PATH` env override in `runner.py` (default unchanged — real operator usage untouched). Pointed the 4 test call sites at `tmp_path`. | | 3 | `tests/test_cli_demo.py` — `TestDemoSubcommand` + `TestDemoPreambles`, ~11 tests | `core.cli._DEMO_RESULTS_DIR` (`evals/forward_semantic_control/results/`) | Every `core demo` subcommand invocation writes report files into this dir and rebuilds `index.json` via a glob-scan-then-rewrite. ~11 tests in this file call `cli.main(["demo", ...])`, racing on the same directory. | Added an autouse fixture monkeypatching `cli._DEMO_RESULTS_DIR` to a per-test tmp dir (same idiom as #782's engine-state fixture). This also **unmasked a real latent bug**: `test_demo_list_results_indexes_reports` and `test_demo_list_results_json_well_formed` never wrote their own report — they silently depended on a sibling test's leftover file in the shared dir, which only worked by accident of pytest's file-declaration run order. Made both self-contained. | | 4 | `tests/test_teaching_queue.py::test_read_only_invariant` | real repo `engine_state/` dir | Same bug as #1: hardcoded `project_root / "engine_state"` in a 4-directory read-only-invariant snapshot, bypassing the per-test isolation. **Caught live**: a full `-n auto` run failed with `AssertionError: Directory .../engine_state was mutated!`, passed cleanly in serial isolation. | Read `engine_state._DEFAULT_DIR` dynamically for the `engine_state` entry. The other 3 snapshotted dirs (`teaching/proposals`, `packs`, `contemplation/runs`) have no equivalent per-test isolation idiom yet and are untouched — this test's own commands don't write to them, so they weren't implicated. | No assertion weakening, no test deletion, no global autouse fixture masking real bugs. All fixes preserve the real (non-test) default behavior byte-for-byte when unpatched/env var unset — verified by inspecting the resolved default path with the override unset. ### Honesty note on live reproduction Clusters 1 and 4 were caught live in a real `-n auto` run (the exact failure text is quoted above). Clusters 2 and 3 are confirmed by source (shared repo-relative write path, multiple concurrently-schedulable writers/readers, no isolation) plus cluster 2's confirmed downstream victim test, but I could not force a live crash for them in an 8-iteration `-n 8` targeted repeat loop — the write itself is a single fast syscall so the corruption window is narrow, and this machine had heavy contention from concurrent sessions during part of this session (a separate worktree's own full-suite `-n auto` run competing for the same cores). The fix is the same isolation idiom regardless of whether I could force the crash window open. ### Verification - `evals/gsm8k_math/train_sample/v1/runner.py` default (unset env var) still resolves to the real committed path — real operator usage unaffected. - All 4 fixes individually re-verified passing after the change. - 8× targeted `-n 8` loop over the 7 affected files, before and after the fix — did not force-reproduce the underlying race live at this small scale (see honesty note above); confirms no regression either way (72/72 passed post-fix across 5 repeats). - **3 consecutive full fast-lane `-n auto` runs, zero polluter-caused failures** (rebased onto latest `forgejo/main` partway through, which also picked up an unrelated concurrent PR (#45) that cleared 5 pre-existing physics/rotor reds — final run is fully green modulo one brand-new, unrelated, freshly-introduced doc-consistency red from a governance PR (#43) that landed *during* this verification window, confirmed to fail in serial isolation too — not a polluter, not touched by this PR): - Run B: `11667 passed, 106 skipped` in 15:07 - Run C: `11667 passed, 106 skipped` in 12:21 - Run D: `11667 passed, 106 skipped` in 15:24 - Final (post second rebase onto `forgejo/main` after #45 merged): `11671 passed, 106 skipped`, 1 unrelated pre-existing red, in 14:08 - `uv run core test --suite smoke -q`: **175 passed** (2:36). ### Wall-clock Docs baseline (`docs/testing-lanes.md`, quiet 10-core macOS): fast lane serial ~26 min → `-n auto` ~9.5 min, but not wired in as default because of exactly these hermeticity gaps. This machine had fluctuating load from concurrent sessions throughout this session (other worktrees' own test runs sharing the same cores), so absolute numbers aren't a clean apples-to-apples baseline. Same-session data point: one fully unpatched `-n auto` run under heavy contention (a sibling session's own full-suite `-n auto` run active concurrently) took 29:14; three post-fix runs under lighter (but not zero) contention averaged ~14:17. The reliability delta is the real result here — before this PR, `-n auto` had known, reproducible-if-you're-unlucky failure modes (one of which I hit live); after, 3/3 consecutive full runs and a 5-repeat targeted loop show zero polluter-caused failures. ### [Verification]: `uv run pytest -m "not quarantine and not slow" -n auto -q` — 3 consecutive clean runs (11667 passed / 106 skipped each, zero polluter-caused failures). `uv run core test --suite smoke -q` — 175 passed.

core-labs added 2 commits 2026-07-16 00:02:16 +00:00

fix(tests): isolate three xdist polluter clusters blocking -n auto default ... 531d4aa0d1

Hunt for the -n auto fast-lane polluters flagged in docs/testing-lanes.md
("Follow-up: xdist by default"). Three root causes, all shared-repo-path
writers with no per-test isolation, following the #782 monkeypatch idiom:

1. tests/test_workbench_replay.py::test_replay_leaves_no_trace hardcoded
   `Path("engine_state")` (the real shared dir) instead of reading
   `engine_state._DEFAULT_DIR`, which the root-conftest autouse fixture
   already redirects per-test. It was a victim, not a polluter: any
   concurrent worker writing the real dir made this snapshot-diff flake.
   Fixed by reading `engine_state._DEFAULT_DIR` dynamically.

2. evals/gsm8k_math/train_sample/v1/runner.py hardcoded its report.json
   output to the committed repo path with no override. Two test files
   (test_rat1_end_to_end_admission.py, test_wave_a_multiplicative_
   aggregation_injector.py; 4 tests total) spawn it as a subprocess and
   read the same file back — a write race under -n auto, and a confirmed
   downstream victim (test_gsm8k_sealed_attempt_scout.py::
   test_report_json_mtime_unchanged_by_scout_import asserts the file's
   mtime is stable). Added an optional CORE_GSM8K_TRAIN_SAMPLE_REPORT_PATH
   env override (default unchanged) and pointed the 4 call sites at
   tmp_path.

3. core/cli.py's `_DEMO_RESULTS_DIR` (evals/forward_semantic_control/
   results/) is written, glob-scanned, and index.json-rebuilt by ~11 tests
   across tests/test_cli_demo.py's TestDemoSubcommand and TestDemoPreambles
   classes. Added an autouse fixture monkeypatching `cli._DEMO_RESULTS_DIR`
   to a per-test tmp dir. This also unmasked a latent order-dependent
   coupling: test_demo_list_results_indexes_reports and
   test_demo_list_results_json_well_formed never wrote their own report,
   relying on a sibling test's leftover file in the shared dir (silently
   correct only because pytest ran the file in declaration order). Made
   both self-contained.

No assertion weakening, no test deletion, no global autouse fixture masking
real bugs. All three fixes preserve the real (non-test) default behavior
byte-for-byte when unpatched/env-unset.

Verification: 8x targeted -n 8 loop over the 7 affected files (before and
after) did not force-reproduce the underlying race live (narrow timing
window, small-scale run) — confirmation is source-level (hardcoded shared
paths bypassing the established isolation idiom) plus the prior documented
flake for test_replay_leaves_no_trace in docs/testing-lanes.md. Full
fast-lane -n auto run recorded in the PR description.

fix(tests): isolate 4th polluter victim caught in live -n auto run ... c0cc8c40c4

tests/test_teaching_queue.py::test_read_only_invariant hardcoded
`project_root / "engine_state"` (the real shared dir) in its read-only-
invariant snapshot, instead of reading `engine_state._DEFAULT_DIR` (the
root-conftest autouse fixture's per-test isolated dir). Same bug pattern
as test_workbench_replay.py::test_replay_leaves_no_trace, and this one
was caught red-handed: a full `-n auto` fast-lane run failed with
"Directory .../engine_state was mutated!" and passed cleanly in serial
isolation, the exact order-dependent-flake signature this wave hunts.

The other three snapshotted dirs (teaching/proposals, packs,
contemplation/runs) have no equivalent per-test isolation idiom in the
repo yet and are left as-is — this test's own hitl-queue commands don't
write to them, so they were not implicated in the observed failure.

core-labs referenced this pull request from a commit 2026-07-16 02:49:00 +00:00

ci: install locked dependency universe in all workflows (uv sync --locked)

core-labs added 1 commit 2026-07-16 13:01:12 +00:00

Merge remote-tracking branch 'forgejo/main' into tmp-46-update fbb4c26e1a

core-labs added 1 commit 2026-07-16 13:15:20 +00:00

build: commit uv.lock — the locked universe CI now installs ... df8d35c12f

#48 switched CI to 'uv sync --locked' but uv.lock was gitignored (an early
library-style choice; this repo is an application — the lock belongs in
git). Every run since failed at install: 'Unable to find lockfile'.
Un-ignore and commit the lock (217 packages, matches pyproject via
'uv lock --check'). CI now installs the exact universe local development
and all green verification ran against.

[Verification]: code tree byte-identical to the smoke-gated state
(175 passed); this commit adds a data file CI requires + one .gitignore
line removal.

core-labs merged commit b0156406dd into main 2026-07-16 13:53:15 +00:00

core-labs referenced this pull request from a commit 2026-07-16 13:53:19 +00:00

Merge pull request 'fix(tests): isolate xdist polluters blocking -n auto default (W3)' (#46) from fix/xdist-polluter-isolation into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: core-labs/core#46

No description provided.