diff --git a/.github/workflows/build_uw3_and_test.yaml b/.github/workflows/build_uw3_and_test.yaml index e7286f99a..f9520f557 100644 --- a/.github/workflows/build_uw3_and_test.yaml +++ b/.github/workflows/build_uw3_and_test.yaml @@ -45,13 +45,16 @@ env: jobs: test: runs-on: ubuntu-latest - # The suite has outgrown 60 minutes. Measured 2026-08-15: the SERIAL phase - # alone is ~55 min (three batches at 14-15 min each), so the parallel phase - # - which runs last - was being cut off mid-run and reported as a failure - # that looked like a test failure. Any addition at all pushed a run over. - # This is a stop-gap that unblocks landing work; the durable fix is to split - # the batches across parallel jobs so wall-clock stops tracking total test - # time. See the CI-runtime issue for that. + # Raised past 60 minutes on a 2026-08-15 measurement (serial phase ~55 min). + # xdist has since cut that: the five successful runs before 2026-09-12 spent + # 34, 35, 43, 44 and 48 minutes in "Run tests", well inside this budget. The + # headroom is why the previously-deferred bands could simply be switched on + # (#721 follow-up) rather than traded against something else. + # + # Sharding the batches across parallel jobs remains the durable fix if this + # tightens again; it was referenced here as "the CI-runtime issue" but never + # actually filed, so the numbers above are recorded in the issue that this + # change opens. timeout-minutes: 120 steps: @@ -87,3 +90,12 @@ jobs: - name: Run tests run: pixi run -e dev ./scripts/test.sh --p 2 + + # The tier C characterisations are a REPORT, not a gate, so they run on a + # push to development rather than on every pull request. They include the + # slow level_3 cases and add ~42 min, which a PR should not pay to learn + # something that cannot block it. Failures here are expected to be read + # and answered, not reverted. + - name: Tier C characterisations (report only) + if: github.event_name == 'push' + run: pixi run -e dev ./scripts/test.sh --tier-c-report diff --git a/scripts/check_test_coverage.py b/scripts/check_test_coverage.py index a384035f0..514f89f39 100644 --- a/scripts/check_test_coverage.py +++ b/scripts/check_test_coverage.py @@ -20,16 +20,11 @@ # Deferred by maintainer decision, not by accident. DEFERRED = { - # No issue: this band was disabled in scripts/test.sh as "potentially - # problematic" without one being filed. Recorded as it stands rather than - # dressed up — #721 follow-up work re-enables it and removes this entry. - "test_06[0-9][0-9]_*": "regression suite disabled in test.sh, no issue filed", - # Narrow: test_1072 is pulled out of this band and run by name, so a broad - # test_107* would list a covered file as deferred. - "test_1070_*": "level_2/level_3 + tier_b/tier_c, awaiting triage (#504)", - "test_1071_*": "level_2/level_3 + tier_b/tier_c, awaiting triage (#504)", - "test_1073_*": "level_2/level_3 + tier_b/tier_c, awaiting triage (#504)", - "test_106*": "level_2/level_3 + tier_b/tier_c, awaiting triage (#504)", + # Empty, and that is the point. Every band that used to sit here — the + # test_06NN regression suite and test_106*/test_107* — is now batched in + # scripts/test.sh. 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 sits in the numbering. } @@ -38,7 +33,10 @@ def globs_run_by(script): text = script.read_text().replace("\\\n", " ") patterns = set() for line in text.splitlines(): - if "$PYTEST" in line and not line.lstrip().startswith("#"): + # Both spellings: the runner is a bash ARRAY, so call sites read + # "${PYTEST[@]}", but a plain $PYTEST is still worth matching in case + # one is left behind or reintroduced. + if re.search(r"\$\{?PYTEST", line) and not line.lstrip().startswith("#"): patterns.update(re.findall(r"tests/[A-Za-z0-9_\[\]*.\-]+", line)) return patterns diff --git a/scripts/test.sh b/scripts/test.sh index 953d24b6a..424881dc1 100755 --- a/scripts/test.sh +++ b/scripts/test.sh @@ -6,6 +6,9 @@ # --p N Run parallel tests with N MPI ranks (default: skip parallel tests) # --full-parallel Add a second parallel pass at 4 ranks (see below; slow on CI) # --parallel-only Run ONLY parallel tests (skip all serial tests) +# --tier-c-report Run ONLY the tier C characterisations and report them. +# Never gates: it always exits 0. CI asks for this on a +# push to development, not on every pull request. # # Examples: # ./test.sh # All serial tests only @@ -22,6 +25,7 @@ status=0 # Parse arguments PARALLEL_RANKS=0 PARALLEL_ONLY=0 +TIER_C_REPORT=0 # Second parallel pass at four ranks. OFF by default: on a 2-core CI runner # np=4 is oversubscribed and the pass costs far more than the ~3 minutes it # takes on a workstation — enough to exceed the 120-minute job cap (#573). @@ -40,9 +44,13 @@ while [[ $# -gt 0 ]]; do PARALLEL_ONLY=1 shift ;; + --tier-c-report) + TIER_C_REPORT=1 + shift + ;; *) echo "Unknown option: $1" - echo "Usage: $0 [--p N] [--full-parallel] [--parallel-only]" + echo "Usage: $0 [--p N] [--full-parallel] [--parallel-only] [--tier-c-report]" exit 1 ;; esac @@ -78,14 +86,56 @@ export MKL_NUM_THREADS=1 # # Unset (or 1) still runs everything in one process, which is what you want # when a test passes alone and fails in a full run. +# An ARRAY, not a string. Every call site below expands it unquoted, so a string +# would be word-split by bash: `-m 'not tier_c'` becomes the two words `'not` and +# `tier_c'`, and pytest answers that by silently collecting NOTHING and exiting 0 +# — a green run that tested nothing. An array carries its elements intact through +# "${PYTEST[@]}", and globs on the command line still expand normally. +PYTEST=(pytest --config-file=tests/pytest.ini) +# Tier C is a REPORT, not a gate: this mode runs the characterisations, prints +# what changed, and always exits 0. +# +# It is separate from the main run because the tier C set includes the slow +# level_3 cases — test_1064 alone is most of it — costing ~42 min, which took +# the job from ~45 to ~100 minutes against a 120-minute cap. Dropping the slow +# ones would re-hide test_1064, whose two-month absence concealed #734, so the +# COST moves off the pull-request path rather than the COVERAGE being cut. +if [ $TIER_C_REPORT -eq 1 ]; then + echo "==========================================" + echo "Tier C characterisations (reported, not gating)" + echo "==========================================" + if pytest --config-file=tests/pytest.ini -m tier_c tests/; then + echo "Tier C: all characterisations still hold." + else + echo "" + echo "⚠️ A tier C characterisation changed. This is NOT a failure." + echo " Explain what moved and re-characterise the test — do not revert" + echo " code to make it pass. See docs/developer/TESTING-RELIABILITY-SYSTEM.md." + fi + exit 0 +fi + if [ -n "$WORKERS" ] && [ "$WORKERS" -gt 1 ]; then echo "Serial batches: $WORKERS worker process(es)" - PYTEST="pytest --config-file=tests/pytest.ini --dist loadfile -n $WORKERS" + PYTEST+=(--dist loadfile -n "$WORKERS") else echo "Serial batches: in-process (set WORKERS=N to distribute)" - PYTEST="pytest --config-file=tests/pytest.ini" fi +# Tier C never gates. A tier C failure demands an EXPLANATION, not a revert +# (Charter S8, docs/developer/TESTING-RELIABILITY-SYSTEM.md): these are +# characterisations that can fail because the code got BETTER, and comparisons +# whose result moves with the fixture. They are excluded from the batches that +# set $status and run in their own reporting pass at the end, which is read but +# cannot fail the build. +# Tier C never gates. A tier C failure demands an EXPLANATION, not a revert +# (Charter S8, docs/developer/TESTING-RELIABILITY-SYSTEM.md): these are +# characterisations that can fail because the code got BETTER, and comparisons +# whose result moves with the fixture. They are excluded from the batches that +# set $status and run in their own reporting pass at the end, which is read but +# cannot fail the build. +PYTEST+=(-m "not tier_c") + # Run serial tests (unless --parallel-only specified) if [ $PARALLEL_ONLY -eq 0 ]; then echo "Running serial test suite..." @@ -98,35 +148,37 @@ if [ $PARALLEL_ONLY -eq 0 ]; then python3 "$(dirname "$0")/check_test_coverage.py" || status=1 # Run simple tests (0000-0299: basic functionality, imports, simple operations) - $PYTEST tests/test_00[0-4]*py || status=1 - $PYTEST tests/test_0050*py || status=1 + "${PYTEST[@]}" tests/test_00[0-4]*py || status=1 + "${PYTEST[@]}" tests/test_0050*py || status=1 # test_006[2-9] and test_007x matched NO batch glob and so never ran in CI: # the whole integration-point suite (0064-0067), swarm repopulation, # mid-time velocity and the VE stress history. They are not covered by the # disabled test_06*py line below either - that one is 0600-0699. The band is # taken whole (00[6-7]) rather than enumerated, so a test added next to its # siblings is not dark again. Verified passing (103 tests) before wiring in. - $PYTEST tests/test_005[1-9]*py tests/test_00[6-7]*py || status=1 - $PYTEST tests/test_01*py || status=1 - $PYTEST tests/test_02*py tests/test_03*py || status=1 + "${PYTEST[@]}" tests/test_005[1-9]*py tests/test_00[6-7]*py || status=1 + "${PYTEST[@]}" tests/test_01*py || status=1 + "${PYTEST[@]}" tests/test_02*py tests/test_03*py || status=1 # Intermediate tests (0500-0799: data structures, transformations, enhanced interfaces) - # NOTE: Temporarily disabling test_06*py regression tests (potentially problematic) - $PYTEST tests/test_05*py tests/test_07*py || status=1 - # $PYTEST tests/test_06*py || status=1 # DISABLED - regression tests need validation + # test_06*py was disabled as "potentially problematic" and stayed that way. The + # whole band was measured 2026-09-12 and passes; a band is not a unit of trust, + # and holding one back for a suspicion nobody recorded meant its level_1 files + # never ran at all. + "${PYTEST[@]}" tests/test_05*py tests/test_06*py tests/test_07*py || status=1 # Units system tests (0800-0899: unit-aware functions, arrays, and conversions) - $PYTEST tests/test_08*py || status=1 + "${PYTEST[@]}" tests/test_08*py || status=1 # Poisson solvers (including Darcy flow) - $PYTEST tests/test_100[0-9]*py tests/test_103*py || status=1 + "${PYTEST[@]}" tests/test_100[0-9]*py tests/test_103*py || status=1 # Solver / system tests (advanced solver problems) # test_101* / test_102* include the rotated free-slip suite (test_1018, # issue #504) and the MG / boundary-flux suites, which previously matched # no batch glob and never ran in CI. - $PYTEST tests/test_101*py tests/test_102*py || status=1 - $PYTEST tests/test_105*py || status=1 + "${PYTEST[@]}" tests/test_101*py tests/test_102*py || status=1 + "${PYTEST[@]}" tests/test_105*py || status=1 # The boundary-normal guard lives under tests/parallel/ but carries NO # mpi(min_size=2) mark, because the defect it guards is present in SERIAL in @@ -134,26 +186,27 @@ if [ $PARALLEL_ONLY -eq 0 ]; then # spherical shell at np=1). Run it here so the serial job covers that path — # every other serial test of the default boundary normal is on a box, where # flat walls make the question vacuous. It also runs in the --p N batch below. - $PYTEST tests/parallel/test_1069_boundary_normal_parallel.py || status=1 - # NOT yet batched (issue #504 audit): test_106*py and test_107*py contain - # level_2/level_3 + slow + tier_b/tier_c suites (e.g. test_1064) and need - # a triage/deselect decision before being wired into CI. + "${PYTEST[@]}" tests/parallel/test_1069_boundary_normal_parallel.py || status=1 + # test_106*/test_107* were held back pending "a triage/deselect decision". That + # decision is made: the band is not the unit of exclusion. Of the 243 tests in + # the deferred bands only 26 are level_3 or tier_c; 55 are level_1 AND tier_a — + # hardened, fast, and exactly what CI exists to run. test_1064 is the slow one + # and it is excluded by WHAT IT IS (tier_c) below, not by its number. + "${PYTEST[@]}" tests/test_106*py tests/test_107*py || status=1 # - # test_1072 is pulled forward out of that group, the same way test_1069 is - # above, because leaving it there defeats its purpose: it is the ONLY guard on - # the 3-D free-surface sign and relaxation rate, and #496 exists precisely - # because those regressions were invisible to CI. Landing the test into an - # unbatched file would have closed the issue without closing the gap. - # level_2/tier_b, ~55s serial; passes at np=1 and np=2. - $PYTEST tests/test_1072_free_surface_spherical.py || status=1 + # test_1072 used to be pulled forward out of test_107* by name, because that + # band was not batched and it is the ONLY guard on the 3-D free-surface sign + # and relaxation rate (#496 exists because those regressions were invisible to + # CI). The band runs as a whole now, so the named line is gone rather than + # running it twice. # Diffusion / Advection tests - $PYTEST tests/test_1100*py || status=1 - $PYTEST tests/test_1110*py tests/test_1120*py || status=1 # Annulus + vector SL - $PYTEST tests/test_1450*py || status=1 + "${PYTEST[@]}" tests/test_1100*py || status=1 + "${PYTEST[@]}" tests/test_1110*py tests/test_1120*py || status=1 # Annulus + vector SL + "${PYTEST[@]}" tests/test_1450*py || status=1 # Named (un-numbered) test files - JIT, docstrings, projections - $PYTEST tests/test_docstring_utils.py tests/test_jit_cache.py \ + "${PYTEST[@]}" tests/test_docstring_utils.py tests/test_jit_cache.py \ tests/test_jit_deterministic_ordering.py \ tests/test_multicomponent_projection.py \ tests/test_snes_vector_asymmetric_jacobian.py \