Skip to content

test: cache scanners per test binary to cut CI runtime - #191

Merged
hermanngeorge15 merged 3 commits into
UnityInFlow:mainfrom
BasilZafar11:codex/cache-test-scanners-188
Oct 21, 2026
Merged

hermanngeorge15 merged 3 commits into
UnityInFlow:mainfrom
BasilZafar11:codex/cache-test-scanners-188

Conversation

@BasilZafar11

@BasilZafar11 BasilZafar11 commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

The recall, pattern, example, and corpus tests repeatedly compiled all 79 embedded patterns for each payload. They now share a OnceLock<Scanner> per integration-test binary through tests/common/mod.rs; relaxed-pattern controls retain a separate cached scanner. The four suites named in #188 use the cache, along with the other repeated scan helpers.

The static cache confirms Scanner is Send + Sync. A concurrent test verifies shared identity and that suppressed, clean, and attack reports remain independent. Synthetic probe scanners and the performance test's deliberate fresh-compilation samples are preserved. Production code, patterns, dependencies, and the complete EXPECTED recall table are unchanged.

Local before/after timings

Windows, Rust stable, unoptimized test profile, same machine and default test parallelism, one complete run each:

Suite Before After
recall_test 232.56 s 1.83 s
report_roundtrip_test 1.37 s 1.00 s
perf_regression_test 10.73 s 7.65 s
pattern_relaxed_control_test 16.26 s 5.94 s
pattern_test 487.35 s 1.92 s
pattern_example_test 523.09 s 1.50 s
corpus_test 70.08 s 3.35 s
cargo test command elapsed 1794.10 s (29.90 min) 259.43 s (4.32 min)

Both commands were cargo test --locked --no-fail-fast -- --skip findings_name_the_repository_path_not_the_staging_copy, excluding the existing Windows hook path-separator assertion. Command elapsed includes compilation (baseline 61 s, candidate 5.53 s); test-binary timings above exclude it. The baseline completed with only the Windows CRLF catalogue equality failure; the candidate passed after local LF normalization, which creates no committed catalogue diff. The unchanged prefilter-equivalence suite remains a substantial runtime cost (150.50 s before, 139.13 s after). These are measured local results, not a forecast for hosted CI.

Validation and CI ceiling

  • Cached suite, concurrency regression test, exact recall, clean corpus, and relaxed controls pass.
  • cargo fmt --all -- --check and cargo clippy --locked --all-targets -- -D warnings pass.
  • The CI timeout has been reconsidered: retain 60 minutes until a complete hosted coverage run supplies the cold-build measurement; update the workflow comment to explain the decision. Coverage still runs the full suite and should benefit from the same caches.
  • Hosted CI baseline from ci: test suite needs ~40 min against a 30-min ceiling — cache the Scanner instead of rebuilding it per test case (S-01) #188: the test step took 18.1 minutes. In run 38538258849, the candidate test step passed in 4 minutes 38 seconds (08:53:41–08:58:19 UTC). Coverage passed in 4 minutes 38 seconds with 93.41% line coverage. Formatting, Clippy, benchmarks, release build, and smoke tests passed.
  • The workflow's only failure was the unchanged PERF-01 gate: best of three was 213 ms against the 200 ms budget. Production code and patterns are identical to the base. This result alone does not distinguish runner variance from a pre-existing budget miss. An unchanged retry requires a maintainer: both connected integration and author credentials returned HTTP 403 for rerun.

Closes #188.

@hermanngeorge15 hermanngeorge15 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and independently verified. This is the #188 fix and it does what it claims.

Measured on macOS (arm64), same machine, same command, cargo test --locked:

branch wall time
main 11m 02s
this PR 3m 26s

3.2x, both green, exit 0. cargo fmt --all -- --check and cargo clippy --all-targets --locked -- -D warnings are both clean on the branch. That corroborates your 29.90 -> 4.32 min on Windows and 4m 38s on CI — three different machines, the same shape of result.

On the design, the parts I checked specifically:

  • tests/common/mod.rs — OnceLock<Scanner> per integration-test binary is the right granularity. Rust builds one binary per tests/*.rs, so this is one compile per binary rather than one global, which keeps the suites independent.
  • pattern_relaxed_control_test.rs correctly keeps a separate cached relaxed scanner. Sharing the shipped one would have broken GATE-05.
  • perf_regression_test.rs keeps its deliberate fresh-compilation sampling. That was the one place a naive cache would have silently invalidated the measurement, and you avoided it.
  • tests/pattern_policy_test.rs walks tests/*.rs non-recursively, so tests/common/mod.rs sitting in a subdirectory does not disturb the test-case policy counts.

CI is red on PERF-01 only, and that is not your fault — see my reply on #188. Nothing in this PR touches production code or patterns.

One non-blocking suggestion. The diff removes the measured timing breakdown comment in ci.yml (the 18.1-minute test leg and the per-binary figures). Those numbers are the evidence for why the ceiling is 60 minutes, and they are also the "before" half of the proof that this PR worked. Consider keeping them, relabelled as the pre-#188 baseline, next to your new note.

Approving. I want the gate fixed on main first so this merges green rather than merging over a red badge.

@hermanngeorge15
hermanngeorge15 force-pushed the codex/cache-test-scanners-188 branch from 2af1a95 to e53c2e2 Compare October 21, 2026 07:36
@hermanngeorge15
hermanngeorge15 merged commit 4cb10d1 into UnityInFlow:main Oct 21, 2026
1 check passed
hermanngeorge15 added a commit that referenced this pull request Oct 21, 2026
The hook runs fmt, clippy and the full test suite after EVERY turn, regardless
of what the turn touched. On a docs-only session that is pure cost: before
#191 it was a guaranteed stall, because `cargo test` took longer than the
hook's own 900s timeout (11m02s locally, 18.1 min on CI) and was killed
without reporting anything. #191 cut that to 3m01s so the gate completes now,
but three minutes of recompiling after editing a Markdown file still buys no
information.

Gate on whether anything fmt, clippy or the tests actually read has changed:
*.rs, patterns/, tests/, Cargo.toml, Cargo.lock, build.rs and
rust-toolchain.toml, in the working tree first and then in commits not yet on
origin/main, so a feature branch stays gated after its work is committed.
tests/ is watched because corpus files drive the recall numbers even when no
pattern moves.

Verified: clean tree exits 0 in 0s; touching src/lib.rs, patterns/core/*.yaml
or a tests/corpus file each still runs the full gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

ci: test suite needs ~40 min against a 30-min ceiling — cache the Scanner instead of rebuilding it per test case (S-01)

2 participants