From f18674dfd957bfd5f30e18b293242960e2d260cd Mon Sep 17 00:00:00 2001 From: Marcus Kainth Date: Fri, 25 Sep 2026 11:14:59 +0100 Subject: [PATCH 1/5] ci: build the test archives without debug info The workspace archive was 2.63 GB (2,631,156,768 bytes), almost all of it DWARF: a driver test binary is about 465 MB with full debug info and about 9 MB without. Uploading it took 78 to 83 s and a test job's download took 33 to 363 s. With CARGO_PROFILE_TEST_DEBUG=0 the same archive is 187 MB. Line tables only (line-tables-only) would keep 860 MB. A panic's message and its file and line come from the panic location, which does not depend on debug info, so a failing test reports the same thing it reports now. The variable is set on the job so rust-cache includes it in its key; the dependencies it restores were built under the same setting. --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c9679ce..8b558d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -110,6 +110,11 @@ jobs: runs-on: ubuntu-latest permissions: contents: read + env: + # The test binaries carry no debug info. A failing test still reports + # its panic message with the file and line; a backtrace names + # functions only. Set for the job so rust-cache keys on it. + CARGO_PROFILE_TEST_DEBUG: "0" steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: From 247ad374cb9f0a5abb60e70424d65aa417420bca Mon Sep 17 00:00:00 2001 From: Marcus Kainth Date: Fri, 25 Sep 2026 11:16:22 +0100 Subject: [PATCH 2/5] test: build the native crate's suites unoptimised The test profile's opt-level 2 is there for the emulator's fixture and trace suites, which run millions of instructions in Rust. The native crate builds SQL text and its suites spend their time waiting on the server, so it gets opt-level 0 through a package override. Every other crate keeps opt-level 2, the reference emulator included. On a standard runner the native crate's 57 test binaries took 876 CPU seconds to compile at opt-level 2 and 200 at opt-level 0 (cargo --timings, CI run 36121287631). Its non-live suites ran in 0.87 s at opt-level 2 and 1.98 s at opt-level 0 locally. opt-level does not change what safe Rust computes. The generated text is byte-identical anyway: xxh64 of both resident statements, run_statement, demo_statement, the load, level and render plans and the parity queries match at opt-level 2, 1 and 0. --- Cargo.toml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Cargo.toml b/Cargo.toml index 64335ea..f0b402e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -52,3 +52,8 @@ codegen-units = 1 # the tests would otherwise report as a wrong hash. [profile.test] opt-level = 2 + +# The native crate builds SQL text and its suites wait on the server, so +# they build unoptimised. +[profile.test.package.clickdoom-native] +opt-level = 0 From 0ac7839f06787d5cc857294ca12796d50ac7696e Mon Sep 17 00:00:00 2001 From: Marcus Kainth Date: Fri, 25 Sep 2026 11:30:27 +0100 Subject: [PATCH 3/5] ci: split the test archive by crate build-tests made one archive holding every crate's test binaries and one holding the reference emulator's release ROM suites, one after the other, and every test job downloaded both. A simulation group needs only the native crate's suites. build-tests is now a matrix of two archives built side by side: native.tar.zst with the native crate's suites and workspace.tar.zst with every other crate's. The ROM suites archive moves to build-refemu, which already builds the reference emulator in release and is off the path to the test jobs. Each group downloads only what scripts/test-group.sh runs it from: the simulation groups the native archive, the emulator group the workspace and ROM suites archives, and native-rest both, since it runs the native crate's other suites and the driver's native_* suites. native-rest runs them as two nextest invocations, one per archive, at the same two threads. Measured on the throwaway run 36122853256 with a warm cache, no debug info and the native crate's suites at opt-level 0, the native archive built in 48 s (119 MB) and the other crates' in 94 s (72 MB), against 2 m 16 s (206 MB) for the single archive under the same settings. Each group selects the same tests as before: listing every group's filter over the workspace before the split and over the two archives after gives the same test lists for all eight groups, 953 tests in all, none in two groups. --- .github/workflows/ci.yml | 70 ++++++++++++++++++++++++++++++++-------- scripts/test-group.sh | 40 ++++++++++++++--------- 2 files changed, 81 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8b558d4..5043b8e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -95,18 +95,33 @@ jobs: with: persist-credentials: false - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + - uses: taiki-e/install-action@3f74d7c16a4242f1c95561e98edc25d36adb4375 # v2.87.12 + with: + tool: cargo-nextest@0.9.140 - name: Build the reference emulator run: cargo build --locked --release -p refemu + - name: Archive the reference emulator's ROM suites + # In release, the build they need. The `emulator` group runs them. + run: | + mkdir -p target/nextest + cargo nextest archive --locked --release -p refemu --features rom-tests \ + --archive-file target/nextest/rom-suites.tar.zst - name: Upload refemu artifact uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: refemu path: target/release/refemu + - name: Upload the ROM suites archive + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: rom-suites + path: target/nextest/rom-suites.tar.zst build-tests: - # Every test binary, compiled once into two nextest archives: the - # workspace with the live suites, and the reference emulator's release - # ROM suites. The test jobs run from the archives and compile nothing. + # The workspace's test binaries with the live suites, compiled once into + # two nextest archives, one per matrix entry: the native crate's suites, + # and every other crate's. The test jobs run from the archives and + # compile nothing. runs-on: ubuntu-latest permissions: contents: read @@ -115,30 +130,44 @@ jobs: # its panic message with the file and line; a backtrace names # functions only. Set for the job so rust-cache keys on it. CARGO_PROFILE_TEST_DEBUG: "0" + strategy: + fail-fast: false + matrix: + include: + - archive: native + packages: -p clickdoom-native + - archive: workspace + packages: --workspace --exclude clickdoom-native + name: build-tests (${{ matrix.archive }}) steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + with: + key: ${{ matrix.archive }} - uses: taiki-e/install-action@3f74d7c16a4242f1c95561e98edc25d36adb4375 # v2.87.12 with: tool: cargo-nextest@0.9.140 - name: Archive the suites + env: + ARCHIVE: ${{ matrix.archive }} + PACKAGES: ${{ matrix.packages }} run: | mkdir -p target/nextest - cargo nextest archive --locked --workspace --features clickhouse-tests \ - --archive-file target/nextest/tests.tar.zst - cargo nextest archive --locked --release -p refemu --features rom-tests \ - --archive-file target/nextest/rom-suites.tar.zst - - name: Upload the archives + # shellcheck disable=SC2086 # $PACKAGES is a list of flags + cargo nextest archive --locked $PACKAGES --features clickhouse-tests \ + --archive-file "target/nextest/$ARCHIVE.tar.zst" + - name: Upload the archive uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: test-archives - path: target/nextest/*.tar.zst + name: ${{ matrix.archive }}-tests + path: target/nextest/${{ matrix.archive }}.tar.zst test: # Every suite, in the groups scripts/test-group.sh names, one runner per - # group with its own ClickHouse, run from the archives build-tests made. + # group with its own ClickHouse, run from the archives build-tests and + # build-refemu made. # Throwaway databases only; never `clickdoom`. `make test` runs the same # suites in one pass locally. needs: [build-tests, build-rom, build-refemu] @@ -176,10 +205,25 @@ jobs: tool: cargo-nextest@0.9.140 - name: Start the pinned ClickHouse run: make up - - name: Download the test archives + # Each group downloads the archives scripts/test-group.sh runs it + # from, and a group that lacks one fails on the missing file. + - name: Download the native crate's suites + if: matrix.group != 'emulator' + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: native-tests + path: target/nextest + - name: Download the other crates' suites + if: matrix.group == 'emulator' || matrix.group == 'native-rest' + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: workspace-tests + path: target/nextest + - name: Download the ROM suites + if: matrix.group == 'emulator' uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: - name: test-archives + name: rom-suites path: target/nextest - name: Download ROM artifact uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 diff --git a/scripts/test-group.sh b/scripts/test-group.sh index 7295497..b6079db 100755 --- a/scripts/test-group.sh +++ b/scripts/test-group.sh @@ -38,25 +38,30 @@ cd "$(dirname "$0")/.." group="${1-}" # With NEXTEST_ARCHIVE_DIR set, the suites come pre-built from -# `cargo nextest archive` (tests.tar.zst for the workspace with the live -# suites, rom-suites.tar.zst for the reference emulator's release ROM -# suites) and nothing is compiled here. Without it, each run builds what it -# needs. +# `cargo nextest archive` and nothing is compiled here: native.tar.zst holds +# the native crate's suites, workspace.tar.zst every other crate's with the +# live suites, and rom-suites.tar.zst the reference emulator's release ROM +# suites. Each group names the archives it runs from. Without it, each run +# builds what it needs. archive="${NEXTEST_ARCHIVE_DIR-}" if [ -n "$archive" ]; then # Extracted over the workspace rather than into a temporary directory: # a test that runs the driver binary reaches it by the path compiled in # at build time, which is the workspace's own target directory. extract=(--workspace-remap . --extract-to . --extract-overwrite) - run() { - cargo nextest run --archive-file "$archive/tests.tar.zst" "${extract[@]}" "$@" + run_native() { + cargo nextest run --archive-file "$archive/native.tar.zst" "${extract[@]}" "$@" + } + run_workspace() { + cargo nextest run --archive-file "$archive/workspace.tar.zst" "${extract[@]}" "$@" } run_rom() { cargo nextest run --archive-file "$archive/rom-suites.tar.zst" "${extract[@]}" "$@" } live="" else - run() { cargo nextest run --locked "$@"; } + run_native() { cargo nextest run --locked "$@"; } + run_workspace() { cargo nextest run --locked "$@"; } run_rom() { cargo nextest run --locked --release -p refemu --features rom-tests "$@"; } live="--workspace --features clickhouse-tests" fi @@ -90,7 +95,7 @@ case "$group" in # them would warm or cool, and the connection suite counts the # server's connections, which a neighbour's session would move. # shellcheck disable=SC2086 # $live is a list of flags or nothing - run $live --test-threads 1 \ + run_workspace $live --test-threads 1 \ -E 'not package(clickdoom-native) and (not binary(/^native_/) or binary(native_connections_live))' # The ROM suites are the reference emulator's, so only it is built # in release. @@ -105,38 +110,41 @@ case "$group" in # measured length. A runner with more cores sets TEST_THREADS higher. native-sim-a) # shellcheck disable=SC2086 - run $live --test-threads "${TEST_THREADS:-4}" \ + run_native $live --test-threads "${TEST_THREADS:-4}" \ -E "package(clickdoom-native) and ($sim_a)" ;; native-sim-b) # shellcheck disable=SC2086 - run $live --test-threads "${TEST_THREADS:-4}" \ + run_native $live --test-threads "${TEST_THREADS:-4}" \ -E "package(clickdoom-native) and ($sim_b)" ;; native-sim-c) # shellcheck disable=SC2086 - run $live --test-threads "${TEST_THREADS:-4}" \ + run_native $live --test-threads "${TEST_THREADS:-4}" \ -E "package(clickdoom-native) and ($sim_c)" ;; native-sim-d) # shellcheck disable=SC2086 - run $live --test-threads "${TEST_THREADS:-4}" \ + run_native $live --test-threads "${TEST_THREADS:-4}" \ -E "package(clickdoom-native) and ($sim_d)" ;; native-sim-e) # shellcheck disable=SC2086 - run $live --test-threads "${TEST_THREADS:-4}" \ + run_native $live --test-threads "${TEST_THREADS:-4}" \ -E "package(clickdoom-native) and binary(/^sim_/) and not ($sim_a | $sim_b | $sim_c | $sim_d | $sim_f)" ;; native-sim-f) # shellcheck disable=SC2086 - run $live --test-threads "${TEST_THREADS:-4}" \ + run_native $live --test-threads "${TEST_THREADS:-4}" \ -E "package(clickdoom-native) and ($sim_f)" ;; native-rest) # shellcheck disable=SC2086 - run $live --test-threads 2 \ - -E '(package(clickdoom-native) and not binary(/^sim_/)) or (package(clickdoom-driver) and binary(/^native_/) and not binary(native_connections_live))' + run_native $live --test-threads 2 \ + -E 'package(clickdoom-native) and not binary(/^sim_/)' + # shellcheck disable=SC2086 + run_workspace $live --test-threads 2 \ + -E 'package(clickdoom-driver) and binary(/^native_/) and not binary(native_connections_live)' ;; *) echo "usage: scripts/test-group.sh emulator|native-sim-a|native-sim-b|native-sim-c|native-sim-d|native-sim-e|native-sim-f|native-rest" >&2 From fed239be759778a1ba28c84df8dae92b6b6cbba6 Mon Sep 17 00:00:00 2001 From: Marcus Kainth Date: Fri, 25 Sep 2026 11:35:12 +0100 Subject: [PATCH 4/5] ci: check that every test runs in exactly one group A group's filter that stops matching some tests leaves them running nowhere, and CI stays green. group_coverage.rs caught that for the simulation suites only, by reading the script's lettered filters. Every other package's tests, and the split of the driver's native_* suites between native-rest and emulator, had no check. scripts/test-group.sh gains two modes. `--list ` prints the tests the group selects, through `cargo nextest list` with the same archives and filters the run uses. `--check` lists every test in the archives (or the workspace, without NEXTEST_ARCHIVE_DIR) and every group, and fails when a test is selected by no group or by two, or when a group selects nothing. A test a group reaches from two archives counts once for that group: four refemu tests in probe_fixture and reference_trace run in both the debug workspace archive and the release ROM suites archive, both in emulator. The new test-groups job runs --check against the three archives, and ci-passed needs it. group_coverage.rs also checks that ci.yml's test matrix, the script's case arms and its new `groups=(...)` line name the same groups, and it reads group names of any length rather than one letter. --- .github/workflows/ci.yml | 29 ++++++- DEVELOPING.md | 5 +- native/tests/group_coverage.rs | 140 ++++++++++++++++++++++++--------- scripts/test-group.sh | 72 +++++++++++++++-- 4 files changed, 202 insertions(+), 44 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5043b8e..4a2971a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -246,6 +246,33 @@ jobs: chmod +x target/release/refemu scripts/test-group.sh "$GROUP" + test-groups: + # Every test in the archives is selected by exactly one group of the + # test matrix, and every group selects at least one test. Lists the + # tests and runs none, so it needs no ClickHouse. + needs: [build-tests, build-refemu] + runs-on: ubuntu-latest + permissions: + contents: read + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: taiki-e/install-action@3f74d7c16a4242f1c95561e98edc25d36adb4375 # v2.87.12 + with: + tool: cargo-nextest@0.9.140 + - name: Download the archives + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + pattern: "*-{tests,suites}" + merge-multiple: true + path: target/nextest + - name: Every test in exactly one group + env: + NEXTEST_ARCHIVE_DIR: target/nextest + run: scripts/test-group.sh --check + native-smoke: # The driver's own path to a frame: load the schema and the committed # probe fixture, render the frame the fixture's metadata names, compare @@ -322,7 +349,7 @@ jobs: # result, which is a failure if any group failed. A job added to this # workflow goes in `needs`, or a branch rule that requires this check does # not wait for it. - needs: [lint, build-rom, build-refemu, build-tests, test, native-smoke, differential-smoke] + needs: [lint, build-rom, build-refemu, build-tests, test, test-groups, native-smoke, differential-smoke] if: always() runs-on: ubuntu-latest permissions: {} diff --git a/DEVELOPING.md b/DEVELOPING.md index 55b3176..565c4f6 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -52,7 +52,10 @@ What `make help` does not say: lists the groups. One of them takes every simulation suite the others do not name, so a suite added to the tree runs there until somebody packs it; `native/tests/group_coverage.rs` fails if that group stops existing, - or if two groups name the same suite. `make test-group + if two groups name the same suite, or if the CI matrix and the script + name different groups. `scripts/test-group.sh --check`, which CI runs + against the archives, fails if any test in any package is in no group or + in two, or if a group selects nothing. `make test-group GROUP=native-sim-a` runs one group the way CI does, building what it needs (`cargo install cargo-nextest --locked` first); `make test` runs every suite in one pass. Two repository diff --git a/native/tests/group_coverage.rs b/native/tests/group_coverage.rs index 6f04dba..519069b 100644 --- a/native/tests/group_coverage.rs +++ b/native/tests/group_coverage.rs @@ -1,13 +1,15 @@ -//! Every simulation suite is selected by exactly one CI group. +//! Every simulation suite is selected by exactly one CI group, and CI runs +//! every group the script defines. //! -//! `scripts/test-group.sh` splits the simulation suites across the lettered -//! groups by name, and one group takes everything the others do not name. -//! A suite named by no group would run nowhere and CI would stay green -//! without it; a suite named by two would run twice and cost a group its -//! budget. Neither shows up in a run's own output, so this reads the -//! script's own filters and the suites on disk and says which. +//! `scripts/test-group.sh` splits the simulation suites across the named +//! groups, and one group takes everything the others do not name. A suite +//! named by no group would run nowhere and CI would stay green without it; +//! a suite named by two would run twice and cost a group its budget. +//! Neither shows up in a run's own output, so this reads the script's own +//! filters and the suites on disk and says which. `test-group.sh --check` +//! makes the same check over the built archives, for every package. //! -//! This parses the `binary(...)` names out of the `sim_=` lines +//! This parses the `binary(...)` names out of the `sim_=` lines //! rather than evaluating a filterset, so a filter that stops being a //! plain list of `binary()` terms fails here rather than being read wrong. @@ -37,18 +39,18 @@ fn suites_on_disk() -> Vec { found } -/// The suites each `sim_=` line names, by letter. -fn named_by_letter(script: &str) -> BTreeMap> { +/// The suites each `sim_=` line names, by name. +fn named_by_group(script: &str) -> BTreeMap> { let mut named = BTreeMap::new(); for line in script.lines() { - let Some(rest) = line.strip_prefix("sim_") else { + let Some((name, body)) = line + .strip_prefix("sim_") + .and_then(|rest| rest.split_once('=')) + .filter(|(name, _)| name.chars().all(|c| c.is_ascii_alphanumeric())) + else { continue; }; - let mut chars = rest.chars(); - let (Some(letter), Some('=')) = (chars.next(), chars.next()) else { - continue; - }; - let body = rest[2..].trim_matches('\''); + let body = body.trim_matches('\''); let mut suites = Vec::new(); for term in body.split('|') { let term = term.trim(); @@ -56,32 +58,47 @@ fn named_by_letter(script: &str) -> BTreeMap> { .strip_prefix("binary(") .and_then(|t| t.strip_suffix(')')) .unwrap_or_else(|| { - panic!("sim_{letter} holds {term}, which is not a plain binary() term") + panic!("sim_{name} holds {term}, which is not a plain binary() term") }); suites.push(inner.to_owned()); } - named.insert(letter, suites); + named.insert(name.to_owned(), suites); } - assert!(!named.is_empty(), "the script names no lettered group"); + assert!(!named.is_empty(), "the script names no simulation group"); named } -/// The letter whose case runs everything the others do not name. -fn catch_all(script: &str) -> char { - let mut found: Option = None; - let mut case: Option = None; +/// The label of each `case` arm that names a group, in order. Arms for +/// options (`--check`) and the fallback (`*`) are left out. +fn case_labels(script: &str) -> Vec { + script + .lines() + .filter_map(|line| { + let label = line.strip_prefix(" ")?.strip_suffix(')')?; + (label.starts_with(|c: char| c.is_ascii_alphanumeric()) + && label.chars().all(|c| c.is_ascii_alphanumeric() || c == '-')) + .then(|| label.to_owned()) + }) + .collect() +} + +/// The name whose case runs everything the others do not name. +fn catch_all(script: &str) -> String { + let mut found: Option = None; + let mut case: Option = None; for line in script.lines() { - if let Some(rest) = line - .trim() - .strip_prefix("native-sim-") - .filter(|rest| rest.trim_end().ends_with(')')) + if let Some(name) = line + .strip_prefix(" native-sim-") + .and_then(|rest| rest.strip_suffix(')')) { - case = rest.chars().next(); + case = Some(name.to_owned()); } if line.contains("binary(/^sim_/) and not (") { - let letter = case.expect("the catch-all filter sits inside a native-sim- case"); + let name = case + .clone() + .expect("the catch-all filter sits inside a native-sim- case"); assert!(found.is_none(), "two groups claim to be the catch-all"); - found = Some(letter); + found = Some(name); } } found.expect( @@ -93,23 +110,23 @@ fn catch_all(script: &str) -> char { fn every_simulation_suite_runs_in_exactly_one_group() { let script = std::fs::read_to_string(repo_root().join("scripts/test-group.sh")) .expect("the script reads"); - let named = named_by_letter(&script); + let named = named_by_group(&script); let rest = catch_all(&script); assert!( !named.contains_key(&rest), "native-sim-{rest} both names suites and takes the rest", ); - let mut groups: BTreeMap> = BTreeMap::new(); + let mut groups: BTreeMap> = BTreeMap::new(); for suite in suites_on_disk() { groups.entry(suite).or_default(); } - for (letter, suites) in &named { + for (name, suites) in &named { for suite in suites { groups .get_mut(suite) - .unwrap_or_else(|| panic!("sim_{letter} names {suite}, which is not on disk")) - .push(*letter); + .unwrap_or_else(|| panic!("sim_{name} names {suite}, which is not on disk")) + .push(name.clone()); } } @@ -133,3 +150,56 @@ fn every_simulation_suite_runs_in_exactly_one_group() { "every suite is named, so native-sim-{rest} runs nothing and its budget is wasted" ); } + +/// The `groups=(...)` list, which `--check` walks and the usage line prints. +fn listed_groups(script: &str) -> Vec { + let line = script + .lines() + .find_map(|line| line.strip_prefix("groups=(")) + .and_then(|rest| rest.strip_suffix(')')) + .expect("the script lists its groups on one `groups=(...)` line"); + line.split_whitespace().map(str::to_owned).collect() +} + +/// The `group: [...]` list of the test matrix in ci.yml. +fn matrix_groups(workflow: &str) -> Vec { + let line = workflow + .lines() + .find_map(|line| line.trim().strip_prefix("group: [")) + .and_then(|rest| rest.strip_suffix(']')) + .expect("ci.yml's test matrix lists its groups on one `group: [...]` line"); + line.split(',').map(|g| g.trim().to_owned()).collect() +} + +#[test] +fn ci_runs_every_group_the_script_defines() { + let script = std::fs::read_to_string(repo_root().join("scripts/test-group.sh")) + .expect("the script reads"); + let workflow = std::fs::read_to_string(repo_root().join(".github/workflows/ci.yml")) + .expect("the workflow reads"); + + let mut listed = listed_groups(&script); + let mut cases = case_labels(&script); + let mut matrix = matrix_groups(&workflow); + assert!( + listed.len() > 1, + "the groups line was read as {listed:?}, which cannot be right" + ); + for names in [&mut listed, &mut cases, &mut matrix] { + names.sort(); + } + assert_eq!( + cases, listed, + "the script's case arms and its groups line name different groups" + ); + assert_eq!( + matrix, listed, + "ci.yml's test matrix and the script's groups line name different groups" + ); + for (name, _) in named_by_group(&script) { + assert!( + listed.contains(&format!("native-sim-{name}")), + "sim_{name} names suites for native-sim-{name}, which is not a group" + ); + } +} diff --git a/scripts/test-group.sh b/scripts/test-group.sh index b6079db..e94bfec 100755 --- a/scripts/test-group.sh +++ b/scripts/test-group.sh @@ -3,6 +3,13 @@ # Runs one group of the test suites, the way ci.yml runs them in parallel. # # scripts/test-group.sh +# scripts/test-group.sh --list +# scripts/test-group.sh --check +# +# --list prints the tests a group selects, one ` ` per +# line, and runs nothing. --check lists every group and every test the +# suites hold, and fails when a test is in no group or in two, or when a +# group selects nothing. # # Groups, and what they hold: # emulator every suite outside the native crate and the driver's @@ -35,8 +42,32 @@ set -euo pipefail cd "$(dirname "$0")/.." +groups=(emulator native-sim-a native-sim-b native-sim-c native-sim-d native-sim-e native-sim-f native-rest) + +verb=run +if [ "${1-}" = --list ]; then + verb=list + shift +fi group="${1-}" +# `cargo nextest run` with the arguments given, or under --list +# `cargo nextest list` with the same selection. +nextest() { + if [ "$verb" = run ]; then + cargo nextest run "$@" + return + fi + local selection=() + while [ $# -gt 0 ]; do + case "$1" in + --test-threads) shift 2 ;; + *) selection+=("$1"); shift ;; + esac + done + cargo nextest list --message-format oneline "${selection[@]}" +} + # With NEXTEST_ARCHIVE_DIR set, the suites come pre-built from # `cargo nextest archive` and nothing is compiled here: native.tar.zst holds # the native crate's suites, workspace.tar.zst every other crate's with the @@ -50,19 +81,19 @@ if [ -n "$archive" ]; then # at build time, which is the workspace's own target directory. extract=(--workspace-remap . --extract-to . --extract-overwrite) run_native() { - cargo nextest run --archive-file "$archive/native.tar.zst" "${extract[@]}" "$@" + nextest --archive-file "$archive/native.tar.zst" "${extract[@]}" "$@" } run_workspace() { - cargo nextest run --archive-file "$archive/workspace.tar.zst" "${extract[@]}" "$@" + nextest --archive-file "$archive/workspace.tar.zst" "${extract[@]}" "$@" } run_rom() { - cargo nextest run --archive-file "$archive/rom-suites.tar.zst" "${extract[@]}" "$@" + nextest --archive-file "$archive/rom-suites.tar.zst" "${extract[@]}" "$@" } live="" else - run_native() { cargo nextest run --locked "$@"; } - run_workspace() { cargo nextest run --locked "$@"; } - run_rom() { cargo nextest run --locked --release -p refemu --features rom-tests "$@"; } + run_native() { nextest --locked "$@"; } + run_workspace() { nextest --locked "$@"; } + run_rom() { nextest --locked --release -p refemu --features rom-tests "$@"; } live="--workspace --features clickhouse-tests" fi @@ -89,6 +120,33 @@ sim_d='binary(sim_floor_live) | binary(sim_hearing_live) | binary(sim_justattack sim_f='binary(sim_aim_live) | binary(sim_damage_live) | binary(sim_impact_live) | binary(sim_noise_live) | binary(sim_parity_live) | binary(sim_plat_live) | binary(sim_removed_live) | binary(sim_sight_live)' case "$group" in + --check) + listed=$(mktemp -d) + trap 'rm -rf "$listed"' EXIT + verb=list + # shellcheck disable=SC2086 + { run_native $live; run_workspace $live; run_rom; } | sort -u > "$listed/all" + failed=0 + for g in "${groups[@]}"; do + "$0" --list "$g" | sort -u | sed "s|^|$g |" > "$listed/group-$g" + echo "$g: $(wc -l < "$listed/group-$g") tests" + if [ ! -s "$listed/group-$g" ]; then + echo "::error::$g selects no tests" >&2 + failed=1 + fi + done + cat "$listed"/group-* | cut -d' ' -f2- | sort > "$listed/selected" + echo "every test: $(wc -l < "$listed/all"); selected: $(wc -l < "$listed/selected")" + if uniq -d "$listed/selected" | grep .; then + echo "::error::the tests above are selected by more than one group" >&2 + failed=1 + fi + if comm -23 "$listed/all" <(sort -u "$listed/selected") | grep .; then + echo "::error::the tests above are selected by no group" >&2 + failed=1 + fi + exit "$failed" + ;; emulator) # One test at a time: the SQL CPU's suite and the executor's share # the server's compiled-expression cache, which a second run beside @@ -147,7 +205,7 @@ case "$group" in -E 'package(clickdoom-driver) and binary(/^native_/) and not binary(native_connections_live)' ;; *) - echo "usage: scripts/test-group.sh emulator|native-sim-a|native-sim-b|native-sim-c|native-sim-d|native-sim-e|native-sim-f|native-rest" >&2 + echo "usage: scripts/test-group.sh [--list] $(IFS='|'; echo "${groups[*]}") | --check" >&2 exit 2 ;; esac From 59062546a1d84a7f8048e43f677ff340ece054a8 Mon Sep 17 00:00:00 2001 From: Marcus Kainth Date: Fri, 25 Sep 2026 11:41:42 +0100 Subject: [PATCH 5/5] ci: start the simulation groups once the native archive is built Every test job needed every build job: both test archives, the ROM and the reference emulator. A simulation group reads only the native crate's archive, so it waited on the slowest of the others for nothing. The native archive gets its own job, build-native-tests, and build-tests builds the other crates' archive. The simulation groups move to their own matrix job, test-sim, which needs only build-native-tests and downloads only its archive. `test` keeps emulator and native-rest and every build job. ci-passed needs the two new jobs. group_coverage.rs reads the groups of every test matrix in ci.yml, so the check that CI runs every group the script defines covers both jobs. --- .github/workflows/ci.yml | 119 +++++++++++++++++++++++---------- native/tests/group_coverage.rs | 22 ++++-- 2 files changed, 99 insertions(+), 42 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4a2971a..a4fdad9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -117,11 +117,10 @@ jobs: name: rom-suites path: target/nextest/rom-suites.tar.zst - build-tests: - # The workspace's test binaries with the live suites, compiled once into - # two nextest archives, one per matrix entry: the native crate's suites, - # and every other crate's. The test jobs run from the archives and - # compile nothing. + build-native-tests: + # The native crate's test binaries with the live suites, compiled once + # into a nextest archive. The simulation groups run from it alone, so + # they start as soon as it exists. runs-on: ubuntu-latest permissions: contents: read @@ -130,47 +129,59 @@ jobs: # its panic message with the file and line; a backtrace names # functions only. Set for the job so rust-cache keys on it. CARGO_PROFILE_TEST_DEBUG: "0" - strategy: - fail-fast: false - matrix: - include: - - archive: native - packages: -p clickdoom-native - - archive: workspace - packages: --workspace --exclude clickdoom-native - name: build-tests (${{ matrix.archive }}) steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: persist-credentials: false - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 + - uses: taiki-e/install-action@3f74d7c16a4242f1c95561e98edc25d36adb4375 # v2.87.12 + with: + tool: cargo-nextest@0.9.140 + - name: Archive the suites + run: | + mkdir -p target/nextest + cargo nextest archive --locked -p clickdoom-native --features clickhouse-tests \ + --archive-file target/nextest/native.tar.zst + - name: Upload the archive + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: native-tests + path: target/nextest/native.tar.zst + + build-tests: + # Every other crate's test binaries with the live suites, compiled once + # into a nextest archive. Built as build-native-tests is. + runs-on: ubuntu-latest + permissions: + contents: read + env: + CARGO_PROFILE_TEST_DEBUG: "0" + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - key: ${{ matrix.archive }} + persist-credentials: false + - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2.9.2 - uses: taiki-e/install-action@3f74d7c16a4242f1c95561e98edc25d36adb4375 # v2.87.12 with: tool: cargo-nextest@0.9.140 - name: Archive the suites - env: - ARCHIVE: ${{ matrix.archive }} - PACKAGES: ${{ matrix.packages }} run: | mkdir -p target/nextest - # shellcheck disable=SC2086 # $PACKAGES is a list of flags - cargo nextest archive --locked $PACKAGES --features clickhouse-tests \ - --archive-file "target/nextest/$ARCHIVE.tar.zst" + cargo nextest archive --locked --workspace --exclude clickdoom-native \ + --features clickhouse-tests --archive-file target/nextest/workspace.tar.zst - name: Upload the archive uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: - name: ${{ matrix.archive }}-tests - path: target/nextest/${{ matrix.archive }}.tar.zst + name: workspace-tests + path: target/nextest/workspace.tar.zst test: # Every suite, in the groups scripts/test-group.sh names, one runner per - # group with its own ClickHouse, run from the archives build-tests and - # build-refemu made. + # group with its own ClickHouse, run from the archives the build jobs + # made. The simulation groups are test-sim's; this job runs the rest. # Throwaway databases only; never `clickdoom`. `make test` runs the same # suites in one pass locally. - needs: [build-tests, build-rom, build-refemu] + needs: [build-native-tests, build-tests, build-rom, build-refemu] # The repository variables CLICKDOOM_TEST_RUNNER (a runs-on label) and # CLICKDOOM_TEST_THREADS (tests per simulation group at once) pick the # machine the groups run on; unset, a standard runner at four. @@ -194,7 +205,7 @@ jobs: strategy: fail-fast: false matrix: - group: [emulator, native-sim-a, native-sim-b, native-sim-c, native-sim-d, native-sim-e, native-sim-f, native-rest] + group: [emulator, native-rest] name: test (${{ matrix.group }}) steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -208,13 +219,12 @@ jobs: # Each group downloads the archives scripts/test-group.sh runs it # from, and a group that lacks one fails on the missing file. - name: Download the native crate's suites - if: matrix.group != 'emulator' + if: matrix.group == 'native-rest' uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: name: native-tests path: target/nextest - name: Download the other crates' suites - if: matrix.group == 'emulator' || matrix.group == 'native-rest' uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: name: workspace-tests @@ -246,11 +256,50 @@ jobs: chmod +x target/release/refemu scripts/test-group.sh "$GROUP" + test-sim: + # The simulation groups, as `test` runs its groups, from the native + # crate's archive alone. They need no ROM and no reference emulator. + needs: [build-native-tests] + runs-on: ${{ vars.CLICKDOOM_TEST_RUNNER || 'ubuntu-latest' }} + permissions: + contents: read + # The same backstop as `test`. + timeout-minutes: 45 + env: + TEST_THREADS: ${{ vars.CLICKDOOM_TEST_THREADS || '4' }} + strategy: + fail-fast: false + matrix: + group: [native-sim-a, native-sim-b, native-sim-c, native-sim-d, native-sim-e, native-sim-f] + name: test (${{ matrix.group }}) + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + - uses: taiki-e/install-action@3f74d7c16a4242f1c95561e98edc25d36adb4375 # v2.87.12 + with: + tool: cargo-nextest@0.9.140 + - name: Start the pinned ClickHouse + run: make up + - name: Download the native crate's suites + uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 + with: + name: native-tests + path: target/nextest + - name: The group's suites + env: + CLICKHOUSE_HOST: localhost + CLICKHOUSE_HTTP_PORT: "8123" + CLICKHOUSE_PASSWORD: clickdoom + GROUP: ${{ matrix.group }} + NEXTEST_ARCHIVE_DIR: target/nextest + run: scripts/test-group.sh "$GROUP" + test-groups: # Every test in the archives is selected by exactly one group of the # test matrix, and every group selects at least one test. Lists the # tests and runs none, so it needs no ClickHouse. - needs: [build-tests, build-refemu] + needs: [build-native-tests, build-tests, build-refemu] runs-on: ubuntu-latest permissions: contents: read @@ -345,11 +394,11 @@ jobs: ci-passed: # Succeeds only when every other job in this workflow succeeded. A job that - # failed, was skipped or was cancelled fails it. The test matrix reports one - # result, which is a failure if any group failed. A job added to this - # workflow goes in `needs`, or a branch rule that requires this check does - # not wait for it. - needs: [lint, build-rom, build-refemu, build-tests, test, test-groups, native-smoke, differential-smoke] + # failed, was skipped or was cancelled fails it. Each test matrix reports + # one result, which is a failure if any of its groups failed. A job added + # to this workflow goes in `needs`, or a branch rule that requires this + # check does not wait for it. + needs: [lint, build-rom, build-refemu, build-native-tests, build-tests, test, test-sim, test-groups, native-smoke, differential-smoke] if: always() runs-on: ubuntu-latest permissions: {} diff --git a/native/tests/group_coverage.rs b/native/tests/group_coverage.rs index 519069b..d2c5fe9 100644 --- a/native/tests/group_coverage.rs +++ b/native/tests/group_coverage.rs @@ -161,14 +161,22 @@ fn listed_groups(script: &str) -> Vec { line.split_whitespace().map(str::to_owned).collect() } -/// The `group: [...]` list of the test matrix in ci.yml. +/// The groups every `group: [...]` line of ci.yml's test matrices lists. fn matrix_groups(workflow: &str) -> Vec { - let line = workflow + let lines: Vec<&str> = workflow .lines() - .find_map(|line| line.trim().strip_prefix("group: [")) - .and_then(|rest| rest.strip_suffix(']')) - .expect("ci.yml's test matrix lists its groups on one `group: [...]` line"); - line.split(',').map(|g| g.trim().to_owned()).collect() + .filter_map(|line| line.trim().strip_prefix("group: [")) + .map(|rest| { + rest.strip_suffix(']') + .expect("a test matrix lists its groups on one `group: [...]` line") + }) + .collect(); + assert!(!lines.is_empty(), "ci.yml has no `group: [...]` line"); + lines + .iter() + .flat_map(|line| line.split(',')) + .map(|g| g.trim().to_owned()) + .collect() } #[test] @@ -194,7 +202,7 @@ fn ci_runs_every_group_the_script_defines() { ); assert_eq!( matrix, listed, - "ci.yml's test matrix and the script's groups line name different groups" + "ci.yml's test matrices and the script's groups line name different groups" ); for (name, _) in named_by_group(&script) { assert!(