From 7ba2d5db471d8e530d54b550eec0aa6d349df3af Mon Sep 17 00:00:00 2001 From: Irakli Kourtis Date: Sat, 17 Oct 2026 04:07:43 -0400 Subject: [PATCH] =?UTF-8?q?Robustness=20goldens:=20one=20test=20per=20seed?= =?UTF-8?q?,=20so=20CI=20spreads=20them=20(full=20runs=20~33=20=E2=86=92?= =?UTF-8?q?=20~15-20=20min)=20(#1851)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Thanks @jyviko. --- .config/nextest.toml | 2 +- .github/workflows/ci.yml | 8 +- CONTRIBUTING.md | 2 +- crates/engine/src/tests/robustness_goldens.rs | 166 +++++++++++++----- docs/book/src/dev/testing.md | 2 +- docs/robustness.md | 6 +- 6 files changed, 134 insertions(+), 52 deletions(-) diff --git a/.config/nextest.toml b/.config/nextest.toml index 66fe1043..92451dd5 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -11,7 +11,7 @@ not ( (package(solidcraft-engine) & ( test(=tests::hostile_params_never_panic) | test(/^tests::robustness::/) - | test(/^tests::robustness_goldens::/) + | test(/^tests::robustness_goldens::seed_/) | test(=pattern_suppress_tests::enclosure_arc_extinction_keeps_a_real_sharp_inner_lip_after_resize_and_reopen) | test(=pattern_suppress_tests::enclosure_datums_follow_wall_height_and_board_depth_with_material_proofs) | test(=cmd::plastic::tests::enclosure_snap_fit_follows_resized_front_wall) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8615b5c2..e4674f0d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -267,15 +267,15 @@ jobs: cargo nextest run --archive-file tests.tar.zst --workspace-remap . --extract-to . --extract-overwrite \ --profile ci --partition "slice:$SHARD/4" --no-tests=pass "${all[@]}" - name: Robustness goldens sample (pull requests that touch the kernel) - # Every 8th seed of tests/robustness/goldens.json, a quarter per shard; pushes to main and - # the nightly run every seed in the step above. + # Every 8th seed of tests/robustness/goldens.json (each seed is a test), a quarter per + # shard; pushes to main and the nightly run every seed in the step above. if: github.event_name == 'pull_request' && needs.changes.outputs.kernel == 'true' env: SHARD: ${{ matrix.shard }} run: | cargo nextest run --archive-file tests.tar.zst --workspace-remap . --extract-to . --extract-overwrite \ - --profile ci --ignore-default-filter \ - -E "package(solidcraft-engine) & test(=tests::robustness_goldens::sample_$((SHARD - 1)))" + --profile ci --ignore-default-filter --partition "slice:$SHARD/4" --no-tests=pass \ + -E 'package(solidcraft-engine) & test(/^tests::robustness_goldens::seed_(00|08|16|24|32|40|48|56|64|72|80|88)$/)' - name: Upload test timings if: always() uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1232a448..5bbeeb20 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -180,7 +180,7 @@ Pull requests skip a few heavy tests (one to four minutes each), which run on ev `.config/nextest.toml` and the [testing page](docs/book/src/dev/testing.md): - `solidcraft-engine`: `tests::hostile_params_never_panic`, everything in `tests::robustness` and - `tests::robustness_goldens` (pull requests that touch `crates/kernel` run its `sample_*` part), + the seeds of `tests::robustness_goldens` (`seed_00` … `seed_95`; pull requests that touch `crates/kernel` run every 8th), `pattern_suppress_tests::enclosure_arc_extinction_keeps_a_real_sharp_inner_lip_after_resize_and_reopen`, `pattern_suppress_tests::enclosure_datums_follow_wall_height_and_board_depth_with_material_proofs`, `cmd::plastic::tests::enclosure_snap_fit_follows_resized_front_wall`, the `atomic_commands` contract tests diff --git a/crates/engine/src/tests/robustness_goldens.rs b/crates/engine/src/tests/robustness_goldens.rs index 9cee7ef0..05922229 100644 --- a/crates/engine/src/tests/robustness_goldens.rs +++ b/crates/engine/src/tests/robustness_goldens.rs @@ -5,12 +5,14 @@ //! volume and area (within a relative 1e-6) and their face, edge and shell counts (exact). Any //! change fails, the better and the worse alike: a seed that used to be refused and now builds //! is recorded on purpose, with the reason. To record: `ROBUSTNESS_GOLDENS_BLESS="why" cargo -//! test --profile ci -p solidcraft-engine --lib robustness_goldens:: -- --include-ignored` -//! (docs/robustness.md). The reason goes into the file's `notes` with the seeds it changed. +//! test --profile ci -p solidcraft-engine --lib robustness_goldens::` (docs/robustness.md); run +//! it with `cargo test`, whose tests share one process and so one lock on the file, not with +//! nextest. The reason goes into the file's `notes` with the seeds it changed. //! -//! Pull requests that touch `crates/kernel` run a sample (`sample_0` … `sample_3`, every 8th -//! seed, split so the shards share them); pushes to main and the nightly run every seed -//! (`all_0` … `all_3`). +//! Each seed is its own test (`seed_00` … `seed_95`), so the runner spreads them over the CI +//! shards and their cores: a few seeds take minutes, and grouped four ways they made one test +//! of half an hour. Pull requests that touch `crates/kernel` run every 8th seed (ci.yml); +//! pushes to main and the nightly run every seed. use std::collections::BTreeMap; use std::path::PathBuf; @@ -52,9 +54,8 @@ fn differs(want: &Value, got: &Value) -> Option { (!out.is_empty()).then(|| out.join("; ")) } -fn check(part: u64, parts: u64, every: u64) { - let seeds: Vec = (0..SEEDS).filter(|s| s % every == 0 && (s / every) % parts == part).collect(); - let got: BTreeMap = seeds.iter().map(|s| (*s, fuzz::golden(*s, MAX_STEPS))).collect(); +fn check(seed: u64) { + let got: BTreeMap = BTreeMap::from([(seed, fuzz::golden(seed, MAX_STEPS))]); let _lock = FILE.lock().unwrap_or_else(|e| e.into_inner()); let mut file = load(); let changed: Vec<(u64, String)> = got @@ -111,42 +112,123 @@ fn check(part: u64, parts: u64, every: u64) { ); } -#[test] -fn sample_0() { - check(0, 4, 8); +/// One test per seed, and the list of them. +macro_rules! seeds { + ($($name:ident = $seed:literal),* $(,)?) => { + $( + #[test] + fn $name() { + check($seed); + } + )* + const LISTED: &[u64] = &[$($seed),*]; + }; } -#[test] -fn sample_1() { - check(1, 4, 8); -} +seeds!( + seed_00 = 0, + seed_01 = 1, + seed_02 = 2, + seed_03 = 3, + seed_04 = 4, + seed_05 = 5, + seed_06 = 6, + seed_07 = 7, + seed_08 = 8, + seed_09 = 9, + seed_10 = 10, + seed_11 = 11, + seed_12 = 12, + seed_13 = 13, + seed_14 = 14, + seed_15 = 15, + seed_16 = 16, + seed_17 = 17, + seed_18 = 18, + seed_19 = 19, + seed_20 = 20, + seed_21 = 21, + seed_22 = 22, + seed_23 = 23, + seed_24 = 24, + seed_25 = 25, + seed_26 = 26, + seed_27 = 27, + seed_28 = 28, + seed_29 = 29, + seed_30 = 30, + seed_31 = 31, + seed_32 = 32, + seed_33 = 33, + seed_34 = 34, + seed_35 = 35, + seed_36 = 36, + seed_37 = 37, + seed_38 = 38, + seed_39 = 39, + seed_40 = 40, + seed_41 = 41, + seed_42 = 42, + seed_43 = 43, + seed_44 = 44, + seed_45 = 45, + seed_46 = 46, + seed_47 = 47, + seed_48 = 48, + seed_49 = 49, + seed_50 = 50, + seed_51 = 51, + seed_52 = 52, + seed_53 = 53, + seed_54 = 54, + seed_55 = 55, + seed_56 = 56, + seed_57 = 57, + seed_58 = 58, + seed_59 = 59, + seed_60 = 60, + seed_61 = 61, + seed_62 = 62, + seed_63 = 63, + seed_64 = 64, + seed_65 = 65, + seed_66 = 66, + seed_67 = 67, + seed_68 = 68, + seed_69 = 69, + seed_70 = 70, + seed_71 = 71, + seed_72 = 72, + seed_73 = 73, + seed_74 = 74, + seed_75 = 75, + seed_76 = 76, + seed_77 = 77, + seed_78 = 78, + seed_79 = 79, + seed_80 = 80, + seed_81 = 81, + seed_82 = 82, + seed_83 = 83, + seed_84 = 84, + seed_85 = 85, + seed_86 = 86, + seed_87 = 87, + seed_88 = 88, + seed_89 = 89, + seed_90 = 90, + seed_91 = 91, + seed_92 = 92, + seed_93 = 93, + seed_94 = 94, + seed_95 = 95, +); +/// Every seed in 0..SEEDS has its test, once, and every recorded seed is one of them. #[test] -fn sample_2() { - check(2, 4, 8); -} - -#[test] -fn sample_3() { - check(3, 4, 8); -} - -#[test] -fn all_0() { - check(0, 4, 1); -} - -#[test] -fn all_1() { - check(1, 4, 1); -} - -#[test] -fn all_2() { - check(2, 4, 1); -} - -#[test] -fn all_3() { - check(3, 4, 1); +fn every_seed_has_a_test() { + assert_eq!(LISTED, (0..SEEDS).collect::>().as_slice()); + let file = load(); + let recorded: Vec = file["seeds"].as_object().into_iter().flatten().filter_map(|(k, _)| k.parse().ok()).collect(); + assert!(recorded.iter().all(|s| *s < SEEDS), "{recorded:?}"); } diff --git a/docs/book/src/dev/testing.md b/docs/book/src/dev/testing.md index e275dc42..219b7328 100644 --- a/docs/book/src/dev/testing.md +++ b/docs/book/src/dev/testing.md @@ -77,7 +77,7 @@ Pull requests skip a few heavy tests (each takes one to four minutes); they run `main`, nightly, and in `cargo xtask test`: - `solidcraft-engine`: `tests::hostile_params_never_panic`, everything in `tests::robustness` and - `tests::robustness_goldens` (pull requests that touch `crates/kernel` run its `sample_*` part), + the seeds of `tests::robustness_goldens` (`seed_00` … `seed_95`; pull requests that touch `crates/kernel` run every 8th), `pattern_suppress_tests::enclosure_arc_extinction_keeps_a_real_sharp_inner_lip_after_resize_and_reopen`, `pattern_suppress_tests::enclosure_datums_follow_wall_height_and_board_depth_with_material_proofs`, `cmd::plastic::tests::enclosure_snap_fit_follows_resized_front_wall`, the `atomic_commands` contract tests diff --git a/docs/robustness.md b/docs/robustness.md index 6c350610..db5f866d 100644 --- a/docs/robustness.md +++ b/docs/robustness.md @@ -43,13 +43,13 @@ its steps, how it ended (ok, or the kind and the start of the error it was refus for the design it left the bodies' total volume and area (within a relative 1e-6) and their face, edge and shell counts (exact). `tests::robustness_goldens` fails on any change, better or worse, so a kernel change shows every seed it moves. Pull requests that touch `crates/kernel` -run every 8th seed (`sample_0` … `sample_3`, about two minutes); pushes to `main` and the -nightly run them all (`all_0` … `all_3`). +run every 8th seed; pushes to `main` and the nightly run them all. Each seed is its own test +(`seed_00` … `seed_95`), so the CI shards and their cores share them out. To record an intended change, give the reason: ``` -ROBUSTNESS_GOLDENS_BLESS="why it changed" cargo test --profile ci -p solidcraft-engine --lib robustness_goldens::all +ROBUSTNESS_GOLDENS_BLESS="why it changed" cargo test --profile ci -p solidcraft-engine --lib robustness_goldens::seed_ ``` The changed seeds and the reason are added to the file's `notes`; say why in the pull request too.