From aa21bef898a65d8e24f7ebff04cc4340cfd0cfc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 13 Jul 2026 07:27:46 +0200 Subject: [PATCH 1/2] ci: run the integration suites a PR's diff names (#5960) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-PR `cargo-test` gate runs `--lib --bins` only, so no `tests/*.rs` integration suite has ever executed on a pull request — including the PR's own new acceptance suite. #5938 landed with `capture_rereg_renamed_class.rs` red at the very commit that introduced it, through green required checks. New `e2e-scoped` job (PR-only) + `scripts/ci_e2e_scope.py`: map the changed file list to the suites it names and run exactly those. crates//tests/.rs -> cargo test -p --test crates//tests//mod.rs -> the parent suite crates//tests/common/... -> every suite in that crate (capped) Deleted suites are skipped, cross-host UI crates are excluded (shared `EXCLUDED` with ci_test_scope), and the selection is capped at 12 so a mass rename can't turn one PR into a multi-hour run. Manifest names are parsed straight out of Cargo.toml, not `cargo metadata`, so the scope step needs no toolchain: a PR that names no suite finishes in ~20-30s and installs nothing. When a suite IS named, the build runs in parallel with cargo-test (the longer pole), so added wall-clock on the critical path is ~0 for most PRs. Also: the concurrency group now keys on the event and only cancels in-progress runs for `pull_request`. The nightly full run is the only backstop for a regression in a suite the diff doesn't name (#6037 class), so it must always reach a conclusion; keeping it cancellable by a manual dispatch on main was a loose end. Superseded PR pushes still cancel, which is where the minutes are. Self-test: `python3 scripts/ci_e2e_scope.py --self-test` (runs in the job). --- .github/workflows/test.yml | 157 +++++++++++++++++++++++++-- scripts/ci_e2e_scope.py | 213 +++++++++++++++++++++++++++++++++++++ 2 files changed, 364 insertions(+), 6 deletions(-) create mode 100755 scripts/ci_e2e_scope.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a6133d8be..84dd6deaa 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -37,9 +37,17 @@ on: type: boolean default: false +# Superseded PR pushes cancel their in-flight run (the real minute saver); the +# nightly cron and release-tag runs never do (#5960). Keying the group on the +# event as well as the ref means a manual `workflow_dispatch` on `main` — the +# only other run that shares `refs/heads/main` with the cron, since pushes to +# main don't trigger this workflow — can't cancel a nightly mid-flight, and +# `cancel-in-progress` is off for those events regardless. The nightly is the +# only backstop for integration-suite regressions a scoped PR run can't see, so +# it must always reach a conclusion. concurrency: - group: test-${{ github.ref }} - cancel-in-progress: true + group: test-${{ github.event_name }}-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} env: CARGO_TERM_COLOR: always @@ -392,10 +400,16 @@ jobs: # ---- FAST per-PR run (<10 min target) ---- # Unit / lib / bin tests for the affected crates only. The slow # auto-optimize *integration* tests (tests/*.rs — each shells out to - # `perry compile`, ~4–6 min apiece, ~30 of them) are NOT run per-PR; - # they run in the nightly full job, on release tags, and on demand - # via the `run-extended-tests` label. No staticlib dependency to - # build (no integration tests). + # `perry compile`, ~4–6 min apiece, 163 of them in crates/perry + # alone) are NOT run wholesale per-PR; they run in the nightly full + # job, on release tags, and on demand via the `run-extended-tests` + # label. No staticlib dependency to build here (no integration + # tests). + # + # #5960: the suites the DIFF NAMES — i.e. a PR's own new/edited + # acceptance suite — do run per-PR, in the separate `e2e-scoped` + # job below. Without it a PR's acceptance test could not fail its + # own CI (#5938). # # Run each crate in its OWN `cargo test` invocation — NOT one # multi-package invocation. Building several crates together unifies @@ -431,6 +445,137 @@ jobs: done fi + # --------------------------------------------------------------------------- + # Scoped e2e: run the integration suites NAMED BY THE DIFF (#5960) + # + # The per-PR `cargo-test` gate is `--lib --bins` only, so *no* `tests/*.rs` + # integration suite runs on a PR — including the PR's own. That is how #5938 + # landed with its acceptance suite (`capture_rereg_renamed_class.rs`) red + # through green required checks: the suite was in the diff, was compiled into + # the scope listing, and was never executed. Running every suite per-PR is not + # an option (163 in `crates/perry` alone, each shelling out to `perry + # compile`), so this tier runs exactly the ones the diff names: + # + # changed `crates//tests/.rs` -> cargo test -p --test + # + # (plus suite module dirs and `common/` helper dirs — see + # scripts/ci_e2e_scope.py, capped at 12 suites). + # + # Cost: PRs that touch no integration suite — the large majority — finish in + # ~20-30s of checkout + scope computation and never install a toolchain or + # build anything (the scope step parses Cargo.toml directly, no `cargo + # metadata`). Only a PR that adds or edits a suite pays the build, and it pays + # it in PARALLEL with `cargo-test`, which is the longer pole anyway. + # + # NOT covered: a source change that regresses an *existing* suite the diff + # doesn't name (#6037 class). There is no coverage data to map that with, and + # a crate-level map (perry-codegen -> all of perry's suites) is precisely the + # full run we're avoiding. The nightly full `cargo test` remains the backstop + # for that class — hence the concurrency carve-out above. + # --------------------------------------------------------------------------- + e2e-scoped: + # PR-only: tags / nightly / workflow_dispatch already run every suite in the + # full `cargo-test` path. + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + timeout-minutes: 90 + env: + RUSTC_WRAPPER: sccache + SCCACHE_GHA_ENABLED: "false" + SCCACHE_DIR: ${{ github.workspace }}/.sccache + SCCACHE_CACHE_SIZE: "12G" + CARGO_INCREMENTAL: "0" + steps: + - uses: actions/checkout@v7 + + # Cheap gate: no toolchain, no cargo, no cache restore. Every step below + # is skipped when the diff names no suite. + - name: Compute e2e suite scope + id: scope + env: + GH_TOKEN: ${{ github.token }} + run: | + python3 scripts/ci_e2e_scope.py --self-test + changed_files="$(gh pr view "${{ github.event.pull_request.number }}" \ + --json files --jq '.files[].path')" + suites="$(printf '%s\n' "$changed_files" | python3 scripts/ci_e2e_scope.py)" + if [ -z "$suites" ]; then + echo "No integration suite named by this diff — nothing to run." + echo "suites=" >> "$GITHUB_OUTPUT" + else + echo "Integration suites in scope:" + printf '%s\n' "$suites" + { + echo 'suites<> "$GITHUB_OUTPUT" + fi + + - name: Install Rust toolchain + if: steps.scope.outputs.suites != '' + uses: dtolnay/rust-toolchain@stable + + - name: Install sccache + if: steps.scope.outputs.suites != '' + uses: mozilla-actions/sccache-action@v0.0.10 + + - name: Cache sccache objects + if: steps.scope.outputs.suites != '' + uses: actions/cache@v6 + with: + path: ${{ github.workspace }}/.sccache + key: sccache-${{ runner.os }}-perry-${{ github.job }}-${{ github.run_id }} + restore-keys: | + sccache-${{ runner.os }}-perry- + + - uses: Swatinem/rust-cache@v2 + if: steps.scope.outputs.suites != '' + with: + shared-key: "${{ runner.os }}-perry" + save-if: ${{ github.ref == 'refs/heads/main' }} + + # Same reasoning as cargo-test: a rust-cache-restored auto-opt archive dir + # keys only on perry-runtime's source hash and would link stale ext + # archives into the binaries these suites compile (#5892). + - name: Evict stale auto-opt archives (#5892) + if: steps.scope.outputs.suites != '' + run: rm -rf target/perry-auto-* target/debug/libperry_ext_*.a 2>/dev/null || true + + - name: Run scoped integration suites + if: steps.scope.outputs.suites != '' + env: + # lld has repeatedly SIGBUS'd large test links on the shared runner. + CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUSTFLAGS: "-C linker-features=-lld" + CARGO_PROFILE_TEST_DEBUG: "0" + CARGO_PROFILE_DEV_DEBUG: "0" + # Bound the heavy per-binary runtime link so the runner doesn't OOM. + CARGO_BUILD_JOBS: "1" + SUITES: ${{ steps.scope.outputs.suites }} + run: | + # `cargo test` never builds the `staticlib` crate-type, so + # libperry_{runtime,stdlib}.a don't exist unless built explicitly — + # and the suites that compile with PERRY_NO_AUTO_OPTIMIZE=1 link them + # directly ("Could not find libperry_runtime.a" otherwise). + if printf '%s\n' "$SUITES" | grep -qE '^(perry|perry-stdlib) '; then + cargo build -p perry-runtime -p perry-stdlib \ + -p perry-runtime-static -p perry-stdlib-static + fi + + status=0 + while read -r package suite; do + [ -n "$package" ] || continue + echo "::group::cargo test -p $package --test $suite" + # Per-suite wall-clock bound: a hung compile must not eat the whole + # job budget and hide the other suites' results. + if ! timeout 1500 cargo test -p "$package" --test "$suite"; then + echo "::error::integration suite failed: $package --test $suite" + status=1 + fi + echo "::endgroup::" + done <<< "$SUITES" + exit "$status" + # --------------------------------------------------------------------------- # GC write-barrier stress (optional / non-blocking) # diff --git a/scripts/ci_e2e_scope.py b/scripts/ci_e2e_scope.py new file mode 100755 index 000000000..9191ac336 --- /dev/null +++ b/scripts/ci_e2e_scope.py @@ -0,0 +1,213 @@ +#!/usr/bin/env python3 +"""Compute which integration-test suites a CI run must execute for a diff. + +The per-PR `cargo-test` gate runs `--lib --bins` only (see +`scripts/ci_test_scope.py` and the `cargo-test` job in `.github/workflows/ +test.yml`): the `crates//tests/*.rs` integration suites are NEVER executed +on a pull request. They each shell out to `perry compile` (~1-6 min apiece, +163 of them in `crates/perry` alone), so running them all per-PR is off the +table — but the consequence was that **a PR's own new acceptance suite could +not fail its own CI** (#5960; #5938 landed with `capture_rereg_renamed_class.rs` +red through green required checks). + +This script closes that hole by scoping the e2e tier to the diff: it reads the +changed file paths (one per line) on stdin and prints the suites the diff names, +one ` ` pair per line, for `cargo test -p --test +`. + +Selection rules (a suite is a `tests/*.rs` target of a workspace crate): + * `crates//tests/.rs` — the direct case: an added or modified + acceptance suite runs. Deleted files are skipped (the target is gone). + * `crates//tests//.rs` — a *module directory* of a suite + (e.g. `perry-codegen/tests/native_proof_regressions/invalidation.rs`, which + `native_proof_regressions.rs` declares with `mod`): selects ``. + * `crates//tests//...` where no `.rs` suite exists (e.g. + a `common/` helper module or a `fixtures/` data dir): every suite in that + crate can be affected, so all of them are selected. + * Everything else selects nothing. In particular a plain `src/` change does + NOT map to suites: there is no coverage data to map it with, and a + crate-level map (`perry-codegen` -> all 163 `perry` suites) is exactly the + full run this scoping exists to avoid. The nightly full `cargo test` stays + the backstop for regressions in suites the diff does not name. + +Cross-host UI crates that don't build on the Linux CI image are excluded +(shared with `ci_test_scope.EXCLUDED`). + +The result is capped (`--cap`, default 12) so a mass rename of suites can't turn +one PR into a multi-hour integration run; the overflow is reported and left to +the nightly. + +Usage: | python3 scripts/ci_e2e_scope.py [--cap N] + python3 scripts/ci_e2e_scope.py --self-test +""" +import os +import re +import sys +import tempfile + +sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) +from ci_test_scope import EXCLUDED # noqa: E402 (same-dir helper) + +DEFAULT_CAP = 12 + +_TESTS_PATH = re.compile(r"^crates/([^/]+)/tests/(.+)$") +_PKG_NAME = re.compile(r'^\s*name\s*=\s*"([^"]+)"', re.M) + + +def _repo_root() -> str: + return os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + + +def _package_name(root: str, crate_dir: str): + """Package name from `crates//Cargo.toml`, or None. + + Parsed directly instead of via `cargo metadata` so the CI scope step can run + before any Rust toolchain is installed — PRs that name no suite must not pay + for a toolchain at all. + """ + manifest = os.path.join(root, "crates", crate_dir, "Cargo.toml") + try: + with open(manifest, encoding="utf-8") as fh: + text = fh.read() + except OSError: + return None + m = _PKG_NAME.search(text) + return m.group(1) if m else None + + +def _suites_of(root: str, crate_dir: str): + """Every `tests/*.rs` integration target of a crate, by suite name.""" + tests_dir = os.path.join(root, "crates", crate_dir, "tests") + try: + entries = os.listdir(tests_dir) + except OSError: + return [] + return sorted( + e[:-3] + for e in entries + if e.endswith(".rs") and os.path.isfile(os.path.join(tests_dir, e)) + ) + + +def select(changed, root: str): + """-> sorted list of (package, suite) named by the changed paths.""" + selected = set() + for path in changed: + m = _TESTS_PATH.match(path.strip()) + if not m: + continue + crate_dir, rest = m.group(1), m.group(2) + pkg = _package_name(root, crate_dir) + if pkg is None or pkg in EXCLUDED: + continue + + if "/" not in rest: + if not rest.endswith(".rs"): + continue + suite = rest[:-3] + # Skip deletions: the target no longer exists. + if os.path.isfile(os.path.join(root, "crates", crate_dir, "tests", rest)): + selected.add((pkg, suite)) + continue + + head = rest.split("/", 1)[0] + sibling = os.path.join(root, "crates", crate_dir, "tests", head + ".rs") + if os.path.isfile(sibling): + # Module directory of `.rs`. + selected.add((pkg, head)) + else: + # Shared helper / fixture dir (`common/`, `fixtures/`): any suite in + # the crate can depend on it. + for suite in _suites_of(root, crate_dir): + selected.add((pkg, suite)) + return sorted(selected) + + +def _self_test() -> int: + with tempfile.TemporaryDirectory() as root: + def touch(rel, body=""): + full = os.path.join(root, rel) + os.makedirs(os.path.dirname(full), exist_ok=True) + with open(full, "w", encoding="utf-8") as fh: + fh.write(body) + + touch("crates/perry/Cargo.toml", '[package]\nname = "perry"\n') + touch("crates/perry/tests/issue_5024_proto.rs") + touch("crates/perry/tests/other_suite.rs") + touch("crates/perry-codegen/Cargo.toml", '[package]\nname = "perry-codegen"\n') + touch("crates/perry-codegen/tests/native_proof_regressions.rs") + touch("crates/perry-codegen/tests/native_proof_regressions/invalidation.rs") + touch("crates/perry-cc/Cargo.toml", '[package]\nname = "perry-cc"\n') + touch("crates/perry-cc/tests/alpha.rs") + touch("crates/perry-cc/tests/beta.rs") + touch("crates/perry-cc/tests/common/mod.rs") + touch("crates/perry-ui-ios/Cargo.toml", '[package]\nname = "perry-ui-ios"\n') + touch("crates/perry-ui-ios/tests/ui.rs") + + cases = [ + # direct suite change + (["crates/perry/tests/issue_5024_proto.rs"], [("perry", "issue_5024_proto")]), + # source-only change selects nothing + (["crates/perry/src/main.rs", "CHANGELOG.md"], []), + # deleted suite is skipped (no file on disk) + (["crates/perry/tests/deleted_suite.rs"], []), + # module dir of a suite maps to the suite + ( + ["crates/perry-codegen/tests/native_proof_regressions/invalidation.rs"], + [("perry-codegen", "native_proof_regressions")], + ), + # shared helper dir selects every suite in that crate + ( + ["crates/perry-cc/tests/common/mod.rs"], + [("perry-cc", "alpha"), ("perry-cc", "beta")], + ), + # cross-host UI crates are excluded + (["crates/perry-ui-ios/tests/ui.rs"], []), + # dedup across several paths of the same suite + ( + [ + "crates/perry/tests/other_suite.rs", + "crates/perry/tests/other_suite.rs", + "crates/perry/tests/issue_5024_proto.rs", + ], + [("perry", "issue_5024_proto"), ("perry", "other_suite")], + ), + # unknown crate dir + (["crates/nope/tests/x.rs"], []), + ] + for changed, expected in cases: + got = select(changed, root) + if got != expected: + print(f"self-test FAILED for {changed}: {got} != {expected}", file=sys.stderr) + return 1 + print("ci_e2e_scope self-test: ok") + return 0 + + +def main() -> int: + if "--self-test" in sys.argv: + return _self_test() + + cap = DEFAULT_CAP + if "--cap" in sys.argv: + cap = int(sys.argv[sys.argv.index("--cap") + 1]) + + changed = [line.strip() for line in sys.stdin if line.strip()] + pairs = select(changed, _repo_root()) + + if len(pairs) > cap: + print( + f"::notice::{len(pairs)} integration suites named by this diff exceeds the " + f"cap of {cap}; running the first {cap} (sorted). The rest are covered by " + f"the nightly full cargo-test.", + file=sys.stderr, + ) + pairs = pairs[:cap] + + for pkg, suite in pairs: + print(f"{pkg} {suite}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From 469c8bb671ceca93a1bbea6a2a45e445bc3de357 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Mon, 13 Jul 2026 14:20:30 +0200 Subject: [PATCH 2/2] ci: e2e-scoped job timeout must cover its own per-suite bound MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The job capped out at 90 minutes while allowing up to 12 suites (DEFAULT_CAP) each bounded by `timeout 1500` (25 min) — a 300-minute worst case. A legitimately-slow selection would hit the job wall clock and GitHub would cancel the whole job, losing every suite's result behind an uninformative "cancelled" instead of the per-suite ::error:: — reintroducing, one level up, exactly the failure mode the per-suite timeout exists to prevent. Derived the job timeout from the two bounds it has to cover (12 x 25 + 30 for toolchain/cache/staticlib build = 330) and documented the invariant so the three numbers can't drift apart. It's a backstop, not a budget: each suite is independently bounded and the common case selects zero suites and exits in ~20-30s. Also drop credential persistence from this job's checkout — it compiles third-party crates (build scripts run) and nothing in it pushes back to the repo. Found by review. --- .github/workflows/test.yml | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 84dd6deaa..b3329132b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -478,7 +478,17 @@ jobs: # full `cargo-test` path. if: github.event_name == 'pull_request' runs-on: ubuntu-latest - timeout-minutes: 90 + # MUST cover the worst case the per-suite bound allows, or the job wall + # clock kills legitimately-running suites and we get an uninformative + # "cancelled" instead of the per-suite `::error::` — reintroducing, one + # level up, exactly what `timeout 1500` below exists to prevent. + # DEFAULT_CAP (ci_e2e_scope.py) = 12 suites + # per-suite bound below = timeout 1500s = 25 min + # 12 x 25 = 300, + 30 min for toolchain/cache/staticlib build = 330 + # Keep these three in sync. This is a backstop, not a budget: each suite is + # independently bounded, and the common case selects zero suites and exits + # in ~20-30s. + timeout-minutes: 330 env: RUSTC_WRAPPER: sccache SCCACHE_GHA_ENABLED: "false" @@ -486,7 +496,11 @@ jobs: SCCACHE_CACHE_SIZE: "12G" CARGO_INCREMENTAL: "0" steps: + # Nothing here pushes back to the repo, and the job compiles third-party + # crates (build scripts run), so don't leave a git credential on disk. - uses: actions/checkout@v7 + with: + persist-credentials: false # Cheap gate: no toolchain, no cargo, no cache restore. Every step below # is skipped when the diff names no suite.