From 33646c6958547b5f6d8a5431e9a823be3b659051 Mon Sep 17 00:00:00 2001 From: Alex Dima Date: Sun, 25 Oct 2026 13:58:02 +0100 Subject: [PATCH] ripple: drop the 50% limit and always run only the affected unit tests `ripple plan` ran every unit test when the selected unit tests were more than `--max-share` (default 0.5) of the unit test work, so a change that can affect 60% of the unit tests ran all of them on macOS and Windows. Selection is now off only for a dependency reason (`Selection::run_all`): a `runEverything` file, a package or script of the test runners, a module that the runners use, or a module that does not parse. Otherwise the plan selects the files the change can affect, however many. `--max-share` is gone (passing it is an error), `select` is `sel.run_all.is_none()`, and the share stays in plan.json and in the reason as information. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- build/ripple/README.md | 4 ++-- build/ripple/src/main.rs | 40 ++++++++++------------------------------ 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/build/ripple/README.md b/build/ripple/README.md index 3674dc1a383742..72bfee73e2627a 100644 --- a/build/ripple/README.md +++ b/build/ripple/README.md @@ -6,7 +6,7 @@ It is a prototype for running far fewer CI minutes on pull requests: instead of ## In pull request CI -The `Detect Changes` job of [pr.yml](../../.github/workflows/pr.yml) runs `ripple plan --config build/ripple/vscode.jsonc --base HEAD^1 --head HEAD --out-dir .build/unit-test-plan` on the pull request's merge commit, uploads the plan as the `unit-test-plan` artifact and writes a summary to the job page. Linux runs every unit test on every push; the macOS and Windows unit test jobs download the artifact and pass its `unit.txt` to each test runner with `--runList`. Each runner runs the tests it would run by default, but only from the listed files (see [test/unit/README.md](../../test/unit/README.md)), so ripple needs no knowledge of the runners. When the plan selects no file, those jobs are skipped instead of setting up to run nothing (the `selected-unit-tests` output of `Detect Changes`). They run every unit test when planning fails, when the artifact cannot be downloaded, when the change touches a file that every unit test depends on (`runEverything` in the configuration), or when it affects more than half of the unit test work. The planning steps never fail `Detect Changes`, which a broken ripple must not block. When ripple fails to build, pass its tests or plan, the `Ripple` check fails for pull requests that change `build/ripple`; it is skipped for the others, which show a warning. So does a module that ripple cannot parse, which disables selection. The binary is cached by its sources; [pr-ripple.yml](../../.github/workflows/pr-ripple.yml) builds it on `main` when they change, and checks daily that the cache still exists. +The `Detect Changes` job of [pr.yml](../../.github/workflows/pr.yml) runs `ripple plan --config build/ripple/vscode.jsonc --base HEAD^1 --head HEAD --out-dir .build/unit-test-plan` on the pull request's merge commit, uploads the plan as the `unit-test-plan` artifact and writes a summary to the job page. Linux runs every unit test on every push; the macOS and Windows unit test jobs download the artifact and pass its `unit.txt` to each test runner with `--runList`. Each runner runs the tests it would run by default, but only from the listed files (see [test/unit/README.md](../../test/unit/README.md)), so ripple needs no knowledge of the runners. When the plan selects no file, those jobs are skipped instead of setting up to run nothing (the `selected-unit-tests` output of `Detect Changes`). They run every unit test when planning fails, when the artifact cannot be downloaded, or when the change touches a file that every unit test depends on (`runEverything` in the configuration); otherwise they run the selected files, however many. The planning steps never fail `Detect Changes`, which a broken ripple must not block. When ripple fails to build, pass its tests or plan, the `Ripple` check fails for pull requests that change `build/ripple`; it is skipped for the others, which show a warning. So does a module that ripple cannot parse, which disables selection. The binary is cached by its sources; [pr-ripple.yml](../../.github/workflows/pr-ripple.yml) builds it on `main` when they change, and checks daily that the cache still exists. ## Configuration @@ -68,7 +68,7 @@ head=$(git stash create) # JSON plan for CI: the unit test files to run. `--out-dir ` also writes plan.json and, when # only the selected files should run, unit.txt; on GitHub Actions it writes the output `select` -# and a job summary. `--max-share` (default 0.5) sets when to run everything instead. +# and a job summary. ./target/release/ripple plan --config vscode.jsonc --base "$(git merge-base origin/main HEAD)" --head HEAD --out-dir ../../.build/unit-test-plan # Replay first-parent history and compare selection strategies diff --git a/build/ripple/src/main.rs b/build/ripple/src/main.rs index 281fec0a30b659..a1d418754745b3 100644 --- a/build/ripple/src/main.rs +++ b/build/ripple/src/main.rs @@ -29,24 +29,13 @@ const USAGE: &str = "usage: ripple --config FILE [--repo PATH] [option \tstats [--rev REV]: graph statistics of a commit\n\ \taffected --base REV [--head REV]: the test files a change can affect\n\ \tanalyze --file PATH: analysis summary of one module\n\ - \tplan --base REV [--head REV] [--max-share N] [--out-dir DIR]: the unit test files to run for a change\n\ + \tplan --base REV [--head REV] [--out-dir DIR]: the unit test files to run for a change\n\ \tblame --test PATH --since REV [--until REV]: the commits in a range that can affect a test\n\ \treplay --since DATE [--until REV] [--out FILE] [--hubs 1]: selection statistics over first-parent history"; /// The options of all commands. Each one takes a value. const OPTIONS: &[&str] = &[ - "config", - "repo", - "rev", - "base", - "head", - "file", - "max-share", - "out-dir", - "test", - "since", - "until", - "out", + "config", "repo", "rev", "base", "head", "file", "out-dir", "test", "since", "until", "out", "hubs", ]; @@ -132,9 +121,6 @@ fn main() -> Result<()> { repo, args.get("base").context("--base is required")?, args.get("head").unwrap_or("HEAD"), - args.get("max-share") - .map_or(Ok(0.5), |s| s.parse()) - .context("--max-share must be a number")?, args.get("out-dir"), ), "blame" => blame( @@ -325,19 +311,18 @@ fn affected(config: &Config, repo: PathBuf, base: &str, head: &str) -> Result<() Ok(()) } -/// Plans which unit test files a change needs and prints the plan as JSON. Selection is off when -/// a change reaches outside the module graph or selects more than `max_share` of the unit test -/// work, so callers then run every unit test. With `out_dir`, also writes the plan to -/// `/plan.json` and, when selection is on and selects any file, the selected files to -/// `/unit.txt`, one per line. Each test runner runs the listed files it would run by -/// default. On GitHub Actions it writes the output `select` (whether to run only the listed files) -/// and a job summary. +/// Plans which unit test files a change needs and prints the plan as JSON. Selection is off only +/// when a change reaches outside the module graph (`Selection::run_all`), so callers then run +/// every unit test; otherwise the plan selects the files the change can affect, however many. With +/// `out_dir`, also writes the plan to `/plan.json` and, when selection is on and selects +/// any file, the selected files to `/unit.txt`, one per line. Each test runner runs the +/// listed files it would run by default. On GitHub Actions it writes the output `select` (whether +/// to run only the listed files) and a job summary. fn plan( config: &Config, repo: PathBuf, base: &str, head: &str, - max_share: f64, out_dir: Option<&str>, ) -> Result<()> { let mut engine = Engine::new(repo.clone(), config); @@ -371,18 +356,13 @@ fn plan( }; let reason = match &sel.run_all { Some(r) => format!("running every unit test: {r}"), - None if share > max_share => format!( - "running every unit test: the change affects {:.0}% of the unit test work (limit {:.0}%)", - share * 100.0, - max_share * 100.0 - ), None => format!( "running the {} unit test files the change can affect ({:.1}% of the unit test work)", selected.len(), share * 100.0 ), }; - let select_only = sel.run_all.is_none() && share <= max_share; + let select_only = sel.run_all.is_none(); let json = serde_json::json!({ "select": select_only,