From 29796762ded5e410deae47e46e0e08701b6f444a Mon Sep 17 00:00:00 2001 From: CypherNaugh_0x <9931495+CypherNaught-0x@users.noreply.github.com> Date: Sat, 10 Oct 2026 18:51:18 +0200 Subject: [PATCH] cargo xtask test runs every test binary at once (cargo test runs them one after another): 323 s to 75 s on 12 cores. cargo xtask ci runs the quick gates (fmt, assets, licences, layers) first, then clippy, then the tests with the wasm check and doc tests alongside; the wasm check uses the ci profile to reuse clippy's host build scripts and proc macros. GitHub CI runs its tests through cargo xtask test --- .github/workflows/ci.yml | 3 +- CONTRIBUTING.md | 13 +++-- xtask/src/main.rs | 40 ++++++++++----- xtask/src/test.rs | 105 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 143 insertions(+), 18 deletions(-) create mode 100644 xtask/src/test.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 67671b44..5697ded6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -107,7 +107,8 @@ jobs: - name: Compile tests run: cargo test --workspace --profile ci --locked --no-run - name: Run tests - run: cargo test --workspace --profile ci --locked + # Every test binary at once (cargo test runs them one after another). + run: cargo xtask test --locked # Preserve the existing check name for branch protection, including docs-only PRs. ci: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b06a05fd..ee245cdd 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -18,7 +18,7 @@ build you also need `rustup target add wasm32-unknown-unknown` and [`trunk`](htt ```sh git clone https://github.com/bherbruck/solvecraft && cd solvecraft cargo run --release -p solvecraft -- --sample # desktop app with a sample part -cargo test --workspace --profile ci # unit, engine and UI-scenario tests +cargo xtask test # unit, engine and UI-scenario tests cargo xtask ci # everything a PR must pass (see below) ``` @@ -81,7 +81,7 @@ the most complete description of the house style. | What | Where | Run | |---|---|---| -| Unit and engine tests | next to the code, or `*_tests.rs` files | `cargo test --workspace --profile ci` | +| Unit and engine tests | next to the code, or `*_tests.rs` files | `cargo xtask test` (or `cargo test --workspace --profile ci`) | | UI scenarios | `crates/ui-egui/tests/scenarios/*.json` | part of `cargo test` | | Fusion oracle | `cargo xtask oracle` | needs the local oracle data (see below) | @@ -105,7 +105,10 @@ put `{"pending": "why"}` in the scenario, and remove it in the PR that fixes the **Use the `ci` profile.** CI runs the tests with `--profile ci`: light optimisation, line tables, no LTO, and debug assertions off (truck has debug-only assertions that reject valid geometry). It builds several times faster than `--release`. If a test fails only in a plain debug build, say so -in the PR. +in the PR. `cargo xtask test` builds the tests with it and then runs every test binary at once +(`cargo test` runs them one after another); arguments after `--` go to each binary, e.g. +`cargo xtask test -- shell`. Tests run beside each other on a busy machine, so a test must not +depend on wall-clock timing. **The Fusion oracle** is a set of 76 reference parts, each built in Fusion from a recipe. SolveCraft rebuilds the same recipe and compares volume, area and topology counts with Fusion's @@ -120,8 +123,8 @@ install. If you work on geometry and want to check against it, ask in an issue. comment on the issue so it gets the `in progress` label and two people don't build the same thing. For anything bigger than a bug fix that has no issue yet, open one first. 2. Branch from `main` and keep the PR to one topic. -3. Run `cargo xtask ci` before pushing. It runs fmt, clippy (`-D warnings`), the tests in - release mode, the asset and layer checks, and the wasm build check. A PR has to pass it. +3. Run `cargo xtask ci` before pushing. It runs fmt, the asset, licence and layer checks, + clippy (`-D warnings`), then the tests with the wasm build check alongside. A PR has to pass it. 4. Reference the issue (`Fixes #12`). In the PR, say what you tested and how. For geometry work, include the measured numbers (and the expected ones). diff --git a/xtask/src/main.rs b/xtask/src/main.rs index ae230c60..ccb7090e 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -8,6 +8,7 @@ mod book; mod layers; mod licences; mod parity; +mod test; use std::path::{Path, PathBuf}; use std::process::{Command, ExitCode}; @@ -33,8 +34,12 @@ commands: licences [--check] write THIRD-PARTY-LICENSES.txt (the licences of every shipped crate, embedded in the programs); --check fails when it is out of date + test [CARGO ARGS] [-- TEST ARGS] + build the tests with the ci profile, then run every test binary at once + (and the doc tests alongside) wasm cargo check every library crate and the web app for wasm32-unknown-unknown - ci fmt --check, clippy -D warnings, test, assets, licences --check, layers, wasm (stops at first failure) + ci fmt --check, assets, licences --check, layers, clippy -D warnings, then the tests + with the wasm check alongside (stops at the first failing step) "; fn main() -> ExitCode { @@ -48,6 +53,10 @@ fn main() -> ExitCode { Some("step-corpus") => cmd_step_corpus(rest.iter().find(|a| !a.starts_with("--")).copied(), rest.contains(&"--iges")), Some("book") => book::run(&root(), !rest.contains(&"--no-build")), Some("licences") => licences::run(&root(), rest.contains(&"--check")), + Some("test") => { + let (cargo_args, test_args) = rest.iter().position(|a| *a == "--").map_or((&rest[..], &[][..]), |i| (&rest[..i], &rest[i + 1..])); + test::run(cargo_args, test_args, vec![test::doc_tests(cargo_args, test_args)]) + } Some("ci") => cmd_ci(), Some("wasm") => cmd_wasm(), Some("-h" | "--help" | "help") | None => { @@ -201,18 +210,25 @@ fn cmd_oracle() -> Result<(), String> { /// Never break wasm: every layered crate (everything below the apps) and the web app must /// build for the browser. fn cmd_wasm() -> Result<(), String> { + let (c, n) = wasm_check()?; + run(c, &format!("cargo check --target wasm32-unknown-unknown ({n} crates)"))?; + println!("wasm: {n} crates build for wasm32-unknown-unknown"); + Ok(()) +} + +/// The wasm check and how many crates it covers. It uses the ci profile so the host build +/// scripts and proc macros built for clippy and the tests are reused. +fn wasm_check() -> Result<(Command, usize), String> { let crates = layers::from_metadata(&metadata()?)?; let mut set: Vec = crates.iter().filter(|c| matches!(layers::classify(&c.name), Some(layers::Class::Layer(_)))).map(|c| c.name.clone()).collect(); set.push("solvecraft-web".into()); let mut c = cargo(); - c.args(["check", "--target", "wasm32-unknown-unknown"]); + c.args(["check", "--target", "wasm32-unknown-unknown", "--profile", "ci"]); for p in &set { c.args(["-p", p]); } - run(c, &format!("cargo check --target wasm32-unknown-unknown ({} crates)", set.len()))?; - println!("wasm: {} crates build for wasm32-unknown-unknown", set.len()); - Ok(()) + Ok((c, set.len())) } /// Import every oracle STEP file (exported by Fusion) and compare with Fusion's measurements; @@ -317,20 +333,20 @@ a manifold solid B-rep on the same surfaces a STEP export uses, with its name an if pass == list.len() { Ok(()) } else { Err(format!("{} file(s) failed", list.len() - pass)) } } +/// The quick gates first, so they fail fast; the wasm check (and doc tests) run while the test +/// binaries do, which leaves cargo's build lock free. fn cmd_ci() -> Result<(), String> { let mut c = cargo(); c.args(["fmt", "--all", "--", "--check"]); run(c, "cargo fmt --check")?; - let mut c = cargo(); - c.args(["clippy", "--workspace", "--all-targets", "--profile", "ci", "--", "-D", "warnings"]); - run(c, "cargo clippy -D warnings")?; - let mut c = cargo(); - c.args(["test", "--workspace", "--profile", "ci"]); - run(c, "cargo test")?; assets::run(&root())?; licences::run(&root(), true)?; cmd_layers()?; - cmd_wasm()?; + let mut c = cargo(); + c.args(["clippy", "--workspace", "--all-targets", "--profile", "ci", "--", "-D", "warnings"]); + run(c, "cargo clippy -D warnings")?; + let (wasm, n) = wasm_check()?; + test::run(&[], &[], vec![(format!("wasm32-unknown-unknown check ({n} crates)"), wasm), test::doc_tests(&[], &[])])?; eprintln!("ci: all gates passed"); Ok(()) } diff --git a/xtask/src/test.rs b/xtask/src/test.rs new file mode 100644 index 00000000..799c820f --- /dev/null +++ b/xtask/src/test.rs @@ -0,0 +1,105 @@ +//! `cargo xtask test`: build the workspace's tests once, then run every test binary at the same +//! time. `cargo test` runs them one after another, so one slow binary held up all the others; +//! the doc tests and any other `side` jobs run alongside on their own thread. + +use std::process::{Command, Stdio}; +use std::sync::mpsc; +use std::time::Instant; + +use crate::{cargo, root}; + +/// A test executable from `cargo test --no-run`. +struct TestBin { + label: String, + exe: String, + dir: String, +} + +/// Build the tests with `cargo_args` (e.g. `--locked`), run each binary with `test_args`, and run +/// the `side` jobs one after another meanwhile. Each prints a line when it ends; a failure prints +/// its whole output. +pub fn run(cargo_args: &[&str], test_args: &[&str], side: Vec<(String, Command)>) -> Result<(), String> { + let bins = build(cargo_args)?; + let start = Instant::now(); + let (tx, rx) = mpsc::channel::<(String, Result, f64)>(); + let mut failed = Vec::new(); + std::thread::scope(|scope| { + for b in &bins { + let tx = tx.clone(); + scope.spawn(move || { + let t0 = Instant::now(); + let mut c = Command::new(&b.exe); + c.args(test_args).current_dir(&b.dir).env("CARGO_MANIFEST_DIR", &b.dir); + let _ = tx.send((b.label.clone(), finish(c), t0.elapsed().as_secs_f64())); + }); + } + let tx_side = tx.clone(); + scope.spawn(move || { + for (label, c) in side { + let t0 = Instant::now(); + let _ = tx_side.send((label, finish(c), t0.elapsed().as_secs_f64())); + } + }); + drop(tx); + for (label, result, secs) in rx { + match result { + Ok(summary) => println!("ok {label:<44} {secs:6.1}s {summary}"), + Err(output) => { + println!("FAIL {label:<44} {secs:6.1}s\n{output}"); + failed.push(label); + } + } + } + }); + println!("test: {} binaries in {:.1}s", bins.len(), start.elapsed().as_secs_f64()); + if failed.is_empty() { Ok(()) } else { Err(format!("failed: {}", failed.join(", "))) } +} + +/// Run to the end: the last `test result:` line on success, all output on failure. +fn finish(mut c: Command) -> Result { + let out = c.stdin(Stdio::null()).output().map_err(|e| format!("failed to spawn: {e}"))?; + let stdout = String::from_utf8_lossy(&out.stdout); + if out.status.success() { + Ok(stdout.lines().rev().find(|l| l.starts_with("test result:")).unwrap_or("").trim_start_matches("test result: ").to_string()) + } else { + Err(format!("{stdout}{}\n({})", String::from_utf8_lossy(&out.stderr), out.status)) + } +} + +/// `cargo test --workspace --profile ci --no-run`, and the executables it built. +fn build(cargo_args: &[&str]) -> Result, String> { + let mut c = cargo(); + c.args(["test", "--workspace", "--profile", "ci", "--no-run", "--message-format=json-render-diagnostics"]).args(cargo_args); + eprintln!("$ cargo test --workspace --profile ci --no-run"); + let out = c.stderr(Stdio::inherit()).output().map_err(|e| format!("cargo test --no-run: failed to spawn: {e}"))?; + if !out.status.success() { + return Err(format!("cargo test --no-run: exited with {}", out.status)); + } + let mut bins: Vec = Vec::new(); + for line in String::from_utf8_lossy(&out.stdout).lines() { + let Ok(m) = serde_json::from_str::(line) else { continue }; + let Some(exe) = m["executable"].as_str().filter(|_| m["reason"] == "compiler-artifact" && m["profile"]["test"] == true) else { + continue; + }; + if bins.iter().any(|b| b.exe == exe) { + continue; + } + let manifest = std::path::Path::new(m["manifest_path"].as_str().unwrap_or_default()).to_path_buf(); + let dir = manifest.parent().map(|d| d.to_path_buf()).unwrap_or_else(root); + let pkg = dir.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); + let kind = m["target"]["kind"][0].as_str().unwrap_or(""); + let label = format!("{pkg} {kind} {}", m["target"]["name"].as_str().unwrap_or("")); + bins.push(TestBin { label, exe: exe.to_string(), dir: dir.to_string_lossy().into_owned() }); + } + if bins.is_empty() { + return Err("cargo test --no-run built no test executables".into()); + } + Ok(bins) +} + +/// The doc tests (`cargo test --doc`), to run as a side job. +pub fn doc_tests(cargo_args: &[&str], test_args: &[&str]) -> (String, Command) { + let mut c = cargo(); + c.args(["test", "--workspace", "--profile", "ci", "--doc"]).args(cargo_args).arg("--").args(test_args); + ("doc tests".into(), c) +}