Repository navigation
ci: test suite needs ~40 min against a 30-min ceiling — cache the Scanner instead of rebuilding it per test case (S-01) #188
Description
CI was cancelled at exactly its 30-minute ceiling three times in a row — #186 twice and #187 once —
with cargo fmt, cargo clippy and cargo test all already green and only the coverage gate
unfinished. The immediate unblock was raising timeout-minutes to 60. That bought time; it did not
fix anything. This issue is the fix.
Measured breakdown
From #186 attempt 2, the run that was cut:
| Step | Duration |
|---|---|
cargo fmt + cargo clippy |
~0.5 min |
cargo test |
18.1 min |
| benchmarks | 0.8 min |
coverage gate (cargo llvm-cov) |
10.5 min, still unfinished at the cut |
So the job needs roughly 40 minutes of work against a 30-minute ceiling. Note the coverage gate
re-runs the whole suite under instrumentation, so the test cost is paid twice per run.
Cause
The slowest test binaries construct a Scanner per test case, and each construction compiles all
79 patterns. Locally:
| Suite | Wall time |
|---|---|
report_roundtrip_test |
~156s |
perf_regression_test |
~155s |
recall_test |
~98s |
pattern_relaxed_control_test |
~80s |
recall_test is the clearest case and was already raised as S-01 in Phase 5's code review
(.planning/phases/05-persistence-lifecycle-hijack-cat-03-35/05-REVIEW.md): detected() calls
scanner() per payload, so recall_matches_the_recorded_numbers compiles the full library 127
times. It was not applied then because it touches shared test infrastructure and was out of scope
for a pattern phase.
Fix
Cache the scanner in a OnceLock<Scanner> (or LazyLock) per test binary rather than rebuilding it
per case, starting with the four suites above. This is the test-side analogue of the code review
checklist's own "regexes compiled once" item, which src/ already honours.
Prerequisite to confirm: Scanner must be Sync to be shared from a OnceLock across the test
harness's threads. If it is not, the cheaper intermediate step is to share the compiled pattern
set and construct the thin per-case wrapper around it.
Why it is worth doing rather than living at 60 minutes
- It cuts both legs, because coverage re-runs the tests.
- Every contributor pays the current cost on every push, and a fork PR pays it on a cold cache.
- The ceiling has now been raised twice (15 → 30 → 60). A third raise is the signal that the suite,
not the limit, is the problem.
Done when
- The four suites above no longer rebuild the scanner per case.
cargo test --lockedwall time is materially down locally and in CI, with before/after numbers
recorded in the PR.timeout-minutesis reconsidered in.github/workflows/ci.yml(the comment there points here).- Detection behaviour is unchanged — the recall numbers and
EXPECTEDrows must be identical.
Activity
ci: raise the job timeout 30 -> 60 and record why (#188)
ci: raise the job timeout 30 -> 60 and record why (#188)
BasilZafar11 commentedon Oct 14, 2026
Implemented in PR #191: #191
The tests now reuse a OnceLock per test binary, with a separate cache for relaxed patterns. Synthetic probes and deliberate compilation measurements remain intact.
On the same Windows machine, the timed suite dropped from 29.90 minutes to 4.32 minutes. Recall dropped from 232.56 seconds to 1.83 seconds with identical EXPECTED rows. A concurrency test confirms shared scans keep their reports and suppression state independent.
The PR records the timing commands and Windows-specific verification caveats. Formatting and Clippy pass. CI is underway; the 60-minute timeout is retained pending a complete hosted coverage measurement.
BasilZafar11 commentedon Oct 14, 2026
PR #191’s tests passed in 4m 38s, down from the 18.1-minute baseline. Coverage passed at 93.41%.
The only failure was the unchanged PERF-01 gate: 213ms against its 200ms budget. Production code and patterns are unchanged by this PR. Could a maintainer rerun the job to check whether that result repeats?
#191
hermanngeorge15 commentedon Oct 20, 2026
Answering your question directly: you don't need a rerun — the gate is nondeterministic, and the failure is not caused by your PR.
Evidence that it isn't a regression:
mainitself has been red on this step since 2026-10-10 (run 38055677954, the Phase 5 completion commit).Your 213ms against the 200ms budget is the same story measured on a hosted runner.
Root cause. The 200ms budget was set in
20c88d1on 2026-08-21, when the library held 48 patterns. v0.2.0 ships 79 — a 65% increase in scan work — and the budget was never revisited. It stayed hidden because the job was being cancelled at 30 minutes before it ever reached this step; raising the ceiling to 60 minutes is what exposed it.There is also an argument already written down in this repo against the gate's design.
tests/perf_regression_test.rssays an absolute millisecond threshold is "either so loose it misses the regression ... or so tight it goes flaky on a loaded CI runner", and uses a self-calibrating ratio for that reason. The CI step then does the thing that comment warns against.I will fix the gate on
mainso your PRs stop being judged by it. Apologies for the six-day silence — that was on me, not on the work.ci: raise the PERF-01 budget 200 -> 400ms for the 79-pattern library (#…
ci: raise the PERF-01 budget 200 -> 400ms for the 79-pattern library (#…