ci: check that every test runs in exactly one group

.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: {}

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

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_<letter>=` lines
+//! This parses the `binary(...)` names out of the `sim_<name>=` 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,51 +39,66 @@ fn suites_on_disk() -> Vec<String> {
     found
 }
 
-/// The suites each `sim_<letter>=` line names, by letter.
-fn named_by_letter(script: &str) -> BTreeMap<char, Vec<String>> {
+/// The suites each `sim_<name>=` line names, by name.
+fn named_by_group(script: &str) -> BTreeMap<String, Vec<String>> {
     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();
             let inner = term
                 .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<char> = None;
-    let mut case: Option<char> = 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<String> {
+    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<String> = None;
+    let mut case: Option<String> = 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-<letter> case");
+            let name = case
+                .clone()
+                .expect("the catch-all filter sits inside a native-sim-<name> 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<String, Vec<char>> = BTreeMap::new();
+    let mut groups: BTreeMap<String, Vec<String>> = 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<String> {
+    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<String> {
+    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"
+        );
+    }
+}

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 <group>
+#   scripts/test-group.sh --list <group>
+#   scripts/test-group.sh --check
+#
+# --list prints the tests a group selects, one `<binary-id> <test>` 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