Stop excluding tests by band, let tier C report instead of gate, and quote $PYTEST

[lmoresi]

Item 4 of the 2026-09 audit. Depends on nothing, but reads best after #729,
which defines what tier C means.

The bands were never a cost decision

test_06* was disabled as "potentially problematic" and stayed that way.
test_106*/test_107* were held back pending "a triage/deselect decision" that
nobody made. Measured 2026-09-12, those three bands hold 243 tests, of which
only 26 are level_3 or tier_c. 55 are level_1 and tier_a — hardened, fast,
and exactly what CI exists to run.

One slow test (test_1064) was quarantining its neighbours because they shared
a number. They run now; the only failure among them was a test-side defect,
fixed separately in #728.

It is affordable today

The comment justifying the 120-minute cap measured the serial phase at ~55 min
on 2026-08-15, before xdist. The five successful runs before 2026-09-12 spent
34, 35, 43, 44 and 48 minutes in "Run tests". The deferred bands add ~22 min
single-process locally, considerably less under -n 4.

Sharding across parallel jobs stays the durable fix if this tightens again. It
was referenced in the workflow as "the CI-runtime issue" but never actually
filed — now #730, with these numbers.

Tier C reports rather than gates

A tier C failure demands an explanation, not a revert: these are
characterisations that can fail because the code got better. They are
excluded from the batches that set $status and run in their own pass at the
end, which is read but cannot fail the build.

One trap worth flagging in review. The exclusion goes through
PYTEST_ADDOPTS, not appended to $PYTEST. $PYTEST is used unquoted at
every call site, so bash splits -m 'not tier_c' into the words 'not and
tier_c' — and pytest then silently collects nothing and exits 0, reporting
a green run that tested nothing. Verified:

$ pytest -m "'not" "level_2'" tests/test_1070_free_surface_plume.py
no tests collected in 0.00s          # exit 0

pytest shlex-splits PYTEST_ADDOPTS, so the quoting survives there. The
reporting pass clears it with env -u, or it would select nothing itself.

Also drops a line that now runs twice

test_1072 was pulled forward out of test_107* by name because that band was
not batched. The band runs as a whole now, so the named line is redundant — it
is still covered, once.

Verified

Dry run with a stub pytest: every gating batch carries the exclusion, all the
previously-deferred bands appear, test_1072 appears exactly once, and the tier
C pass runs with PYTEST_ADDOPTS unset.

Underworld development team with AI support from Claude Code

[Copilot]

🟡 Changes recommended

Critical test failures and unresolved Tier C, dependency, and option-handling issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Broadens CI coverage by re-enabling deferred test bands and reporting Tier C tests without gating.

Changes:

• Re-enables test_06*, test_106*, and test_107*.
• Adds a non-gating Tier C reporting pass.
• Updates CI runtime rationale and removes redundant execution.

File summaries

File	Summary
scripts/test.sh	Adjusts batching and Tier C handling. Review findings include two critical gating/dependency issues and three moderate issues involving collection scope, marker documentation, and PYTEST_ADDOPTS preservation.
.github/workflows/build_uw3_and_test.yaml	Updates CI runtime rationale and sharding documentation.

Review details

Suppressed comments (1)

scripts/test.sh:269

• This command does not limit collection to Tier C: pytest collects/imports every test module under tests/ before applying -m tier_c. That reintroduces the monolithic run this script explicitly avoids and can trigger the import-time solves/side effects documented in tests/conftest.py (including a 20+ minute collection), even when only a few Tier C tests execute. Run an explicit, collection-safe Tier C file set instead of passing the whole tree.

# answered with an explanation or a re-characterisation, never with a revert.

• Files reviewed: 2/2 changed files
• Comments generated: 4
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[lmoresi]

Review points addressed.

Dependencies, which this PR did not declare — correct, and now stated. It
cannot land alone:

• Nitsche free-slip tests: a wiring check, a real assert, and the comparison at tier C #728 fixes test_1060_nitsche_freeslip, whose bound is still <1e-4
  against an observed 1.0081e-4. Enabling test_106* without it makes the
  batch fail.
• Tier C is what does not gate: the comparison tests, and the Charter wording for them #729 puts the tier_c mark on the test_1070 comparison and updates the
  tier definitions in tests/pytest.ini, TESTING-RELIABILITY-SYSTEM.md and
  HOW-TO-WRITE-UW3-SCRIPTS.md. Without it this PR enables test_107* while that
  comparison still gates, and the new reporting pass contradicts a marker
  contract that still reads "development only, not for automation".

Merge order: #728 → #729 → #723 → this. Retargeted onto #723 to make the
last step explicit.

PYTEST_ADDOPTS — already fixed in an amended commit the review had not seen.
$PYTEST is now a bash array, which also removes the objection behind that
comment: an inherited PYTEST_ADDOPTS is no longer discarded, because the
script no longer sets it.

One thing the rebase found. Landing on #723 surfaced an integration defect
that #723's own guard caught: globs_run_by matched the literal $PYTEST, and
this PR rewrites every call site to "${PYTEST[@]}". The guard saw no globs at
all and declared all 302 test files dark. It now matches both spellings — a
useful demonstration that the guard does its job.

DEFERRED is now empty. Every band that sat there is batched; nothing is
excluded from CI by its number any more. A test that should not gate says so
with @pytest.mark.tier_c, which is a property of the test rather than of where
it falls in the numbering.

Verified: the marker arrives as a single ARG[not tier_c], the tier C pass runs
without it, and the guard reports 302 reachable, 0 deferred.