diff --git a/.github/workflows/_ci-flake.yml b/.github/workflows/_ci-flake.yml index 37f6d228e1..f2b4f8c23e 100644 --- a/.github/workflows/_ci-flake.yml +++ b/.github/workflows/_ci-flake.yml @@ -75,7 +75,7 @@ jobs: steps: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - - uses: rui314/setup-mold@7e4f20ad28a2e8ca6fd0892ccf72e2abb706b9c3 # v1 + - uses: rui314/setup-mold@339e96b4ff1f3bd865856ba4cbbe8818a7acb038 # v1 # Restores the Unit Tests lane's cache (_ci-rust.yml) read-only; this # lane never saves one of its own. The restore only hits while the cache # key matches: the same shared-key, and rust-cache's environment hash — @@ -99,7 +99,7 @@ jobs: run: node scripts/flake-check.mjs rust --iterations=10 $SELECTED - name: Rust iteration reports if: ${{ !cancelled() }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: flake-rust path: test-results/flake @@ -134,7 +134,7 @@ jobs: run: node scripts/flake-check.mjs node --iterations=10 $SELECTED - name: node:test iteration logs if: ${{ !cancelled() }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: flake-node path: test-results/flake @@ -203,7 +203,7 @@ jobs: run: node scripts/flake-check.mjs playwright-summary desktop/playwright-report.json "Flake check (Playwright smoke, $SHARD/$SHARD_TOTAL)" - name: Playwright repetition artifacts if: ${{ !cancelled() }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: flake-playwright-${{ matrix.shard }} path: | @@ -252,7 +252,7 @@ jobs: run: node scripts/flake-check.mjs dart --iterations=10 $SELECTED - name: Flutter iteration reports if: ${{ !cancelled() }} - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 with: name: flake-dart path: test-results/flake diff --git a/scripts/flake-check.mjs b/scripts/flake-check.mjs index e49d5c88e7..fcb1b8a27a 100644 --- a/scripts/flake-check.mjs +++ b/scripts/flake-check.mjs @@ -402,21 +402,23 @@ export function cratePackage(file) { return /^\[package\][\s\S]*?^name\s*=\s*"([^"]+)"/m.exec(manifest)[1]; } -// Cargo target flags building only the test binaries a file can be part of: -// src/** is the lib (when the crate has one) or a bin, tests/.rs and -// tests//*.rs (with a main.rs) the integration test of that name. A -// file placed by a custom `[[test]]` path builds every target instead. +// Cargo target flags building only the test binaries a file can be part of, +// one flag per entry so rustGroups can merge files' flags without splitting +// `--test `: src/** is the lib (when the crate has one) or a bin, +// tests/.rs and tests//*.rs (with a main.rs) the integration test +// of that name. A file placed by a custom `[[test]]` path builds every target +// instead. export function rustTarget(file) { const [, , kind, first, second] = file.split("/"); if (kind === "src") { const lib = existsSync(`${crateDir(file)}/src/lib.rs`) || /^\[lib\]/m.test(readFileSync(`${crateDir(file)}/Cargo.toml`, "utf8")); - return lib ? ["--lib", "--bins"] : ["--bins"]; + return lib ? [["--lib"], ["--bins"]] : [["--bins"]]; } - if (second === undefined) return ["--test", first.replace(/\.rs$/, "")]; + if (second === undefined) return [["--test", first.replace(/\.rs$/, "")]]; return existsSync(`${crateDir(file)}/tests/${first}/main.rs`) - ? ["--test", first] + ? [["--test", first]] : []; } @@ -672,13 +674,12 @@ export function chooseRust(tests, candidates) { return [...chosen.values()]; } -// nextest has no repeat flag, so run the selection once per iteration under -// the `flake` profile (.config/nextest.toml: no retries, JUnit per run), one -// cargo invocation per package so each builds only the targets its files can -// be in. The selection is listed first, by in-file path, and narrowed with -// chooseRust to exact (binary, test) pairs; a selection listing no runnable -// test is one of ignored or infrastructure-backed tests, not a failure. -function repeatRust(entries, iterations) { +// The repeat's cargo invocation per package: `-p ` plus the target +// flags of its selected files, each whole flag once (`--test codec --test +// connection`; merging tokens instead gave `--test codec connection`, which +// nextest reads as a test-name filter that matches nothing), and the tests +// to list in those targets. +export function rustGroups(entries) { const groups = new Map(); for (const entry of entries) { const [, file, path] = @@ -686,19 +687,35 @@ function repeatRust(entries, iterations) { if (!file) throw new Error(`rust entry must be ::: ${entry}`); const crate = cratePackage(file); - const group = groups.get(crate) ?? { targets: new Set(), tests: [] }; + const group = groups.get(crate) ?? { targets: new Map(), tests: [] }; const targets = rustTarget(file); if (targets.length === 0) group.every = true; - for (const target of targets) group.targets.add(target); + for (const target of targets) group.targets.set(target.join(" "), target); group.tests.push({ path, exact: [...rustModulePath(file), path].join("::"), }); groups.set(crate, group); } + for (const [crate, group] of groups) + group.args = [ + "-p", + crate, + ...(group.every ? [] : [...group.targets.values()].flat()), + ]; + return groups; +} + +// nextest has no repeat flag, so run the selection once per iteration under +// the `flake` profile (.config/nextest.toml: no retries, JUnit per run), one +// cargo invocation per package so each builds only the targets its files can +// be in. The selection is listed first, by in-file path, and narrowed with +// chooseRust to exact (binary, test) pairs; a selection listing no runnable +// test is one of ignored or infrastructure-backed tests, not a failure. +function repeatRust(entries, iterations) { + const groups = rustGroups(entries); const runs = []; - for (const [crate, group] of groups) { - group.args = ["-p", crate, ...(group.every ? [] : group.targets)]; + for (const group of groups.values()) { const suffixes = [...new Set(group.tests.map(({ path }) => path))] .map((path) => `test(/(^|::)${path}$/)`) .join(" | "); diff --git a/scripts/flake-check.test.mjs b/scripts/flake-check.test.mjs index 21b4006ca2..fa868c993a 100644 --- a/scripts/flake-check.test.mjs +++ b/scripts/flake-check.test.mjs @@ -15,6 +15,7 @@ import { parseJunit, playwrightRepetitions, playwrightShards, + rustGroups, rustModulePath, rustTarget, rustTests, @@ -261,14 +262,45 @@ test("same-named Rust tests in different modules are distinct, and a hunk select ); }); -test("Rust files build only the targets they can be part of", () => { +test("Rust files build only the targets they can be part of, merged per package one whole flag each", () => { for (const [file, flags] of [ - ["crates/buzz-relay/src/api/mod.rs", ["--lib", "--bins"]], - ["crates/buzz-admin/src/main.rs", ["--bins"]], - ["crates/buzz-test-client/tests/e2e_relay.rs", ["--test", "e2e_relay"]], + ["crates/buzz-relay/src/api/mod.rs", [["--lib"], ["--bins"]]], + ["crates/buzz-admin/src/main.rs", [["--bins"]]], + ["crates/buzz-test-client/tests/e2e_relay.rs", [["--test", "e2e_relay"]]], ["crates/buzz-agent/tests/common/mod.rs", []], ]) assert.deepEqual(rustTarget(file), flags, file); + // Two integration tests keep two `--test` flags: `--test codec connection` + // would make nextest filter on a test named "connection" and repeat nothing. + const args = (...entries) => + [...rustGroups(entries).values()].map((group) => group.args); + assert.deepEqual( + args( + "crates/buzz-audio-client/tests/codec.rs::roundtrip", + "crates/buzz-audio-client/tests/connection.rs::connects", + ), + [["-p", "buzz-audio-client", "--test", "codec", "--test", "connection"]], + ); + assert.deepEqual( + args( + "crates/buzz-audio-client/src/wire.rs::tests::frame", + "crates/buzz-audio-client/src/jitter.rs::tests::order", + "crates/buzz-audio-client/tests/codec.rs::roundtrip", + ), + [["-p", "buzz-audio-client", "--lib", "--bins", "--test", "codec"]], + ); + // A custom-path file builds every target of its package; packages stay apart. + assert.deepEqual( + args( + "crates/buzz-agent/tests/regressions.rs::replay", + "crates/buzz-agent/tests/common/mod.rs::helper", + "crates/buzz-admin/src/main.rs::tests::parses", + ), + [ + ["-p", "buzz-agent"], + ["-p", "buzz-admin", "--bins"], + ], + ); }); test("Rust selection: a hunk inside a test selects it, elsewhere selects every runnable test; ignored tests never run", () => {