Skip to content

Widen the skip ceiling to the whole suite, fix five CWD-dependent defects, add a cwd-independence CI job - #721

Merged
willhea merged 13 commits into
developfrom
fixture-layout-runtime-boundary
Oct 4, 2026
Merged

willhea merged 13 commits into
developfrom
fixture-layout-runtime-boundary

Conversation

@willhea

@willhea willhea commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator

Based on develop. #720 has merged, so the diff shows only this PR's own commits. Code-only.

The runtime skip ceiling already in the repo watches only 25 of 104 test modules. This PR widens it to the whole suite — and in doing so found five live defects the static rules report green over. It does not follow, and this PR does not claim, that the runtime boundary could replace the static scan; see the note at the end.

1. The skip ceiling now watches the whole suite

tests/conftest.py carried a session-level ceiling: a skip in a watched module that is not declared fails the session. It watched an enumerated roster of 25 test modules out of 104, so a fixture-absence skip in any of the other 79 reported green. That roster is the shape #654 removed from the file scan one tier over, with the same defect: a module nobody listed is exempt the day it is created.

The default is now watched. The three tier rosters survive to route a skip to the allowlist documenting it; membership is no longer something anyone has to remember.

The suite named the widening cost, not me: seven declarations, six with the corpus present and one more CI-shaped. All seven are opt-in modes or environment requirements rather than corpus gaps — four regenerate maintenance commands behind a flag, one live-network gate (#278/#342), one local-only byte comparison, one two-root collision probe. None is a fixture that should simply be committed, so unlike ALLOWED_FAST_GATE_SKIPS an empty state is not the goal here.

Proven to fire, in a module no roster names. Pointing a fixture constant in test_classify_bill.py at an absent bill:

52 passed, 1 skipped        <- pytest's own verdict
rc=1, "default suite-wide skip ceiling"   <- the session's

The counterfactual isolates it: the identical mutation against the pre-widening conftest exits 0 with no ceiling output.

2. Five CWD-dependent defects, fixed

#404 moved test_data/ to tests/data/ and left a regex behind to stop the CWD-relative spelling coming back. That regex enumerates the tree names it knows — test_data and tests/data — so a CWD-relative path into any other directory is unpoliced. Running the suite from a directory that is not the repository root found five live ones:

Site Shape
test_canonical_tree.py:26 Path("schema/canonical-diff.schema.json") (failed 2 tests)
test_diff_bill.py:1397 bare "diff_bill.py" to subprocess.run, no cwd=
test_corpus_manifest.py:681 child pytest collection against a relative target, exit 4
test_reconcile.py:209 repo-relative literals behind a skip guard — skipped green

The last one is the one that matters, and no boundary in the repository caught it alone:

  • the static rules enumerate test_data/tests/data, never tests/corpus — and a bare repo-relative string is deliberately not treated as path construction
  • the CWD-varying run missed it because it is a skip, not a failure
  • the ceiling missed it because from the repository root the fixture resolves and nothing skips

The two composing caught it: the foreign CWD created the condition, the widened ceiling made the silent skip loud. That is the argument for the runtime boundary in one example — it observes the consequence, so it catches spellings no enumeration anticipated.

3. A cwd-independence CI job

Every other job runs from the repository root, so this class of defect is green in all of them.

Non-matrix on purpose: it varies the working directory, not the interpreter, and path resolution does not differ across supported versions.

Every non-browser tier, not the fast one originally proposed. That correction was measured: three of the five live dependencies sat behind @pytest.mark.slow, including the only one that skipped rather than failed — a fast-tier job would have shipped green while missing the case it exists for.

Proven to fire, and the proof is the whole argument:

result
repository root 10 passed, rc=0 — the bug hides, as it really did
this job 3821 passed, zero failures, rc=1

It reddens with no test failing at all.

4. The browser tier is exempt, and that is deliberate

Widening the ceiling swept in the browser tier, whose skip channel is the module-scoped chromium fixture. #599 established that skipping there is correct for the default tier — a contributor's machine may lack Playwright — while CI's browser step passes --run-browser to make the same condition a failure.

Measured before the exemption: with the browsers path pointed at nothing, 41 undeclared skips and exit 1. The widening turned documented local behaviour into a wall of red for exactly the contributor #599 accommodates. That regression was introduced and fixed inside this branch, found by running the one tier the non-browser suite never covers.

The flag is the stronger control and needs no list. An allowlist could not express it anyway: the skip reason interpolates the launch exception, so no entry could ever match. The exemption is two named modules, not the marker or a prefix, so it cannot widen into a way to silence any skip by phrasing it like a browser one — both directions are pinned in a test.

⚠️ Behaviour change for in-flight branches

This makes previously-green sessions fail. Any branch with an undeclared skip in one of the 79 previously-unwatched modules now fails the session with a named case and reason. That is the intended effect, but it will surprise someone mid-review, so it is worth saying out loud rather than being discovered.

The remedy is in the failure message: fix the regression, or declare the case in the allowlist its ceiling names.

Verification

  • Full non-browser suite: 3827 passed, rc=0, on the current base.
  • All six ci.yml job invocations run verbatim in a CI-shaped checkout (no bills/): rc=0 each. The ceiling is session-scoped, so each job's own narrow subset had to be checked rather than inferred from a full run.
  • Browser tier: 41 passed rc=0 with Chromium, 41 skipped rc=0 without it.
  • The new CI job's exact command: 3822 passed, rc=0, 2m40s — it does not land red.
  • Suite is now CWD-independent: byte-identical totals from the repository root and from outside it.

Review follow-up: collection-time skips, and #700's lockfile rule

Two review findings, fixed on top of a merge of develop (which now contains #700).

Collection-time skips reached exit 0. The ceiling recorded skips only in pytest_runtest_logreport. A module that skips while importing (pytest.skip(..., allow_module_level=True), or a module-level importorskip) never creates its tests, so they never reach that hook. With one passing module next to it, the session exited 0 while that module's assertions never ran. pytest_collectreport now writes into the same record, so the existing policy covers it unchanged: exact nodeid + reason matching, the allowlists, the two-module browser exemption, and "escalate only a clean run". Under xdist (the default -n auto), the controller re-emits a worker's skipped collection report through this hook.

test_ceiling_fails_session_on_a_collection_time_skip_end_to_end (serial and xdist) runs a child session with an unlisted module that skips during import plus a passing control module. It asserts exit 1 and a banner that names tests/test_unlisted.py with reason: missing fixture. Proven decisive: with the hook body disabled, both cases fail with expected exit 1, got 0 (1 passed, 1 skipped), and the existing end-to-end ceiling tests stay green. The new test covers a different hook, so no older test became redundant.

cwd-independence now passes #700's guard. The job now runs uv sync --locked and uv run --locked --project "$GITHUB_WORKSPACE" pytest "$GITHUB_WORKSPACE/tests" -m "not browser" -q. It still runs from outside the checkout and still covers every non-browser tier. After the merge, test_ci_workflow.py went from 1 failed (exactly these two commands) to 33 passed.

Local verification of this revision (GitHub checks are reported separately on this PR):

Gate Result
ruff check . / ruff format --check . clean
pytest -m "not slow and not browser" 2089 passed, 2 skipped, 15 xfailed, rc=0
pytest -m browser --run-browser 41 passed, rc=0
pytest -m browser, Chromium unavailable 41 skipped, rc=0 (exemption intact)
pytest -m slow --deselect tests/test_govinfo_corpus_parity.py 1809 passed, 30 skipped, rc=0
cwd-independence command verbatim, CI-shaped clone (no bills/, CI=true), cwd outside it 3894 passed, 37 skipped, 15 xfailed, rc=0

The last run collects every module, so it is also the check that the new hook flags no collection-time skip anywhere in the suite today.

Known limitation, not addressed here: an allowlist key for a whole module (no ::) would match exactly. However, test_every_allowlist_key_names_a_test_that_still_exists would report that key as stale, because it looks for a def. No such declaration exists or is needed today. If one is ever needed, that guard has to accept module-level keys.

Review notes

test_ci_workflow.py's aggregator and leaf-job rosters are widened to eight by hand. Those are ceilings of the shape the audit argues for — literals inside an assertion — so adding a job is deliberately an edit to a test.

One thing to know for later: #720's G2 exemption ceiling would retire if the static scan were ever narrowed to scripts/ — no fetcher or test file would be scanned, so the exemption sets it bounds would cease to exist. Whether that narrowing should happen is unresolved, so the ceiling is not written as a temporary measure: it is the right bound for the scan as it stands.

This PR is not a step toward narrowing the static scan. Whether the scan could shrink is unresolved and out of scope: file-level coverage does not establish that a file's path-bearing lines ran, and a skip ceiling sees only skips that actually occur, so neither supports replacing the scan. The ceiling widening and this CI job stand on their own — the ceiling closes a hole that exists regardless, and the job tests a property the regex only approximates.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NwVE7osYo39KaUe7CEitFJ

@willhea
willhea force-pushed the fixture-layout-runtime-boundary branch 2 times, most recently from f7f1534 to f65d5a2 Compare September 11, 2026 01:14
@willhea
willhea force-pushed the fixture-layout-audit-and-gap-repairs branch from 43e0b7b to 3b3b366 Compare September 11, 2026 15:12
@willhea
willhea force-pushed the fixture-layout-runtime-boundary branch from f65d5a2 to de4f690 Compare September 11, 2026 15:17
willhea and others added 10 commits September 11, 2026 14:02
…suite

The ceiling watched an enumerated roster of 25 test modules out of 104, so
a fixture-absence skip in any of the other 79 reported green. That roster
is the exact shape #654 removed from the file scan one tier over, and it
carries the same defect: a module nobody listed is exempt the day it is
created.

The default is now WATCHED. DEFAULT_GATE_MODULES = ("tests/",) is matched
LAST, so the three tier rosters still own their cases and keep routing a
skip to the allowlist that documents its argument; membership is simply no
longer a decision anyone has to remember to make.

Widening cost seven declarations, and the suite named them rather than me
guessing: six with the corpus present, one more CI-shaped. All seven are
opt-in modes or environment requirements, not corpus gaps -- four are
`regenerate` maintenance commands behind a flag, one needs live network by
design (#278/#342), one compares freshly-downloaded bytes, one needs a
bill fetched into both roots. None is a fixture that should be committed
instead, so unlike ALLOWED_FAST_GATE_SKIPS an empty state is not the goal.

Proven to fire, in a module no roster names: pointing a fixture constant in
tests/test_classify_bill.py at an absent bill makes pytest report
"52 passed, 1 skipped" while the session exits 1, labelled "default
suite-wide skip ceiling". The counterfactual isolates it -- the identical
mutation against the pre-widening conftest exits 0 with no ceiling output.

Also adds a liveness guard over all four allowlists, because a suite-wide
ceiling is only as good as the list that excuses it: an entry whose test
was renamed or deleted is inert while still reading as policy, the #424
shape. The two existing drift guards compare fixture ids and are blind to
it, and every ALLOWED_DEFAULT_SKIPS key carries no fixture id at all.

Verified: 3822 passed with the corpus, rc=0 CI-shaped without it, and the
liveness guard shown to fire on both a missing module and a renamed test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwVE7osYo39KaUe7CEitFJ
_SCHEMA was a bare relative literal, Path("schema/canonical-diff.schema.json"),
so it resolved only when pytest happened to be run from the repository
root. Run from anywhere else, both tests that validate against the
published schema died on FileNotFoundError.

This is #404's shape in a tree #404 did not touch. The fixture-layout
rule that exists to catch it reports green here, because
_DATA_TREE_NAME_RES enumerates the tree names it knows -- test_data and
tests/data -- and schema/ was never on that list.

PROJECT_ROOT comes from tests.corpus_paths, which the module already
imports from, keeping one home for the name rather than composing a
fourth spelling from __file__ (whose .parent chain is coupled to the
module's own depth).

Verified: 10 passed from the repository root, and 10 passed from a
directory outside it, where this previously failed 2.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwVE7osYo39KaUe7CEitFJ
test_subprocess_entrypoint passed a bare "diff_bill.py" to subprocess.run,
which the child resolved against whatever directory pytest was started in.
Run from anywhere but the repository root it died with "can't open file
'<cwd>/diff_bill.py'", so the only smoke test of the real entrypoint was
silently tied to one working directory.

The script is now addressed absolutely and cwd is deliberately left alone.
Passing cwd=PROJECT_ROOT would also fix the failure, but it asserts less:
the entrypoint is meant to work from anywhere, and running it from an
unrelated directory is what proves it does.

Verified: 73 passed from the repository root and 73 from a directory
outside it, where this previously failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwVE7osYo39KaUe7CEitFJ
_collect_pdf_smoke_parameters ran a child pytest session against the
repo-relative "tests/test_pdf_corpus_smoke.py", which the child resolved
against whatever directory the outer session was started in. Run from
anywhere but the repository root it collected nothing and exited 4, so the
#601 completeness floor -- the check that every smoke pair is actually
collected -- was itself tied to one working directory.

The target is now absolute. Nodeids stay rootdir-relative either way, so
the "tests/test_pdf_corpus_smoke.py::TestPdfCorpusSmoke::" prefix the
recorder filters on is unaffected.

Verified: 40 passed from the repository root and 40 from a directory
outside it, where this previously failed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwVE7osYo39KaUe7CEitFJ
…WD literal

TestReconcileIntegration held its two 118-hr-2882 fixtures as repo-relative
string literals, so off the repository root they resolved to nothing and
_skip_if_missing SKIPPED the case rather than failing it.

This is the worse half of #404's shape. The other three CWD dependencies
fixed on this branch announce themselves as errors; this one reported
green while asserting nothing, which is the exact fail-open ADR 0015 and
the whole fixture-layout cluster exist to remove.

It was invisible to both boundaries until now. The static rules do not see
it because _DATA_TREE_NAME_RES enumerates test_data and tests/data, never
tests/corpus, and a bare repo-relative string is deliberately not treated
as path construction. The CWD-varying run does not see it either, because
it is a skip and not a failure. What caught it was the two composing: the
foreign CWD created the condition and the widened skip ceiling made the
silent skip loud, naming the case and its reason.

Verified: 10 passed from the repository root, and 10 passed with NO skip
from a directory outside it, where the case previously skipped green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwVE7osYo39KaUe7CEitFJ
…ximates

Every other job runs from the repository root, so a fixture path resolved
against the CURRENT WORKING DIRECTORY rather than against the tree is
green in all of them. That is #404, and it survived its own fix: the rule
left behind to catch it (_DATA_TREE_NAME_RES) matches only the tree names
it enumerates -- test_data and tests/data -- so a CWD-relative path into
any other directory is unpoliced. Five were live when this branch started.

The property is about how a path RESOLVES, and a text scan can only ask
which directory is spelled, so it must enumerate -- and an enumeration is
what #367, #398, #424 and #654 each found a hole in. Varying the CWD tests
the property directly and needs no list.

Non-matrix on purpose: it varies the working directory, not the
interpreter, and path resolution does not differ across the supported
versions. A matrix would buy four identical answers.

Every non-browser tier, NOT just the fast one. Measured while writing
this: three of the five live dependencies sat behind @pytest.mark.slow,
including the only one that SKIPPED rather than failed, so
-m "not slow" would have shipped a job that missed the case it exists for.

Proven to fire, and the proof is the argument for the job:
  repo root       10 passed, rc=0  -- the bug hides, as it really did
  this job        3821 passed, ZERO failures, rc=1
The job reddens with no test failing at all. The foreign CWD creates the
condition and the widened skip ceiling turns the silent skip into a named
result; neither catches it alone.

Green today: 3822 passed, rc=0, 2m40s under the job's exact command, so it
does not land red.

test_ci_workflow.py's aggregator and leaf-job rosters are widened to eight
by hand. They are ceilings of the shape #10 argues for -- literals inside
an assertion -- so adding a job is deliberately an edit to a test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwVE7osYo39KaUe7CEitFJ
Widening the ceiling to every test module swept in the browser tier, whose
skip channel is the module-scoped chromium fixture. #599 established that
skipping there is CORRECT for the default tier -- a contributor's machine
may lack Playwright -- while CI's browser step passes --run-browser to
turn the same condition into a failure.

Measured before this commit, with the browsers path pointed at nothing:
41 undeclared skips and exit 1. The widening turned the deliberate,
documented local behaviour into a wall of red for exactly the contributor
#599 set out to accommodate. That was a regression introduced by the
previous commit, found by running the tier the non-browser suite never
covers.

The flag is the stronger control and needs no list. An allowlist could not
express this in any case: the skip reason interpolates the launch
exception, so no entry could ever match, and the ceiling compares reasons
by design.

The exemption is two named modules rather than the `browser` marker or a
prefix, so it cannot widen into a way to silence any skip by phrasing it
like a browser one. Both directions are pinned in a test.

Verified: browser tier 41 passed rc=0 with Chromium and 41 skipped rc=0
without it; the ceiling still fires in an unwatched non-browser module
(rc=1); full non-browser suite 3823 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwVE7osYo39KaUe7CEitFJ
The PROJECT_ROOT import added for the CLI smoke test sat between two
`from conftest import` lines; ruff sorts it into the tests. group. The
_WATCHED_SKIP_MODULES concatenation fits on one line once formatted.

Both cosmetic. Found by CI, not locally -- the lint gate was the one of
the five I did not run before opening the PR.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwVE7osYo39KaUe7CEitFJ
The widened ceiling reddened CI in the fast tier and the cwd-independence
job on two cases I had not declared:

  tests/test_research_probes.py::test_adjacent_pairs_are_consecutive
  tests/test_research_probes.py::test_body_less_target_nodes_are_always_containers
      reason: the union corpus needs the gitignored bills/ tree, which CI
              does not have

Both read the UNION of the two corpus roots, so they need the gitignored
bills/ tree, and their authors gated them on `os.environ.get("CI") ==
"true"` directly rather than on the tree being absent. Declaring them is
correct: the skip is an environment requirement no fixture can retire.

The reason my pre-flight missed them is the point worth recording. I
simulated a CI-shaped checkout by moving bills/ aside, which is only half
of what CI is -- the other half is CI=true in the environment. The skip
fires on the VARIABLE, not on the missing path, so the probe was green for
the wrong reason while CI was red on the same commit. The harness now
exports CI=true and also runs the cwd-independence invocation.

Verified under a faithful CI shape (CI=true, no bills/): all seven ci.yml
invocations rc=0, including cwd-independence. Plus ruff check and ruff
format --check clean, which is the gate the previous push missed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwVE7osYo39KaUe7CEitFJ
The ceiling, its allowlists, the browser exemption, the CI job and the four
CWD fixes each documented their own discovery: measured pass counts, a
fast-tier design that was tried and rejected, the local-versus-CI debugging
that found the CI=true gating. That is review history and belongs in the
pull request, not the source.

Kept: why the default group is matched last, what --run-browser does that
an allowlist cannot, why the exemption is two named modules rather than a
marker, why the CWD job exists at all, and the absolute-path reason at each
of the four fix sites.

Behaviour unchanged -- no logic touched, only prose and assertion messages.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwVE7osYo39KaUe7CEitFJ
@willhea
willhea force-pushed the fixture-layout-runtime-boundary branch from de4f690 to c94f999 Compare September 11, 2026 18:12
Base automatically changed from fixture-layout-audit-and-gap-repairs to develop September 11, 2026 18:37
willhea and others added 3 commits October 4, 2026 00:55
#700's guard requires every resolving uv command to carry --locked
directly after the subcommand. The job still runs from outside the
checkout and covers every non-browser tier.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The ceiling recorded skips only through pytest_runtest_logreport, so a
module-level pytest.skip(allow_module_level=True) or importorskip, whose
tests are never created, left an otherwise green session at exit 0.
pytest_collectreport now feeds the same record, so classification,
allowlists, the browser exemption and the exit-status policy apply
unchanged. The xdist controller re-emits worker collection skips through
the same hook.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@willhea
willhea added this pull request to the merge queue Oct 4, 2026
Merged via the queue into develop with commit f8d769c Oct 4, 2026
28 checks passed
@willhea
willhea deleted the fixture-layout-runtime-boundary branch October 4, 2026 05:32
willhea added a commit that referenced this pull request Oct 4, 2026
Resolves the module-docstring conflict in tests/test_ci_workflow.py by
pinning both #678's lockfile rule and #672's partition guards.

#721 now watches every module's skips through the default group, so the
partition section no longer claims modules outside CI_SLOW_MODULES run
with unwatched skips. The duplicate guard is described as covering
explicit module listings only, since the cwd-independence job reruns
the non-browser suite by directory. The roster guard is described by
what it protects: members' matrix coverage under a slow step.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
rocks737 pushed a commit to rocks737/DeltaTrack that referenced this pull request Oct 5, 2026
…e the checkout

The project supports one interpreter, the one `.python-version` pins and
contributors and the deployment already use, and moves to a newer one by
editing the pin. Nothing is published, so no consumer installs it on
another version. CI ran every job on four (3.12.0, 3.12, 3.13, 3.14): 26
jobs per run. The 3.12.0 leg finished last in every sampled run (384-400s
for corpus-gates, against 172-344s on the other legs); CPython 3.12.0 runs
this suite about 25% slower than 3.12.11, measured locally on the same
tests. Every interpreter-specific defect found so far was in the fast tier
or the packaging/subprocess tests, never in the corpus gates, and the
argparse re-entry guard that 3.12.0 exercised is already simulated on any
interpreter by TestIntermixedSubParserGuard.

The separate cwd-independence job (civictechdc#721) reran the whole non-browser
suite from outside the checkout. Once 3.12.0 left the slow jobs it would
have been the longest job (365-396s). Instead every non-browser test step
now runs from a runner.temp directory with absolute paths, which tests the
same property (civictechdc#404) for the same modules without running them twice. The
collected counts were checked: the steps' 3961 tests plus the one
network-only test corpus-parity.yml runs equal the 3962 non-browser tests.

Guards (tests/test_ci_workflow.py):
- test_ci_runs_only_the_pinned_interpreter replaces the fail-fast and
  UV_PYTHON matrix guards, which had nothing left to check. Its message
  says to restore both if a matrix returns. Red when a job installs
  another version or declares a matrix (both checked).
- test_non_browser_test_steps_run_from_outside_the_checkout pins the
  property cwd-independence used to carry as a job of its own. Red when
  corpus-gates loses its working-directory (checked).
- The module-argument regex accepts the quoted "$GITHUB_WORKSPACE/tests/..."
  form. With the old regex the slow-module coverage guards go red on the
  live workflow (checked), so a regression here is loud.
- Aggregator and leaf-job sets drop cwd-independence.

requires-python stays ">=3.12": nothing installs the package, so the pin,
not the range, decides what runs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013Bo57HgnRZjxDPvzi8eJvo
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