From 1272bb57d30386339239b4ac7d1ccbd4d744c2bc Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 27 Oct 2026 00:09:53 +1100 Subject: [PATCH 1/7] ci: repeat changed tests ten times on pull requests Add an advisory flake check that selects the tests a pull request changes and repeats each one ten times with retries off, so a test that passes CI by luck is caught before it merges. This mirrors block/buzz-app#890 (and its sharding follow-up), adapted to the runners this repository actually has. scripts/flake-check.mjs `select` diffs HEAD^1..HEAD and maps hunks to tests per runner: Rust files to `::` entries by parsing `#[test]`-style attributes (shared code changes select the file's tests; `#[ignore]`d and Postgres tests are excluded), desktop Playwright specs to `file:line` smoke journeys via `playwright --list` (non-smoke and unregistered specs are excluded), and desktop node:test and Flutter test files whole. Deleted and renamed-away files drop out of the selection, and any runner whose selection exceeds 64 KiB is dropped with a note so GitHub outputs stay within limits. The reusable _ci-flake.yml adds `flake-select` plus one repeat lane per runner (`flake-rust`, `flake-node`, `flake-playwright` sharded by journey count, `flake-dart`). Each lane skips when its selection is empty, runs only on pull_request, and is not read by any required gate; its status and step summary carry the result. A `flake` nextest profile disables retries and emits JUnit so the Rust lane can report per-test, per-iteration results. The Unit Tests rust-cache gains a `shared-key` so the flake lane can restore it read-only. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- .config/nextest.toml | 10 + .github/workflows/_ci-desktop.yml | 2 + .github/workflows/_ci-flake.yml | 244 +++++++++ .github/workflows/_ci-rust.yml | 2 + .github/workflows/ci.yml | 11 + CONTRIBUTING.md | 11 + scripts/flake-check.mjs | 820 ++++++++++++++++++++++++++++++ scripts/flake-check.test.mjs | 454 +++++++++++++++++ 8 files changed, 1554 insertions(+) create mode 100644 .github/workflows/_ci-flake.yml create mode 100644 scripts/flake-check.mjs create mode 100644 scripts/flake-check.test.mjs diff --git a/.config/nextest.toml b/.config/nextest.toml index 1f9f3d156f4..8f8d2436b2a 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -43,3 +43,13 @@ threads-required = "num-test-threads" filter = "(test(/postgres_tests::/) or binary(/^postgres_/)) and not test(/(^|::)external_infra[^:]*::/)" setup = "postgres-template" run-wrapper = "postgres-isolation" + +[profile.flake] +# The advisory pull-request flake lane (scripts/flake-check.mjs rust) repeats +# a selection and must see every failed repetition, so no retries; the JUnit +# report gives it per-test statuses. +retries = 0 +fail-fast = false + +[profile.flake.junit] +path = "junit.xml" diff --git a/.github/workflows/_ci-desktop.yml b/.github/workflows/_ci-desktop.yml index a0b5e7e6283..90eacc59f1e 100644 --- a/.github/workflows/_ci-desktop.yml +++ b/.github/workflows/_ci-desktop.yml @@ -52,6 +52,8 @@ jobs: run: just desktop-check - name: Desktop unit tests run: just desktop-test + - name: Flake check selection tests + run: node --test scripts/flake-check.test.mjs - name: Desktop build run: just desktop-build - name: Save pnpm store cache diff --git a/.github/workflows/_ci-flake.yml b/.github/workflows/_ci-flake.yml new file mode 100644 index 00000000000..2cd77f16edb --- /dev/null +++ b/.github/workflows/_ci-flake.yml @@ -0,0 +1,244 @@ +name: CI / Flake Check +on: + workflow_call: + +env: + CARGO_TERM_COLOR: always + BUZZ_TEST_POSTGRES_PASSWORD: buzz_dev + PLAYWRIGHT_BROWSERS_PATH: ${{ github.workspace }}/.cache/ms-playwright + +# Advisory flake check: repeat the tests a pull request changes, ten times +# each with retries off, so a race inside a test surfaces before it lands on +# main. The lanes skip when no test changed, so they stay out of the required +# checks in ci.yml (whose gates reject skipped lanes); their own statuses and +# step summaries carry the result. Selections reach the lanes through env +# vars, never interpolated into run scripts; flake-check.mjs admits only plain +# path and identifier characters. +jobs: + flake-select: + name: Flake check selection + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + outputs: + rust: ${{ steps.select.outputs.rust }} + node: ${{ steps.select.outputs.node }} + playwright: ${{ steps.select.outputs.playwright }} + playwright_shards: ${{ steps.select.outputs.playwright_shards }} + playwright_shard_total: ${{ steps.select.outputs.playwright_shard_total }} + dart: ${{ steps.select.outputs.dart }} + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + # The merge commit's first parent is the base branch tip, so this + # diff is exactly the pull request's change without a full clone. + fetch-depth: 2 + - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + - name: Get pnpm store directory + id: pnpm-cache + run: echo "STORE_PATH=$(pnpm store path --silent)" >> "$GITHUB_OUTPUT" + - name: Restore pnpm store cache + uses: actions/cache/restore@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: ${{ steps.pnpm-cache.outputs.STORE_PATH }} + key: pnpm-${{ runner.os }}-${{ hashFiles('**/pnpm-lock.yaml') }} + restore-keys: pnpm-${{ runner.os }}- + # Playwright lists the smoke journeys of a changed spec, and the desktop + # app's TypeScript parses it. + - name: Install desktop dependencies + run: just desktop-install-ci + - name: Map changed hunks to tests + id: select + run: node scripts/flake-check.mjs select HEAD^1 HEAD + + flake-rust: + name: Flake check (Rust) + needs: [flake-select] + if: needs.flake-select.outputs.rust != '' + runs-on: ubuntu-latest + timeout-minutes: 30 + permissions: + contents: read + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + - uses: rui314/setup-mold@7e4f20ad28a2e8ca6fd0892ccf72e2abb706b9c3 # v1 + # Restores the Unit Tests lane's cache read-only: this lane only runs on + # pull requests, so it never saves one of its own. + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 + with: + shared-key: unit-tests + key: sherpa-cache-v1 + save-if: ${{ github.event_name != 'pull_request' }} + - name: Install cargo-nextest + uses: taiki-e/install-action@0fd46367812ee04360509b4169d9f659d6892bb2 # v2.79.15 + with: + tool: cargo-nextest@0.9.136 + # nextest has no repeat flag; the script runs the selection once per + # iteration under the `flake` profile (.config/nextest.toml). + - name: Repeat changed Rust tests + env: + SELECTED: ${{ needs.flake-select.outputs.rust }} + run: node scripts/flake-check.mjs rust --iterations=10 $SELECTED + - name: Rust iteration reports + if: ${{ !cancelled() }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: flake-rust + path: test-results/flake + if-no-files-found: ignore + retention-days: 7 + + flake-node: + name: Flake check (node:test) + needs: [flake-select] + if: needs.flake-select.outputs.node != '' + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: read + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + - name: Get pnpm store directory + id: pnpm-cache + run: echo "STORE_PATH=$(pnpm store path --silent)" >> "$GITHUB_OUTPUT" + - name: Restore pnpm store cache + uses: actions/cache/restore@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: ${{ steps.pnpm-cache.outputs.STORE_PATH }} + key: pnpm-${{ runner.os }}-${{ hashFiles('**/pnpm-lock.yaml') }} + restore-keys: pnpm-${{ runner.os }}- + - name: Install desktop dependencies + run: just desktop-install-ci + - name: Repeat changed node:test files + env: + SELECTED: ${{ needs.flake-select.outputs.node }} + run: node scripts/flake-check.mjs node --iterations=10 $SELECTED + - name: node:test iteration logs + if: ${{ !cancelled() }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: flake-node + path: test-results/flake + if-no-files-found: ignore + retention-days: 7 + + flake-playwright: + name: Flake check (Playwright smoke, ${{ matrix.shard }}/${{ needs.flake-select.outputs.playwright_shard_total }}) + needs: [flake-select] + if: needs.flake-select.outputs.playwright != '' + runs-on: ubuntu-latest + # Same bound as a smoke shard: Chromium dependency setup is bounded by + # ci-apt-retry.sh (~25m worst case), and flake-check.mjs sizes the shards + # so the repetitions fit the rest. + timeout-minutes: 45 + strategy: + fail-fast: false + matrix: + shard: ${{ fromJSON(needs.flake-select.outputs.playwright_shards) }} + permissions: + contents: read + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + - name: Get pnpm store directory + id: pnpm-cache + run: echo "STORE_PATH=$(pnpm store path --silent)" >> "$GITHUB_OUTPUT" + - name: Restore pnpm store cache + uses: actions/cache/restore@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: ${{ steps.pnpm-cache.outputs.STORE_PATH }} + key: pnpm-${{ runner.os }}-${{ hashFiles('**/pnpm-lock.yaml') }} + restore-keys: pnpm-${{ runner.os }}- + - name: Install desktop dependencies + run: just desktop-install-ci + - name: Get Playwright version + id: pw-version + run: echo "version=$(cd desktop && node -e "console.log(require('@playwright/test/package.json').version)")" >> "$GITHUB_OUTPUT" + - name: Restore Playwright browser cache + id: playwright-cache + uses: actions/cache/restore@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: ${{ env.PLAYWRIGHT_BROWSERS_PATH }} + key: playwright-${{ runner.os }}-${{ steps.pw-version.outputs.version }} + - name: Install Playwright Chromium + if: steps.playwright-cache.outputs.cache-hit != 'true' + run: cd desktop && pnpm exec playwright install chromium + - name: Install Playwright system dependencies + run: cd desktop && ../scripts/ci-apt-retry.sh pnpm exec playwright install-deps chromium + - name: Desktop E2E build + run: pnpm -C desktop build:e2e + # --retries=0 overrides the config's CI retries, so every failed + # repetition reports as unexpected and fails the step. + - name: Repeat changed smoke journeys + env: + PLAYWRIGHT_JSON_OUTPUT_FILE: playwright-report.json + SELECTED: ${{ needs.flake-select.outputs.playwright }} + SHARD: ${{ matrix.shard }} + SHARD_TOTAL: ${{ needs.flake-select.outputs.playwright_shard_total }} + run: cd desktop && pnpm exec playwright test --project=smoke --repeat-each=10 --retries=0 --shard="$SHARD/$SHARD_TOTAL" --reporter=list,json $SELECTED + - name: Summarize repetitions + if: ${{ !cancelled() }} + env: + SHARD: ${{ matrix.shard }} + SHARD_TOTAL: ${{ needs.flake-select.outputs.playwright_shard_total }} + 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 + with: + name: flake-playwright-${{ matrix.shard }} + path: | + desktop/playwright-report.json + desktop/test-results + if-no-files-found: ignore + retention-days: 7 + + flake-dart: + name: Flake check (Flutter) + needs: [flake-select] + if: needs.flake-select.outputs.dart != '' + runs-on: ubuntu-latest + timeout-minutes: 30 + permissions: + contents: read + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + # Restores the Mobile lane's Hermit and pub caches read-only; that lane + # saves them. + - name: Compute Hermit cache key + id: hermit-bin-hash + run: | + hash="$(find ./bin ! -type d | sort | xargs openssl sha256 | openssl sha256 -r | cut -d' ' -f1)" + echo "hash=$hash" >> "$GITHUB_OUTPUT" + - name: Restore Hermit package cache + uses: actions/cache/restore@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: ~/.cache/hermit/pkg + key: ${{ runner.os }}-hermit-cache-${{ steps.hermit-bin-hash.outputs.hash }} + restore-keys: ${{ runner.os }}-hermit-cache- + - name: Prime Flutter SDK + run: flutter --version + - name: Restore pub cache + uses: actions/cache/restore@caa296126883cff596d87d8935842f9db880ef25 # v5 + with: + path: ~/.pub-cache + key: pub-${{ runner.os }}-${{ hashFiles('mobile/pubspec.lock') }} + restore-keys: pub-${{ runner.os }}- + - name: Install dependencies + run: cd mobile && flutter pub get + - name: Repeat changed Flutter tests + env: + SELECTED: ${{ needs.flake-select.outputs.dart }} + run: node scripts/flake-check.mjs dart --iterations=10 $SELECTED + - name: Flutter iteration reports + if: ${{ !cancelled() }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: flake-dart + path: test-results/flake + if-no-files-found: ignore + retention-days: 7 diff --git a/.github/workflows/_ci-rust.yml b/.github/workflows/_ci-rust.yml index 4f615eb1622..fa60042f672 100644 --- a/.github/workflows/_ci-rust.yml +++ b/.github/workflows/_ci-rust.yml @@ -58,6 +58,8 @@ jobs: - uses: rui314/setup-mold@7e4f20ad28a2e8ca6fd0892ccf72e2abb706b9c3 # v1 - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 with: + # Named so the advisory flake lane (_ci-flake.yml) can restore it. + shared-key: unit-tests key: sherpa-cache-v1 save-if: ${{ github.event_name != 'pull_request' }} - name: Install cargo-nextest diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 403c8096049..c4cc6590bd8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -83,6 +83,9 @@ jobs: - 'scripts/normative-corpus.json' - 'desktop/**' - 'pnpm-lock.yaml' + - 'scripts/flake-check.mjs' + - 'scripts/flake-check.test.mjs' + - '.github/workflows/_ci-flake.yml' desktop-rust: - 'desktop/src-tauri/**' web: @@ -259,6 +262,14 @@ jobs: mobile: ${{ needs.changes.outputs.mobile == 'true' }} lane: mobile-swift + # Advisory flake check: repeats the tests a pull request changes. Its lanes + # skip when no test changed and none of the required gates below reads them; + # their own statuses and step summaries carry the result. + flake-check: + name: Flake Check + if: github.event_name == 'pull_request' + uses: ./.github/workflows/_ci-flake.yml + security-domain: name: Security Domain needs: [changes] diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 2beb40bd5a4..a5f3aa61f53 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -304,6 +304,17 @@ just ci This is the same check that runs in CI. PRs that fail `just ci` will not be merged. If `just ci` fails on formatting, `just fix-all` fixes it in one shot (`rustfmt` + Tauri fmt + desktop, web, and mobile formatters). +Pull requests also get an advisory **flake check**. `scripts/flake-check.mjs +select` maps the diff against the base branch to the Rust tests, desktop +`node --test` files, Playwright smoke journeys and Flutter test files it +changed: a hunk inside a test body selects that test; a change anywhere else +in a test file (imports, helpers, hooks, the code under test beside Rust +tests) or a new file selects the whole file. The `Flake check (…)` lanes then +run that selection ten times with retries off, so a repetition that fails is +a race in the test or the code under it, worth fixing before it lands. The +lanes skip when no test changed and are not required to merge; their step +summaries show the test × iteration table. + --- ## Code Style diff --git a/scripts/flake-check.mjs b/scripts/flake-check.mjs new file mode 100644 index 00000000000..b1c24ed8958 --- /dev/null +++ b/scripts/flake-check.mjs @@ -0,0 +1,820 @@ +// Advisory pull-request flake check. `select` maps a diff to the tests it +// changed, per runner; `rust`, `node` and `dart` repeat a selection and report +// every iteration. Playwright repeats natively with --repeat-each, and +// `playwright-summary` tabulates its JSON report afterwards. +import { spawnSync } from "node:child_process"; +import { + appendFileSync, + copyFileSync, + mkdirSync, + readFileSync, + rmSync, +} from "node:fs"; +import { createRequire } from "node:module"; +import { fileURLToPath } from "node:url"; + +const root = fileURLToPath(new URL("../", import.meta.url)); +export const ITERATIONS = 10; +// Repetitions spread across shards, so a whole-file selection stays inside +// the Playwright lane's job timeout. Playwright shards at file × repetition +// granularity (workers: 1, no fullyParallel), and the smoke lane runs ~1,500 +// journeys over ten ~11-minute shards, so five journeys × ten repetitions is a +// few minutes of test time per shard. The regular smoke lane never uses more +// than ten runners, so neither does this one. +const JOURNEYS_PER_SHARD = 5; +const MAX_SHARDS = 10; +export const playwrightShards = (journeys) => + Math.min(MAX_SHARDS, Math.max(1, Math.ceil(journeys / JOURNEYS_PER_SHARD))); +// GitHub caps a job's outputs at 1 MiB in total. A selection this large is a +// sweeping refactor, not a test change worth ten repetitions, so the lane +// skips rather than overrunning its timeout. +export const MAX_OUTPUT_BYTES = 64 * 1024; +export const runners = ["rust", "node", "playwright", "dart"]; +// Mirrors desktop/package.json "test": plain helpers run under the TS loader; +// jsdom tests also preload the DOM shim and force-exit past their timers. +export const nodeTestArgs = (file) => + file.endsWith(".jsdom-test.mjs") + ? [ + "--import", + "./test-jsdom-setup.mjs", + "--import", + "./test-loader.mjs", + "--experimental-strip-types", + "--test-force-exit", + "--test", + ] + : ["--import", "./test-loader.mjs", "--experimental-strip-types", "--test"]; +// Mirrors the mobile test shards in .github/workflows/_ci-clients.yml. +export const dartDefine = + "--dart-define=BUZZ_PUSH_GATEWAY_URL=https://push.example"; +// Mirrors the postgres-ci default-filter in .config/nextest.toml: those tests +// are #[ignore]d and need Postgres, Redis, S3 or MinIO, so a selection that +// reached one would fail every repetition without being flaky. +const infraTests = + "test(/postgres_tests::/) | binary(/^postgres_/) | test(/(^|::)external_infra[^:]*::/)"; + +// Mirrors desktop/package.json "test", desktop/playwright.config.ts testDir, +// the root Cargo workspace's crates and mobile's test directory. Paths are +// later split on whitespace in a shell, so admit only plain names. +export function classify(file) { + if (!/^[\w./@-]+$/.test(file)) return undefined; + if (/^crates\/[^/]+\/(?:src|tests)\/(?!fixtures\/).*\.rs$/.test(file)) + return "rust"; + if ( + /^desktop\/(?:src\/.*\.(?:test|jsdom-test)\.mjs|scripts\/[^/]+\.test\.mjs)$/.test( + file, + ) + ) + return "node"; + if (/^desktop\/tests\/e2e\/[^/]+\.spec\.ts$/.test(file)) return "playwright"; + if (/^mobile\/test\/.*_test\.dart$/.test(file)) return "dart"; + return undefined; +} + +// Line ranges of a -U0 diff: the new-side lines each hunk writes and the +// old-side lines it removes (empty, end < start, for a pure insertion). A pure +// deletion writes no lines and attaches to the line before it (0 at the top of +// the file); `removed` keeps what it took out, since that line cannot tell. +export function hunkRanges(diff) { + return [ + ...diff.matchAll(/^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/gm), + ].map(([, oldStart, oldCount = "1", start, count = "1"]) => ({ + start: Number(start), + end: Number(start) + Math.max(Number(count), 1) - 1, + removed: { + start: Number(oldStart), + end: Number(oldStart) + Number(oldCount) - 1, + }, + })); +} + +// Members that keep a `test`/`it` chain a test declaration. Anything else on +// the chain is a group (`describe`), a hook (`test.beforeEach`), configuration +// (`test.use`) or a harness object that happens to be named `test`. +const declaring = [ + "only", + "skip", + "fixme", + "fails", + "todo", + "concurrent", + "sequential", + "each", + "for", + "runIf", + "skipIf", +]; + +// The desktop app's TypeScript parses the specs; load it only for a selection +// that needs it, so Rust-only runs work without desktop dependencies. +let typescript; +const ts = () => + (typescript ??= createRequire( + new URL("../desktop/package.json", import.meta.url), + )("typescript")); + +// Line spans of every test declaration in a TypeScript spec, from its first +// line to its closing parenthesis. +export function testCalls(source, file) { + const t = ts(); + const sourceFile = t.createSourceFile( + file, + source, + t.ScriptTarget.Latest, + true, + ); + const lineAt = (position) => + sourceFile.getLineAndCharacterOfPosition(position).line + 1; + const names = (callee) => + t.isIdentifier(callee) + ? [callee.text] + : t.isPropertyAccessExpression(callee) + ? names(callee.expression)?.concat(callee.name.text) + : t.isCallExpression(callee) + ? names(callee.expression) + : undefined; + const calls = []; + const visit = (node) => { + const chain = t.isCallExpression(node) ? names(node.expression) : undefined; + if ( + chain && + ["test", "it"].includes(chain[0]) && + chain.slice(1).every((member) => declaring.includes(member)) + ) { + calls.push({ + start: lineAt(node.getStart(sourceFile)), + end: lineAt(node.end - 1), + }); + return; + } + t.forEachChild(node, visit); + }; + visit(sourceFile); + return calls; +} + +// Rust source with every comment and literal blanked to spaces, newlines kept, +// so attribute and brace scanning never see a `{` or `#[` inside a string, +// char literal or comment. Lifetimes (`'a`) are kept; char literals are not. +export function blankRust(source) { + let out = ""; + let i = 0; + const blank = (end) => { + for (; i < end && i < source.length; i += 1) + out += source[i] === "\n" ? "\n" : " "; + }; + const closingQuote = (from, raw = false, hashes = "") => { + for (let j = from; j < source.length; j += 1) { + if (source[j] === "\\" && !raw) j += 1; + else if (source[j] === '"' && source.startsWith(hashes, j + 1)) + return j + 1 + hashes.length; + } + return source.length; + }; + const rawPrefix = /[bc]?r(#*)"/y; + while (i < source.length) { + const rest = source.slice(i, i + 2); + rawPrefix.lastIndex = i; + const raw = rawPrefix.exec(source); + if (rest.startsWith("//")) { + const end = source.indexOf("\n", i); + blank(end === -1 ? source.length : end); + } else if (rest.startsWith("/*")) { + let depth = 0; + let j = i; + do { + if (source.startsWith("/*", j)) { + depth += 1; + j += 2; + } else if (source.startsWith("*/", j)) { + depth -= 1; + j += 2; + } else j += 1; + } while (depth > 0 && j < source.length); + blank(j); + } else if (raw) { + blank(closingQuote(i + raw[0].length, true, raw[1])); + } else if (/^[bc]?"/.test(rest)) { + blank(closingQuote(i + rest.indexOf('"') + 1)); + } else if (source[i] === "'") { + const width = source.codePointAt(i + 1) > 0xffff ? 2 : 1; + if (source[i + 1] === "\\") blank(source.indexOf("'", i + 2) + 1); + else if (source[i + 1 + width] === "'") blank(i + width + 2); + else { + out += source[i]; + i += 1; + } + } else { + out += source[i]; + i += 1; + } + } + return out; +} + +const matching = (text, open, close, from) => { + let depth = 0; + for (let j = from; j < text.length; j += 1) { + if (text[j] === open) depth += 1; + else if (text[j] === close) { + depth -= 1; + if (depth === 0) return j; + } + } + return text.length - 1; +}; + +// Every test function in a Rust file: a `fn` whose attribute block has an +// attribute path ending in `test` (`#[test]`, `#[tokio::test]`, …), spanning +// the first attribute line to the body's closing brace. `ignored` marks +// `#[ignore]`, which nextest skips by default. +export function rustTests(source) { + const text = blankRust(source); + const lineStarts = [0]; + for (let i = 0; i < text.length; i += 1) + if (text[i] === "\n") lineStarts.push(i + 1); + const lineAt = (offset) => { + let low = 0; + let high = lineStarts.length - 1; + while (low < high) { + const mid = Math.ceil((low + high) / 2); + if (lineStarts[mid] <= offset) low = mid; + else high = mid - 1; + } + return low + 1; + }; + const tests = []; + const attribute = /#\[/g; + for (let match = attribute.exec(text); match; match = attribute.exec(text)) { + const paths = []; + let j = match.index; + while (text.startsWith("#[", j)) { + const close = matching(text, "[", "]", j + 1); + paths.push(/^\s*([\w:]+)/.exec(text.slice(j + 2, close))?.[1] ?? ""); + j = close + 1; + while (/\s/.test(text[j] ?? "")) j += 1; + } + attribute.lastIndex = j; + if (!paths.some((path) => /^(?:\w+::)*test$/.test(path))) continue; + const fn = + /^(?:pub(?:\([\w\s:]*\))?\s+)?(?:async\s+)?(?:unsafe\s+)?fn\s+(\w+)/.exec( + text.slice(j), + ); + if (!fn) continue; + const open = text.indexOf("{", j + fn[0].length); + if (open === -1) continue; + tests.push({ + name: fn[1], + start: lineAt(match.index), + end: lineAt(matching(text, "{", "}", open)), + ignored: paths.includes("ignore"), + }); + } + return tests; +} + +// Bodies of the listed tests: their calls, keyed by the line the runner +// listed. Runners only report a call-site line, which may sit inside the call, +// so each listed line is owned by the enclosing test call; a listed line +// outside every call leaves its lines to the whole-file rule. +export function testBodies(calls, lines) { + const bodies = new Map(); + for (const line of [...lines].sort((a, b) => a - b)) { + const call = calls.find(({ start, end }) => start <= line && line <= end); + if (call && !bodies.has(call)) bodies.set(call, { ...call, line }); + } + return [...bodies.values()].sort((a, b) => a.start - b.start); +} + +// The calls owning every line of a range, or undefined once a line lies +// outside them all. +function owners(calls, { start, end }) { + const found = []; + for (let line = start; line <= end; ) { + const call = calls.find((c) => c.start <= line && line <= c.end); + if (!call) return undefined; + found.push(call); + line = call.end + 1; + } + return found; +} + +// A hunk whose every line lies in a test body selects those tests. A changed +// line outside every body (imports, helpers, hooks or constants before, +// between or after tests, or the code under test in a Rust module) can change +// every test, so it, a new file, or a file with no listed tests selects the +// whole file. Removed lines are judged by the old file's calls (`previous`): a +// deletion attaches to its preceding line, which belongs to the test before a +// removed hook, not to the hook. +export function selectTests( + ranges, + bodies, + { added = false, previous = [] } = {}, +) { + if (added) return { whole: true, reason: "new file" }; + if (bodies.length === 0) return { whole: true, reason: "no tests listed" }; + const selected = new Set(); + for (const { start, end, removed } of ranges) { + if (!owners(previous, removed)) + return { whole: true, reason: "shared code removed" }; + const current = owners(bodies, { start, end }); + if (!current) return { whole: true, reason: "shared code changed" }; + for (const body of current) selected.add(body.line); + } + return { lines: [...selected].sort((a, b) => a - b) }; +} + +// Rust tests a diff selects: hunks inside a test body select that test, any +// other hunk selects every test in the file (the module's code under test +// lives beside them), and `#[ignore]`d tests own their lines but never run. +// A file without test functions is not a test file and selects nothing. +export function selectRust( + diff, + source, + { added = false, previous = "" } = {}, +) { + const tests = rustTests(source); + if (tests.length === 0) return undefined; + const selection = selectTests( + hunkRanges(diff), + tests.map((test) => ({ ...test, line: test.start })), + { added, previous: added ? [] : rustTests(previous) }, + ); + const chosen = selection.whole + ? tests + : tests.filter((test) => selection.lines.includes(test.start)); + return { + ...selection, + names: chosen.filter((test) => !test.ignored).map((test) => test.name), + }; +} + +const run = (command, args, options = {}) => { + const result = spawnSync(command, args, { + cwd: root, + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + ...options, + }); + if (result.error) throw result.error; + return result; +}; +const git = (...args) => { + const result = run("git", args); + if (result.status !== 0) throw new Error(`git ${args[0]}: ${result.stderr}`); + return result.stdout; +}; +const summarize = (text) => { + if (process.env.GITHUB_STEP_SUMMARY) + appendFileSync(process.env.GITHUB_STEP_SUMMARY, text); + else process.stdout.write(text); +}; +const cell = (value) => String(value).replaceAll("|", "\\|"); + +// The `[package]` name of the crate owning a file under crates/. +export function cratePackage(file) { + const manifest = readFileSync( + `${root}crates/${file.split("/")[1]}/Cargo.toml`, + "utf8", + ); + return /^\[package\][\s\S]*?^name\s*=\s*"([^"]+)"/m.exec(manifest)[1]; +} + +// Smoke journeys per spec file as Playwright lists them. Integration-only +// specs list nothing for the smoke project, so a selection of them stays empty. +export function listPlaywright(files) { + const result = run(`${root}bin/pnpm`, [ + "-C", + "desktop", + "--silent", + "exec", + "playwright", + "test", + "--project=smoke", + "--list", + "--reporter=json", + ...files, + ]); + const report = JSON.parse(result.stdout); + if (result.status !== 0 || report.errors.length) + throw new Error( + `playwright --list: ${report.errors.map((error) => error.message).join("\n") || result.stderr}`, + ); + const specs = new Map(files.map((file) => [file, []])); + const collect = (suites) => { + for (const suite of suites) { + for (const spec of suite.specs ?? []) + if (spec.tests?.length) + specs.get(`desktop/tests/e2e/${spec.file}`)?.push(spec.line); + collect(suite.suites ?? []); + } + }; + collect(report.suites); + return specs; +} + +function select(base = "HEAD^1", head = "HEAD") { + const changed = git( + "diff", + "--name-status", + "--no-renames", + "-z", + "--diff-filter=AM", + base, + head, + "--", + ).split("\0"); + const files = Object.fromEntries(runners.map((runner) => [runner, []])); + const added = new Set(); + for (let i = 0; i + 1 < changed.length; i += 2) { + const [status, file] = [changed[i], changed[i + 1]]; + const runner = classify(file); + if (runner) files[runner].push(file); + if (status === "A") added.add(file); + } + const journeys = files.playwright.length + ? listPlaywright(files.playwright) + : new Map(); + const outputs = Object.fromEntries(runners.map((runner) => [runner, []])); + const rows = []; + const previous = (file) => + added.has(file) ? "" : git("show", `${base}:${file}`); + const diff = (file) => + git("diff", "-U0", "--no-color", base, head, "--", file); + for (const runner of runners) { + for (const file of files[runner]) { + let entries = [file]; + let note = "whole file"; + if (runner === "rust") { + const selection = selectRust( + diff(file), + readFileSync(`${root}${file}`, "utf8"), + { + added: added.has(file), + previous: previous(file), + }, + ); + if (!selection) continue; + const crate = cratePackage(file); + entries = selection.names.map((name) => `${crate}::${name}`); + note = + selection.names.length === 0 + ? "excluded (ignored tests only)" + : selection.whole + ? `whole file (${selection.reason}): ${selection.names.length} test(s)` + : `${selection.names.length} changed test(s)`; + } else if (runner === "playwright") { + const listed = journeys.get(file) ?? []; + const source = readFileSync(`${root}${file}`, "utf8"); + const selection = selectTests( + hunkRanges(diff(file)), + testBodies(testCalls(source, file), listed), + { + added: added.has(file), + previous: added.has(file) ? [] : testCalls(previous(file), file), + }, + ); + const lines = selection.whole ? listed : selection.lines; + entries = lines.map((line) => `${file}:${line}`); + note = + lines.length === 0 + ? "excluded (no smoke journeys)" + : selection.whole + ? `whole file (${selection.reason}): ${lines.length} journey(s)` + : `${lines.length} changed journey(s)`; + } + outputs[runner].push(...entries); + rows.push(`| ${cell(file)} | ${runner} | ${cell(note)} |`); + } + if (Buffer.byteLength(outputs[runner].join(" ")) > MAX_OUTPUT_BYTES) { + rows.push( + `| *(${outputs[runner].length} ${runner} entries)* | ${runner} | selection exceeds ${MAX_OUTPUT_BYTES} bytes; lane skipped |`, + ); + outputs[runner] = []; + } + } + summarize( + [ + "## Flake check selection", + "", + ...(rows.length + ? ["| File | Runner | Selection |", "| --- | --- | --- |", ...rows] + : ["No Rust, node:test, Playwright or Flutter test files changed."]), + "", + "", + ].join("\n"), + ); + const shards = playwrightShards(outputs.playwright.length); + const text = [ + ...Object.entries(outputs).map( + ([key, entries]) => `${key}=${entries.join(" ")}\n`, + ), + `playwright_shards=${JSON.stringify(Array.from({ length: shards }, (_, i) => i + 1))}\n`, + `playwright_shard_total=${shards}\n`, + ].join(""); + if (process.env.GITHUB_OUTPUT) + appendFileSync(process.env.GITHUB_OUTPUT, text); + else process.stdout.write(text); +} + +const resultsDir = (runner) => { + const dir = `${root}test-results/flake/${runner}`; + rmSync(dir, { recursive: true, force: true }); + mkdirSync(dir, { recursive: true }); + return dir; +}; + +// Test × iteration table for a repeated selection; a run passes only when its +// process exits 0 and every expected test reports. Tests that never ran in any +// iteration are left out, so the table holds only the repeated ones. +function report(title, runs) { + const columns = runs.map((_, i) => String(i + 1)); + const mark = (status) => + ({ passed: "✅", failed: "❌" })[status] ?? (status ? "⏭" : "—"); + const ran = (status) => status === "passed" || status === "failed"; + const tests = [ + ...new Set(runs.flatMap((run) => [...run.statuses.keys()])), + ].filter((test) => runs.some((run) => ran(run.statuses.get(test)))); + const failures = runs.filter((run) => !run.ok).length; + summarize( + [ + `## ${title}: ${failures} of ${runs.length} iterations failed`, + "", + `| Test | ${columns.join(" | ")} | Failed |`, + `| --- | ${columns.map(() => ":-:").join(" | ")} | ---: |`, + `| **Run** | ${runs.map((run) => (run.ok ? "✅" : "❌")).join(" | ")} | ${failures}/${runs.length} |`, + ...tests.sort().map((test) => { + const statuses = runs.map((run) => run.statuses.get(test)); + return `| ${cell(test)} | ${statuses.map(mark).join(" | ")} | ${statuses.filter((status) => status === "failed").length}/${runs.length} |`; + }), + "", + "", + ].join("\n"), + ); + process.exitCode = failures === 0 ? 0 : 1; +} + +const progress = (i, iterations, ok) => + console.log( + `flake-check: iteration ${i}/${iterations} ${ok ? "passed" : "FAILED"}`, + ); + +// Per-test statuses of a JUnit report, keyed ` `. +export function parseJunit(xml) { + const decode = (text) => + text.replace( + /&(quot|apos|lt|gt|amp);/g, + (_, entity) => + ({ quot: '"', apos: "'", lt: "<", gt: ">", amp: "&" })[entity], + ); + const statuses = new Map(); + for (const chunk of xml.split("")); + const body = chunk.split("")[0]; + const name = decode(/\bname="([^"]*)"/.exec(attributes)[1]); + const classname = decode( + /\bclassname="([^"]*)"/.exec(attributes)?.[1] ?? "", + ); + statuses.set( + `${classname} ${name}`, + /<(?:failure|error)[\s/>]/.test(body) + ? "failed" + : /]/.test(body) + ? "skipped" + : "passed", + ); + } + return statuses; +} + +// nextest has no repeat flag, so run the filterset once per iteration under +// the `flake` profile (.config/nextest.toml: no retries, JUnit per run). The +// selection is listed first: a filterset matching no runnable test is a +// selection of ignored or infrastructure-backed tests, not a failure. +function repeatRust(entries, iterations) { + const byPackage = new Map(); + for (const entry of entries) { + const [, crate, name] = /^(.+)::(\w+)$/.exec(entry) ?? []; + if (!crate) + throw new Error(`rust entry must be ::: ${entry}`); + byPackage.set(crate, (byPackage.get(crate) ?? new Set()).add(name)); + } + const filterset = `(${[...byPackage] + .map( + ([crate, names]) => + `(package(=${crate}) & (${[...names].map((name) => `test(/(^|::)${name}$/)`).join(" | ")}))`, + ) + .join(" | ")}) & !(${infraTests})`; + const packages = [...byPackage.keys()].flatMap((crate) => ["-p", crate]); + const listed = run("cargo", [ + "nextest", + "list", + ...packages, + "-E", + filterset, + "--message-format", + "json", + ]); + if (listed.status !== 0) + throw new Error(`cargo nextest list: ${listed.stderr}`); + const expected = Object.entries( + JSON.parse(listed.stdout)["rust-suites"], + ).flatMap(([binary, suite]) => + Object.entries(suite.testcases) + .filter( + ([, test]) => + !test.ignored && test["filter-match"].status === "matches", + ) + .map(([name]) => `${binary} ${name}`), + ); + if (expected.length === 0) { + summarize( + "## Flake check (Rust): nothing to repeat\n\nEvery selected test is ignored or infrastructure-backed.\n\n", + ); + return; + } + // nextest keeps its store under the workspace root's target/nextest, not + // CARGO_TARGET_DIR; the `flake` profile writes its JUnit report there. + const junit = `${root}target/nextest/flake/junit.xml`; + const dir = resultsDir("rust"); + const runs = []; + for (let i = 1; i <= iterations; i += 1) { + rmSync(junit, { force: true }); + const result = run( + "cargo", + ["nextest", "run", "--profile", "flake", ...packages, "-E", filterset], + { stdio: "inherit" }, + ); + let statuses = new Map(); + try { + statuses = parseJunit(readFileSync(junit, "utf8")); + copyFileSync(junit, `${dir}/iter-${i}.xml`); + } catch { + statuses = new Map(); + } + const ok = + result.status === 0 && + expected.every((test) => statuses.get(test) === "passed"); + runs.push({ ok, statuses }); + progress(i, iterations, ok); + } + report("Flake check (Rust)", runs); +} + +// node:test has no repeat flag either. Each file runs alone, so a failure is +// attributed to its file; the regular lane's cross-file concurrency is not +// reproduced here. +function repeatNode(files, iterations) { + const dir = resultsDir("node"); + const runs = []; + for (let i = 1; i <= iterations; i += 1) { + const statuses = new Map(); + for (const file of files) { + const path = file.replace(/^desktop\//, ""); + const result = run(process.execPath, [...nodeTestArgs(path), path], { + cwd: `${root}desktop`, + }); + process.stdout.write(result.stdout); + process.stderr.write(result.stderr); + appendFileSync( + `${dir}/iter-${i}.log`, + `# ${file}\n${result.stdout}${result.stderr}\n`, + ); + statuses.set(file, result.status === 0 ? "passed" : "failed"); + } + const ok = [...statuses.values()].every((status) => status === "passed"); + runs.push({ ok, statuses }); + progress(i, iterations, ok); + } + report("Flake check (node:test)", runs); +} + +// Per-test statuses of a `dart test` JSON report (one event per line), keyed +// ` › `. Hidden entries are the runner's own file-loading steps. +export function parseDartReport(text, mobileRoot = `${root}mobile/`) { + const started = new Map(); + const statuses = new Map(); + for (const line of text.split("\n")) { + let event; + try { + event = JSON.parse(line); + } catch { + continue; + } + if (event.type === "testStart") started.set(event.test.id, event.test); + if (event.type !== "testDone" || event.hidden) continue; + const test = started.get(event.testID); + if (!test) continue; + const file = fileURLToPath(test.url ?? "file:///").replace(mobileRoot, ""); + statuses.set( + `${file} › ${test.name}`, + event.skipped + ? "skipped" + : event.result === "success" + ? "passed" + : "failed", + ); + } + return statuses; +} + +// `flutter test` has no repeat flag; each iteration recompiles only the +// selected files, as the sharded mobile lane does. +function repeatDart(files, iterations) { + const dir = resultsDir("dart"); + const runs = []; + // The Justfile unsets these so a worktree's git context never reaches flutter. + const { GIT_DIR, GIT_WORK_TREE, ...env } = process.env; + for (let i = 1; i <= iterations; i += 1) { + const reportFile = `${dir}/iter-${i}.json`; + const result = run( + "flutter", + [ + "test", + dartDefine, + `--file-reporter=json:${reportFile}`, + ...files.map((file) => file.replace(/^mobile\//, "")), + ], + { cwd: `${root}mobile`, env, stdio: "inherit" }, + ); + let statuses = new Map(); + try { + statuses = parseDartReport(readFileSync(reportFile, "utf8")); + } catch { + statuses = new Map(); + } + const ok = result.status === 0 && statuses.size > 0; + runs.push({ ok, statuses }); + progress(i, iterations, ok); + } + report("Flake check (Flutter)", runs); +} + +// Playwright's JSON report lists a `--repeat-each` journey once per +// repetition, in order; `unexpected` is a failed repetition since the lane +// runs with --retries=0. +export function playwrightRepetitions(report) { + const repetitions = new Map(); + const collect = (suites) => { + for (const suite of suites) { + for (const spec of suite.specs ?? []) { + const key = `${spec.file}:${spec.line} › ${spec.title}`; + const statuses = repetitions.get(key) ?? []; + for (const test of spec.tests ?? []) + statuses.push( + { expected: "passed", unexpected: "failed", flaky: "failed" }[ + test.status + ] ?? "skipped", + ); + repetitions.set(key, statuses); + } + collect(suite.suites ?? []); + } + }; + collect(report.suites ?? []); + return repetitions; +} + +function playwrightSummary(reportPath, title) { + const repetitions = playwrightRepetitions( + JSON.parse(readFileSync(reportPath, "utf8")), + ); + const iterations = Math.max( + 0, + ...[...repetitions.values()].map((s) => s.length), + ); + const runs = Array.from({ length: iterations }, (_, i) => ({ + ok: [...repetitions.values()].every((statuses) => statuses[i] !== "failed"), + statuses: new Map( + [...repetitions].map(([test, statuses]) => [test, statuses[i]]), + ), + })); + report(title, runs); + // Playwright already failed its own step; the summary only reports. + process.exitCode = 0; +} + +function main() { + const [command, ...args] = process.argv.slice(2); + if (command === "select" && args.length <= 2) return select(...args); + if (command === "playwright-summary" && args.length === 2) + return playwrightSummary(...args); + const repeat = { rust: repeatRust, node: repeatNode, dart: repeatDart }[ + command + ]; + if (repeat) { + const iterations = Number( + args + .find((argument) => argument.startsWith("--iterations=")) + ?.slice("--iterations=".length) ?? ITERATIONS, + ); + const entries = args.filter((argument) => !argument.startsWith("--")); + if (Number.isInteger(iterations) && iterations > 0 && entries.length > 0) + return repeat(entries, iterations); + } + throw new Error( + "usage: flake-check select [base] [head] | rust|node|dart [--iterations=N] | playwright-summary ", + ); +} + +if (process.argv[1] === fileURLToPath(import.meta.url)) main(); diff --git a/scripts/flake-check.test.mjs b/scripts/flake-check.test.mjs new file mode 100644 index 00000000000..876675b973e --- /dev/null +++ b/scripts/flake-check.test.mjs @@ -0,0 +1,454 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import test from "node:test"; +import { + classify, + cratePackage, + dartDefine, + hunkRanges, + listPlaywright, + nodeTestArgs, + parseDartReport, + parseJunit, + playwrightRepetitions, + playwrightShards, + rustTests, + selectRust, + selectTests, + testBodies, + testCalls, +} from "./flake-check.mjs"; + +const read = (path) => readFileSync(new URL(path, import.meta.url), "utf8"); +// A hunk writing new-side lines start..end; by default it removed nothing. +const hunk = (start, end, removed = { start: start + 1, end: start }) => ({ + start, + end, + removed, +}); +// A -U0 diff of (old start, old count, new start, new count) hunks. +const diff = (...hunks) => + hunks.map(([o, oc, n, nc]) => `@@ -${o},${oc} +${n},${nc} @@`).join("\n"); + +test("classification follows each runner's test file layout", () => { + for (const [file, runner] of [ + ["crates/buzz-core/src/filter.rs", "rust"], + ["crates/buzz-test-client/tests/e2e_relay.rs", "rust"], + ["desktop/src/features/channels/lib/channelDescription.test.mjs", "node"], + ["desktop/src/shared/ui/thing.jsdom-test.mjs", "node"], + ["desktop/scripts/demo-build-config.test.mjs", "node"], + ["desktop/tests/e2e/channels.spec.ts", "playwright"], + ["mobile/test/features/invites/invite_join_provider_test.dart", "dart"], + ]) + assert.equal(classify(file), runner, file); + for (const file of [ + "crates/buzz-core/Cargo.toml", + "crates/buzz-feature-flags/tests/fixtures/relay-feature-selection/src/main.rs", + "desktop/src-tauri/src/lib.rs", + "desktop/src/features/channels/lib/channelDescription.ts", + "desktop/tests/e2e/helpers/animations.ts", + "desktop/tests/e2e/typing-latency.perf.ts", + "desktop/tests/e2e/fixtures/x.spec.ts", + "mobile/lib/main.dart", + "mobile/test/helpers.dart", + "scripts/flake-check.test.mjs", + "web/src/x.test.mjs", + // Selections are shell-split downstream; refuse unusual names. + "desktop/tests/e2e/odd name.spec.ts", + "crates/buzz-core/src/$(id).rs", + ]) + assert.equal(classify(file), undefined, file); +}); + +test("-U0 hunks map to new-side ranges, with deletions attached to the preceding line and their old-side lines kept", () => { + const text = [ + "diff --git a/x.spec.ts b/x.spec.ts", + "--- a/x.spec.ts", + "+++ b/x.spec.ts", + "@@ -1 +1 @@", + '-import { test } from "@playwright/test";', + '+import { expect, test } from "@playwright/test";', + "@@ -12,0 +13,3 @@ test('a', async () => {", + "+ const x = 1;", + "+ const y = 2;", + "+ expect(x).toBe(y - 1);", + "@@ -30,2 +33,0 @@ test('b', async () => {", + "- removed();", + "- alsoRemoved();", + "@@ -1,3 +0,0 @@", + "-gone", + "", + ].join("\n"); + assert.deepEqual(hunkRanges(text), [ + { start: 1, end: 1, removed: { start: 1, end: 1 } }, + { start: 13, end: 15, removed: { start: 12, end: 11 } }, + { start: 33, end: 33, removed: { start: 30, end: 31 } }, + { start: 0, end: 0, removed: { start: 1, end: 3 } }, + ]); + assert.deepEqual(hunkRanges(""), []); +}); + +test("Playwright test calls span the declaration; groups, hooks and configuration are outside", () => { + const spec = [ + 'import { expect, test } from "@playwright/test";', + "test.use({ viewport: { width: 800, height: 600 } });", + "const seed = 1;", + 'test.describe("suite", () => {', + " test.beforeEach(async ({ page }) => {", + ' await page.goto("/");', + " });", + ' test("journey", async ({ page }) => {', + ' test.skip(seed === 2, "not here");', + ' await expect(page.getByText("Go")).toBeVisible();', + " });", + ' for (const mode of ["mouse", "keyboard"]) {', + ' test.fixme(mode + " later", async () => {', + " await Promise.resolve();", + " });", + " }", + "});", + 'test.describe.configure({ mode: "serial" });', + "", + ].join("\n"); + const calls = testCalls(spec, "x.spec.ts"); + assert.deepEqual(calls, [ + { start: 8, end: 11 }, + { start: 13, end: 15 }, + ]); + // Playwright lists a looped declaration once per iteration, at one line. + assert.deepEqual(testBodies(calls, [13, 13, 8]), [ + { start: 8, end: 11, line: 8 }, + { start: 13, end: 15, line: 13 }, + ]); + // A listed line outside every call (a hook) has no body. + assert.deepEqual(testBodies(calls, [5]), []); +}); + +const rustSource = [ + "//! Module docs mentioning #[test] in prose.", + "use std::fmt;", + "", + "pub fn code_under_test(input: &str) -> String {", + " let brace = '{';", + ' let raw = r"\\";', + ' format!("{brace}{raw}{input}")', + "}", + "", + "#[cfg(test)]", + "mod tests {", + " use super::*;", + "", + " fn helper() -> &'static str {", + ' "}"', + " }", + "", + " /// Docs for the first test.", + " #[test]", + " fn plain() {", + ' assert_eq!(code_under_test("x"), "{\\\\x");', + " }", + "", + ' #[tokio::test(flavor = "multi_thread")]', + ' #[ignore = "requires Postgres }"]', + " async fn needs_infra() {", + " /* a block comment with a } brace", + " and #[test] inside */", + " helper();", + " }", + "", + " #[ignore]", + " #[test]", + " pub(crate) fn also_ignored() {}", + "", + " mod postgres_tests {", + " #[sqlx::test]", + " async fn in_nested_module(pool: Pool) -> Result<(), E> {", + " Ok(())", + " }", + " }", + "}", + "", +].join("\n"); + +test("Rust test functions span their attribute block to the closing brace, through strings, chars and comments", () => { + assert.deepEqual(rustTests(rustSource), [ + { name: "plain", start: 19, end: 22, ignored: false }, + { name: "needs_infra", start: 24, end: 30, ignored: true }, + { name: "also_ignored", start: 32, end: 34, ignored: true }, + { name: "in_nested_module", start: 37, end: 40, ignored: false }, + ]); + assert.deepEqual(rustTests("fn main() {}\n"), []); +}); + +test("Rust selection: a hunk inside a test selects it, elsewhere selects every runnable test; ignored tests never run", () => { + const select = (text, options) => + selectRust(text, rustSource, { previous: rustSource, ...options }); + assert.deepEqual(select(diff([21, 1, 21, 1])), { + lines: [19], + names: ["plain"], + }); + // Inside an ignored test: owned, so not shared code, but nothing to run. + assert.deepEqual(select(diff([28, 0, 29, 1])), { lines: [24], names: [] }); + // The code under test, a helper, or a deleted helper (attached to line 13) + // can change every test. + const whole = (reason) => ({ + whole: true, + reason, + names: ["plain", "in_nested_module"], + }); + assert.deepEqual(select(diff([4, 0, 5, 1])), whole("shared code changed")); + assert.deepEqual( + select(diff([21, 1, 21, 1], [14, 0, 15, 1])), + whole("shared code changed"), + ); + assert.deepEqual(select(diff([14, 3, 13, 0])), whole("shared code removed")); + assert.deepEqual( + select(diff([21, 1, 21, 1]), { added: true, previous: "" }), + whole("new file"), + ); + // A file without test functions is not a test file. + assert.equal(selectRust(diff([1, 1, 1, 1]), "fn main() {}\n"), undefined); + assert.equal(cratePackage("crates/buzz-core/src/filter.rs"), "buzz-core"); +}); + +test("hunks inside tests select those tests; shared code, new or unlisted files select the whole file", () => { + const bodies = [ + { start: 10, end: 22, line: 10 }, + { start: 25, end: 37, line: 25 }, + { start: 38, end: 52, line: 40 }, + ]; + // The old file had the same tests, with a hook between the last two. + const previous = [ + { start: 10, end: 22 }, + { start: 25, end: 37 }, + { start: 41, end: 55 }, + ]; + const select = (ranges, options) => + selectTests(ranges, bodies, { previous, ...options }); + // Edit inside one test, including its first and last lines. + assert.deepEqual(select([hunk(12, 14)]), { lines: [10] }); + assert.deepEqual(select([hunk(10, 10)]), { lines: [10] }); + assert.deepEqual(select([hunk(22, 22)]), { lines: [10] }); + // A hunk across two adjacent bodies selects both, by their listed lines. + assert.deepEqual(select([hunk(36, 39)]), { lines: [25, 40] }); + // A hunk that also covers the gap between tests, or lines after the last + // test, touches shared code. + for (const range of [hunk(22, 25), hunk(52, 53), hunk(400, 405)]) + assert.deepEqual(select([range]), { + whole: true, + reason: "shared code changed", + }); + // Several hunks accumulate distinct tests in file order. + assert.deepEqual(select([hunk(41, 41), hunk(11, 11), hunk(12, 12)]), { + lines: [10, 40], + }); + // Replacing lines inside a test, or deleting some of them (attached to the + // preceding line), selects that test when the old lines were its own. + assert.deepEqual(select([hunk(30, 31, { start: 30, end: 32 })]), { + lines: [25], + }); + assert.deepEqual(select([hunk(30, 30, { start: 31, end: 32 })]), { + lines: [25], + }); + // Deleting the hook that followed the second test attaches to that test's + // closing line, but the removed lines were never a test's. + assert.deepEqual(select([hunk(37, 37, { start: 38, end: 40 })]), { + whole: true, + reason: "shared code removed", + }); + // Deletion at the top of the file (line 0) and edits before the first test + // fall back to the whole file. + assert.deepEqual(select([hunk(0, 0, { start: 1, end: 1 })]), { + whole: true, + reason: "shared code removed", + }); + assert.deepEqual(select([hunk(12, 12), hunk(3, 3)]), { + whole: true, + reason: "shared code changed", + }); + assert.deepEqual(select([hunk(12, 12)], { added: true }), { + whole: true, + reason: "new file", + }); + assert.deepEqual(selectTests([hunk(12, 12)], []), { + whole: true, + reason: "no tests listed", + }); + // Without old-side calls, every removal is shared code. + assert.deepEqual( + selectTests([hunk(30, 30, { start: 31, end: 32 })], bodies), + { whole: true, reason: "shared code removed" }, + ); +}); + +test("Playwright lists smoke journeys per spec; integration-only specs list none", () => { + const smoke = "desktop/tests/e2e/channel-sort.spec.ts"; + const integration = "desktop/tests/e2e/agents.spec.ts"; + const listed = listPlaywright([smoke, integration]); + assert.deepEqual(listed.get(integration), []); + const lines = listed.get(smoke); + assert.ok(lines.length > 0, "a smoke spec must list its journeys"); + assert.equal( + testBodies(testCalls(read(`../${smoke}`), smoke), lines).length, + new Set(lines).size, + "every listed journey must be owned by a test call", + ); +}); + +test("Playwright repetitions get one shard per five journeys, at most ten", () => { + assert.deepEqual( + [0, 1, 5, 6, 18, 40, 41, 50, 51, 200].map(playwrightShards), + [1, 1, 1, 2, 4, 8, 9, 10, 10, 10], + ); +}); + +test("reports give per-test statuses per iteration", () => { + assert.deepEqual( + parseJunit( + [ + '<?xml version="1.0" encoding="UTF-8"?>', + '<testsuites name="nextest-run">', + '<testsuite name="buzz-core" tests="3">', + '<testcase name="filter::tests::a" classname="buzz-core" time="0.1"/>', + '<testcase name="filter::tests::b <x>" classname="buzz-core" time="0.1">', + '<failure type="test failure">assertion `left == right` failed</failure>', + "</testcase>", + '<testcase name="filter::tests::c" classname="buzz-core"><skipped/></testcase>', + "</testsuite>", + '<testsuite name="buzz-core::e2e" tests="1">', + '<testcase name="d" classname="buzz-core::e2e" time="0.1"><error/></testcase>', + "</testsuite>", + "</testsuites>", + ].join("\n"), + ), + new Map([ + ["buzz-core filter::tests::a", "passed"], + ["buzz-core filter::tests::b <x>", "failed"], + ["buzz-core filter::tests::c", "skipped"], + ["buzz-core::e2e d", "failed"], + ]), + ); + const event = (value) => JSON.stringify(value); + assert.deepEqual( + parseDartReport( + [ + event({ type: "start", protocolVersion: "0.1.1" }), + event({ + type: "testStart", + test: { id: 1, name: "loading /m/test/a_test.dart", url: null }, + }), + event({ + type: "testDone", + testID: 1, + result: "success", + hidden: true, + skipped: false, + }), + event({ + type: "testStart", + test: { + id: 2, + name: "group renders", + url: "file:///m/test/a_test.dart", + }, + }), + event({ + type: "testStart", + test: { + id: 3, + name: "group fails", + url: "file:///m/test/a_test.dart", + }, + }), + event({ + type: "testStart", + test: { id: 4, name: "later", url: "file:///m/test/b_test.dart" }, + }), + "not json", + event({ + type: "testDone", + testID: 2, + result: "success", + hidden: false, + skipped: false, + }), + event({ + type: "testDone", + testID: 3, + result: "error", + hidden: false, + skipped: false, + }), + event({ + type: "testDone", + testID: 4, + result: "success", + hidden: false, + skipped: true, + }), + event({ type: "done", success: false }), + ].join("\n"), + "/m/", + ), + new Map([ + ["test/a_test.dart › group renders", "passed"], + ["test/a_test.dart › group fails", "failed"], + ["test/b_test.dart › later", "skipped"], + ]), + ); + // Playwright lists a --repeat-each journey once per repetition, in order. + const spec = (title, line, status) => ({ + title, + file: "channels.spec.ts", + line, + tests: [{ status }], + }); + assert.deepEqual( + playwrightRepetitions({ + suites: [ + { + specs: [spec("a", 10, "expected"), spec("a", 10, "unexpected")], + suites: [ + { specs: [spec("b", 30, "skipped"), spec("a", 10, "expected")] }, + ], + }, + ], + }), + new Map([ + ["channels.spec.ts:10 › a", ["passed", "failed", "passed"]], + ["channels.spec.ts:30 › b", ["skipped"]], + ]), + ); +}); + +test("repeat invocations mirror the lanes they stand in for", () => { + // node:test flags come from desktop/package.json "test"; the Flutter define + // from the mobile test shards; nextest's flake profile must keep retries off. + const script = JSON.parse(read("../desktop/package.json")).scripts.test; + for (const file of ["x.test.mjs", "x.jsdom-test.mjs"]) + assert.ok(script.includes(`node ${nodeTestArgs(file).join(" ")} "`), file); + assert.ok( + read("../.github/workflows/_ci-clients.yml").includes( + `scripts/mobile-test-shard.sh \${{ matrix.shard }} 3 ${dartDefine}`, + ), + ); + assert.match( + read("../.config/nextest.toml"), + /^\[profile\.flake\]\n(?:#.*\n)*retries = 0\nfail-fast = false\n\n\[profile\.flake\.junit\]\npath = "junit\.xml"\n/m, + ); + const flake = read("../.github/workflows/_ci-flake.yml"); + for (const runner of ["rust", "node", "dart"]) + assert.ok( + flake.includes(`flake-check.mjs ${runner} --iterations=10 $SELECTED`), + runner, + ); + assert.ok(flake.includes("--repeat-each=10 --retries=0")); + // The lanes are advisory: ci.yml calls the workflow on pull requests only, + // and no required gate depends on it. + const ci = read("../.github/workflows/ci.yml"); + assert.match( + ci, + /^ {2}flake-check:\n(?: {4}.*\n)*? {4}if: github\.event_name == 'pull_request'\n(?: {4}.*\n)*? {4}uses: \.\/\.github\/workflows\/_ci-flake\.yml\n/m, + ); + assert.doesNotMatch(ci, /needs: \[[^\]]*flake/); +}); From 6b9ad6e309d1e7a3af668676ad6e15d809a41777 Mon Sep 17 00:00:00 2001 From: Matt Toohey <contact@matttoohey.com> Date: Tue, 27 Oct 2026 07:40:10 +1100 Subject: [PATCH 2/7] ci: harden flake-check selection and Rust targeting Address the review of the flake check port (99b21e4fb). An integration-only Playwright selection no longer fails flake-select: Playwright exits 1 with a lone "No tests found" error when the file filters match nothing in the smoke project, which listPlaywright now reads as an empty selection, as buzz-app's listBrowser does. Listed journeys are deduplicated, since a looped test() declaration is listed once per iteration, and a selection over 250 journeys skips the Playwright lane with a note: the smoke lane runs ~150 journey-runs per shard in ~11 minutes, so 250 journey-runs per shard across ten shards is what fits the 45-minute timeout after setup and the E2E build. Rust entries now carry the file and the test's in-file module path (`crates/.../auth.rs::tests::name`). repeatRust runs one cargo invocation per package with target flags derived from the path (src/** builds --lib --bins, tests/<stem>.rs builds --test <stem>), so a buzz-relay change no longer compiles every test target of the crate. It lists candidates by in-file path, prefers the test whose full name the file's path predicts, and falls back to every suffix match only when nothing matches (a `#[path]` module), so a same-named test in another module is no longer repeated for a change it never saw. The rust-cache bust knob moves into the shared key (`unit-tests-sherpa-cache-v1`) in both the Unit Tests and flake-rust jobs, since rust-cache ignores `key` once shared-key is set; the contract script checks both and the flake restore's save-if, and the regression harness covers both. flake-select installs desktop dependencies only when a spec changed. Changed .rs files without test functions get a summary row, test.fail() declarations count as tests, a captured nextest 0.9.136 list/JUnit pair pins the classname contract, and CONTRIBUTING lists what the lanes deliberately do not repeat. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com> --- .github/workflows/_ci-flake.yml | 30 +- .github/workflows/_ci-rust.yml | 7 +- CONTRIBUTING.md | 14 +- scripts/flake-check.mjs | 275 +++++++++++++----- scripts/flake-check.test.mjs | 172 ++++++++++- .../test-rust-cache-contract-regressions.sh | 21 +- scripts/test-rust-cache-contract.sh | 25 +- 7 files changed, 434 insertions(+), 110 deletions(-) diff --git a/.github/workflows/_ci-flake.yml b/.github/workflows/_ci-flake.yml index 2cd77f16edb..37f6d228e13 100644 --- a/.github/workflows/_ci-flake.yml +++ b/.github/workflows/_ci-flake.yml @@ -35,18 +35,30 @@ jobs: # diff is exactly the pull request's change without a full clone. fetch-depth: 2 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + # Only a changed spec needs desktop dependencies: Playwright lists its + # smoke journeys and the desktop app's TypeScript parses it. Pull + # requests without one skip the install. + - name: Check for changed Playwright specs + id: specs + run: | + if git diff --quiet --diff-filter=AM HEAD^1 HEAD -- 'desktop/tests/e2e/*.spec.ts'; then + echo "changed=false" >> "$GITHUB_OUTPUT" + else + echo "changed=true" >> "$GITHUB_OUTPUT" + fi - name: Get pnpm store directory + if: steps.specs.outputs.changed == 'true' id: pnpm-cache run: echo "STORE_PATH=$(pnpm store path --silent)" >> "$GITHUB_OUTPUT" - name: Restore pnpm store cache + if: steps.specs.outputs.changed == 'true' uses: actions/cache/restore@caa296126883cff596d87d8935842f9db880ef25 # v5 with: path: ${{ steps.pnpm-cache.outputs.STORE_PATH }} key: pnpm-${{ runner.os }}-${{ hashFiles('**/pnpm-lock.yaml') }} restore-keys: pnpm-${{ runner.os }}- - # Playwright lists the smoke journeys of a changed spec, and the desktop - # app's TypeScript parses it. - name: Install desktop dependencies + if: steps.specs.outputs.changed == 'true' run: just desktop-install-ci - name: Map changed hunks to tests id: select @@ -64,13 +76,17 @@ jobs: - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 - uses: rui314/setup-mold@7e4f20ad28a2e8ca6fd0892ccf72e2abb706b9c3 # v1 - # Restores the Unit Tests lane's cache read-only: this lane only runs on - # pull requests, so it never saves one of its own. + # 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 — + # the CARGO*/RUST*/CC* environment variables, the lockfiles and the rustc + # version — must stay identical to the Unit Tests job's, so any env this + # job sets must be set there too (scripts/test-rust-cache-contract.sh + # checks the key). - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 with: - shared-key: unit-tests - key: sherpa-cache-v1 - save-if: ${{ github.event_name != 'pull_request' }} + shared-key: unit-tests-sherpa-cache-v1 + save-if: false - name: Install cargo-nextest uses: taiki-e/install-action@0fd46367812ee04360509b4169d9f659d6892bb2 # v2.79.15 with: diff --git a/.github/workflows/_ci-rust.yml b/.github/workflows/_ci-rust.yml index fa60042f672..3190d877ae7 100644 --- a/.github/workflows/_ci-rust.yml +++ b/.github/workflows/_ci-rust.yml @@ -58,9 +58,10 @@ jobs: - uses: rui314/setup-mold@7e4f20ad28a2e8ca6fd0892ccf72e2abb706b9c3 # v1 - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 with: - # Named so the advisory flake lane (_ci-flake.yml) can restore it. - shared-key: unit-tests - key: sherpa-cache-v1 + # Named so the advisory flake lane (_ci-flake.yml) can restore it; + # with shared-key set, rust-cache ignores `key`, so the cache-bust + # suffix lives here. Bump it to discard every saved cache. + shared-key: unit-tests-sherpa-cache-v1 save-if: ${{ github.event_name != 'pull_request' }} - name: Install cargo-nextest uses: taiki-e/install-action@0fd46367812ee04360509b4169d9f659d6892bb2 # v2.79.15 diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a5f3aa61f53..6d020a7f9ab 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -310,10 +310,16 @@ select` maps the diff against the base branch to the Rust tests, desktop changed: a hunk inside a test body selects that test; a change anywhere else in a test file (imports, helpers, hooks, the code under test beside Rust tests) or a new file selects the whole file. The `Flake check (…)` lanes then -run that selection ten times with retries off, so a repetition that fails is -a race in the test or the code under it, worth fixing before it lands. The -lanes skip when no test changed and are not required to merge; their step -summaries show the test × iteration table. +run that selection ten times with retries off; a repetition that fails is +usually a race in the test or the code under it, worth fixing before it +lands, though a test that needs infrastructure or the regular lane's +cross-file concurrency can fail every repetition instead. Some changed tests +are deliberately not repeated: `#[ignore]`d and Postgres-backed Rust tests, +Playwright specs outside the `smoke` project, `*.perf.ts` measurements, and a +selection too large for its lane (over 64 KiB of entries, or more Playwright +journeys than ten repetitions can finish within the lane's timeout). +The lanes skip when no test changed and are not required to merge; their +step summaries show the selection and the test × iteration table. --- diff --git a/scripts/flake-check.mjs b/scripts/flake-check.mjs index b1c24ed8958..e49d5c88e7e 100644 --- a/scripts/flake-check.mjs +++ b/scripts/flake-check.mjs @@ -6,6 +6,7 @@ import { spawnSync } from "node:child_process"; import { appendFileSync, copyFileSync, + existsSync, mkdirSync, readFileSync, rmSync, @@ -16,18 +17,26 @@ import { fileURLToPath } from "node:url"; const root = fileURLToPath(new URL("../", import.meta.url)); export const ITERATIONS = 10; // Repetitions spread across shards, so a whole-file selection stays inside -// the Playwright lane's job timeout. Playwright shards at file × repetition -// granularity (workers: 1, no fullyParallel), and the smoke lane runs ~1,500 -// journeys over ten ~11-minute shards, so five journeys × ten repetitions is a -// few minutes of test time per shard. The regular smoke lane never uses more -// than ten runners, so neither does this one. +// the Playwright lane's 45-minute job timeout. Playwright shards at file × +// repetition granularity (workers: 1, no fullyParallel). The regular smoke +// lane runs ~150 journeys per shard in ~11 minutes, about 4.5 s each, so five +// journeys × ten repetitions is a few minutes of test time per shard; it never +// uses more than ten runners, so neither does this one. Past ten shards each +// one grows instead, and the timeout minus dependency setup and the E2E build +// (~10 minutes, more when apt retries) leaves room for about 250 journey-runs +// (~19 minutes) per shard, so a selection beyond that many journeys skips the +// lane with a note rather than timing out. const JOURNEYS_PER_SHARD = 5; const MAX_SHARDS = 10; +const MAX_JOURNEY_RUNS_PER_SHARD = 250; export const playwrightShards = (journeys) => Math.min(MAX_SHARDS, Math.max(1, Math.ceil(journeys / JOURNEYS_PER_SHARD))); -// GitHub caps a job's outputs at 1 MiB in total. A selection this large is a -// sweeping refactor, not a test change worth ten repetitions, so the lane -// skips rather than overrunning its timeout. +export const MAX_PLAYWRIGHT_JOURNEYS = Math.floor( + (MAX_JOURNEY_RUNS_PER_SHARD * MAX_SHARDS) / ITERATIONS, +); +// GitHub caps a job's outputs at 1 MiB in total. A selection this large, like +// one past the journey cap, is a sweeping refactor rather than a test change +// worth ten repetitions, so the lane skips rather than overrunning its timeout. export const MAX_OUTPUT_BYTES = 64 * 1024; export const runners = ["rust", "node", "playwright", "dart"]; // Mirrors desktop/package.json "test": plain helpers run under the TS loader; @@ -55,7 +64,9 @@ const infraTests = // Mirrors desktop/package.json "test", desktop/playwright.config.ts testDir, // the root Cargo workspace's crates and mobile's test directory. Paths are -// later split on whitespace in a shell, so admit only plain names. +// later split on whitespace in a shell, so admit only plain names. The +// desktop `*.perf.ts` measurements are deliberately not specs here: they time +// the app rather than assert on it, so a slow repetition is not a flake. export function classify(file) { if (!/^[\w./@-]+$/.test(file)) return undefined; if (/^crates\/[^/]+\/(?:src|tests)\/(?!fixtures\/).*\.rs$/.test(file)) @@ -95,6 +106,7 @@ const declaring = [ "only", "skip", "fixme", + "fail", "fails", "todo", "concurrent", @@ -226,10 +238,18 @@ const matching = (text, open, close, from) => { // Every test function in a Rust file: a `fn` whose attribute block has an // attribute path ending in `test` (`#[test]`, `#[tokio::test]`, …), spanning -// the first attribute line to the body's closing brace. `ignored` marks -// `#[ignore]`, which nextest skips by default. +// the first attribute line to the body's closing brace. `name` is the path +// within the file through its inline modules (`tests::plain`), so a namesake +// in another module stays a different test. `ignored` marks `#[ignore]`, +// which nextest skips by default. export function rustTests(source) { const text = blankRust(source); + const modules = []; + const inline = /\bmod\s+(\w+)\s*\{/g; + for (let m = inline.exec(text); m; m = inline.exec(text)) { + const open = m.index + m[0].length - 1; + modules.push({ name: m[1], open, close: matching(text, "{", "}", open) }); + } const lineStarts = [0]; for (let i = 0; i < text.length; i += 1) if (text[i] === "\n") lineStarts.push(i + 1); @@ -263,8 +283,11 @@ export function rustTests(source) { if (!fn) continue; const open = text.indexOf("{", j + fn[0].length); if (open === -1) continue; + const path = modules + .filter((mod) => mod.open < match.index && match.index < mod.close) + .map((mod) => mod.name); tests.push({ - name: fn[1], + name: [...path, fn[1]].join("::"), start: lineAt(match.index), end: lineAt(matching(text, "{", "}", open)), ignored: paths.includes("ignore"), @@ -371,17 +394,50 @@ const summarize = (text) => { }; const cell = (value) => String(value).replaceAll("|", "\\|"); +const crateDir = (file) => `${root}crates/${file.split("/")[1]}`; + // The `[package]` name of the crate owning a file under crates/. export function cratePackage(file) { - const manifest = readFileSync( - `${root}crates/${file.split("/")[1]}/Cargo.toml`, - "utf8", - ); + const manifest = readFileSync(`${crateDir(file)}/Cargo.toml`, "utf8"); return /^\[package\][\s\S]*?^name\s*=\s*"([^"]+)"/m.exec(manifest)[1]; } -// Smoke journeys per spec file as Playwright lists them. Integration-only -// specs list nothing for the smoke project, so a selection of them stays empty. +// 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/<stem>.rs and +// tests/<dir>/*.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"]; + } + if (second === undefined) return ["--test", first.replace(/\.rs$/, "")]; + return existsSync(`${crateDir(file)}/tests/${first}/main.rs`) + ? ["--test", first] + : []; +} + +// The module path a file's items carry in their binary under cargo's layout +// conventions: src/a/b.rs is a::b, a mod.rs/main.rs/lib.rs names its directory +// or the root, src/bin/x.rs and tests/<target>.rs are roots, tests/<target>/x.rs +// is x. A `#[path]` attribute can break this; repeatRust then falls back to the +// in-file path alone. +export function rustModulePath(file) { + const [, , kind, ...parts] = file.replace(/\.rs$/, "").split("/"); + if (kind === "tests") parts.shift(); + else if (parts[0] === "bin") parts.splice(0, 2); + if (["mod", "main"].includes(parts.at(-1)) || parts.join() === "lib") + parts.pop(); + return parts; +} + +// Smoke journeys per spec file as Playwright lists them, each line once (a +// looped declaration is listed once per iteration). Integration-only specs +// list nothing for the smoke project, which Playwright reports as its "No +// tests found" error, so a selection of them stays empty. export function listPlaywright(files) { const result = run(`${root}bin/pnpm`, [ "-C", @@ -396,21 +452,24 @@ export function listPlaywright(files) { ...files, ]); const report = JSON.parse(result.stdout); - if (result.status !== 0 || report.errors.length) + const empty = + report.errors.length === 1 && + /No tests found/.test(report.errors[0].message); + if ((result.status !== 0 || report.errors.length) && !empty) throw new Error( `playwright --list: ${report.errors.map((error) => error.message).join("\n") || result.stderr}`, ); - const specs = new Map(files.map((file) => [file, []])); + const specs = new Map(files.map((file) => [file, new Set()])); const collect = (suites) => { for (const suite of suites) { for (const spec of suite.specs ?? []) if (spec.tests?.length) - specs.get(`desktop/tests/e2e/${spec.file}`)?.push(spec.line); + specs.get(`desktop/tests/e2e/${spec.file}`)?.add(spec.line); collect(suite.suites ?? []); } }; - collect(report.suites); - return specs; + collect(empty ? [] : report.suites); + return new Map([...specs].map(([file, lines]) => [file, [...lines]])); } function select(base = "HEAD^1", head = "HEAD") { @@ -454,9 +513,11 @@ function select(base = "HEAD^1", head = "HEAD") { previous: previous(file), }, ); - if (!selection) continue; - const crate = cratePackage(file); - entries = selection.names.map((name) => `${crate}::${name}`); + if (!selection) { + rows.push(`| ${cell(file)} | ${runner} | no test functions |`); + continue; + } + entries = selection.names.map((name) => `${file}::${name}`); note = selection.names.length === 0 ? "excluded (ignored tests only)" @@ -486,9 +547,16 @@ function select(base = "HEAD^1", head = "HEAD") { outputs[runner].push(...entries); rows.push(`| ${cell(file)} | ${runner} | ${cell(note)} |`); } - if (Buffer.byteLength(outputs[runner].join(" ")) > MAX_OUTPUT_BYTES) { + const exceeded = + Buffer.byteLength(outputs[runner].join(" ")) > MAX_OUTPUT_BYTES + ? `selection exceeds ${MAX_OUTPUT_BYTES} bytes` + : runner === "playwright" && + outputs[runner].length > MAX_PLAYWRIGHT_JOURNEYS + ? `selection exceeds ${MAX_PLAYWRIGHT_JOURNEYS} journeys, which ${ITERATIONS} repetitions would not finish in time` + : undefined; + if (exceeded) { rows.push( - `| *(${outputs[runner].length} ${runner} entries)* | ${runner} | selection exceeds ${MAX_OUTPUT_BYTES} bytes; lane skipped |`, + `| *(${outputs[runner].length} ${runner} entries)* | ${runner} | ${exceeded}; lane skipped |`, ); outputs[runner] = []; } @@ -587,45 +655,81 @@ export function parseJunit(xml) { return statuses; } -// nextest has no repeat flag, so run the filterset once per iteration under -// the `flake` profile (.config/nextest.toml: no retries, JUnit per run). The -// selection is listed first: a filterset matching no runnable test is a -// selection of ignored or infrastructure-backed tests, not a failure. +// The listed tests an entry stands for: a test the file's module path predicts +// exactly when nextest lists one, otherwise every test whose name ends in the +// in-file path (a `#[path]` module breaks the prediction; a namesake in another +// module is then repeated too rather than nothing). +export function chooseRust(tests, candidates) { + const chosen = new Map(); + for (const { path, exact } of tests) { + const suffix = candidates.filter( + ({ name }) => name === path || name.endsWith(`::${path}`), + ); + const predicted = suffix.filter(({ name }) => name === exact); + for (const test of predicted.length ? predicted : suffix) + chosen.set(`${test.binary} ${test.name}`, test); + } + 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) { - const byPackage = new Map(); + const groups = new Map(); for (const entry of entries) { - const [, crate, name] = /^(.+)::(\w+)$/.exec(entry) ?? []; - if (!crate) - throw new Error(`rust entry must be <package>::<test>: ${entry}`); - byPackage.set(crate, (byPackage.get(crate) ?? new Set()).add(name)); + const [, file, path] = + /^(crates\/[^:]+\.rs)::((?:\w+::)*\w+)$/.exec(entry) ?? []; + if (!file) + throw new Error(`rust entry must be <file>::<module::test>: ${entry}`); + const crate = cratePackage(file); + const group = groups.get(crate) ?? { targets: new Set(), tests: [] }; + const targets = rustTarget(file); + if (targets.length === 0) group.every = true; + for (const target of targets) group.targets.add(target); + group.tests.push({ + path, + exact: [...rustModulePath(file), path].join("::"), + }); + groups.set(crate, group); } - const filterset = `(${[...byPackage] - .map( - ([crate, names]) => - `(package(=${crate}) & (${[...names].map((name) => `test(/(^|::)${name}$/)`).join(" | ")}))`, - ) - .join(" | ")}) & !(${infraTests})`; - const packages = [...byPackage.keys()].flatMap((crate) => ["-p", crate]); - const listed = run("cargo", [ - "nextest", - "list", - ...packages, - "-E", - filterset, - "--message-format", - "json", - ]); - if (listed.status !== 0) - throw new Error(`cargo nextest list: ${listed.stderr}`); - const expected = Object.entries( - JSON.parse(listed.stdout)["rust-suites"], - ).flatMap(([binary, suite]) => - Object.entries(suite.testcases) - .filter( - ([, test]) => - !test.ignored && test["filter-match"].status === "matches", - ) - .map(([name]) => `${binary} ${name}`), + const runs = []; + for (const [crate, group] of groups) { + group.args = ["-p", crate, ...(group.every ? [] : group.targets)]; + const suffixes = [...new Set(group.tests.map(({ path }) => path))] + .map((path) => `test(/(^|::)${path}$/)`) + .join(" | "); + const listed = run("cargo", [ + "nextest", + "list", + ...group.args, + "-E", + `(${suffixes}) & !(${infraTests})`, + "--message-format", + "json", + ]); + if (listed.status !== 0) + throw new Error(`cargo nextest list: ${listed.stderr}`); + const candidates = Object.entries( + JSON.parse(listed.stdout)["rust-suites"], + ).flatMap(([binary, suite]) => + Object.entries(suite.testcases) + .filter( + ([, test]) => + !test.ignored && test["filter-match"].status === "matches", + ) + .map(([name]) => ({ binary, name })), + ); + group.chosen = chooseRust(group.tests, candidates); + group.filterset = group.chosen + .map(({ binary, name }) => `(binary_id(=${binary}) & test(=${name}))`) + .join(" | "); + } + const expected = [...groups.values()].flatMap((group) => + group.chosen.map(({ binary, name }) => `${binary} ${name}`), ); if (expected.length === 0) { summarize( @@ -637,24 +741,35 @@ function repeatRust(entries, iterations) { // CARGO_TARGET_DIR; the `flake` profile writes its JUnit report there. const junit = `${root}target/nextest/flake/junit.xml`; const dir = resultsDir("rust"); - const runs = []; for (let i = 1; i <= iterations; i += 1) { - rmSync(junit, { force: true }); - const result = run( - "cargo", - ["nextest", "run", "--profile", "flake", ...packages, "-E", filterset], - { stdio: "inherit" }, - ); - let statuses = new Map(); - try { - statuses = parseJunit(readFileSync(junit, "utf8")); - copyFileSync(junit, `${dir}/iter-${i}.xml`); - } catch { - statuses = new Map(); + const statuses = new Map(); + let ok = true; + for (const [crate, group] of groups) { + if (group.chosen.length === 0) continue; + rmSync(junit, { force: true }); + const result = run( + "cargo", + [ + "nextest", + "run", + "--profile", + "flake", + ...group.args, + "-E", + group.filterset, + ], + { stdio: "inherit" }, + ); + ok &&= result.status === 0; + try { + for (const [test, status] of parseJunit(readFileSync(junit, "utf8"))) + statuses.set(test, status); + copyFileSync(junit, `${dir}/iter-${i}-${crate}.xml`); + } catch { + ok = false; + } } - const ok = - result.status === 0 && - expected.every((test) => statuses.get(test) === "passed"); + ok &&= expected.every((test) => statuses.get(test) === "passed"); runs.push({ ok, statuses }); progress(i, iterations, ok); } diff --git a/scripts/flake-check.test.mjs b/scripts/flake-check.test.mjs index 876675b973e..21b4006ca23 100644 --- a/scripts/flake-check.test.mjs +++ b/scripts/flake-check.test.mjs @@ -2,16 +2,21 @@ import assert from "node:assert/strict"; import { readFileSync } from "node:fs"; import test from "node:test"; import { + chooseRust, classify, cratePackage, dartDefine, hunkRanges, + ITERATIONS, listPlaywright, + MAX_PLAYWRIGHT_JOURNEYS, nodeTestArgs, parseDartReport, parseJunit, playwrightRepetitions, playwrightShards, + rustModulePath, + rustTarget, rustTests, selectRust, selectTests, @@ -108,12 +113,16 @@ test("Playwright test calls span the declaration; groups, hooks and configuratio " }", "});", 'test.describe.configure({ mode: "serial" });', + 'test.fail("known broken", async () => {', + " await Promise.resolve();", + "});", "", ].join("\n"); const calls = testCalls(spec, "x.spec.ts"); assert.deepEqual(calls, [ { start: 8, end: 11 }, { start: 13, end: 15 }, + { start: 19, end: 21 }, ]); // Playwright lists a looped declaration once per iteration, at one line. assert.deepEqual(testBodies(calls, [13, 13, 8]), [ @@ -170,22 +179,104 @@ const rustSource = [ "", ].join("\n"); -test("Rust test functions span their attribute block to the closing brace, through strings, chars and comments", () => { +test("Rust test functions span their attribute block to the closing brace, through strings, chars and comments, named by their inline module path", () => { assert.deepEqual(rustTests(rustSource), [ - { name: "plain", start: 19, end: 22, ignored: false }, - { name: "needs_infra", start: 24, end: 30, ignored: true }, - { name: "also_ignored", start: 32, end: 34, ignored: true }, - { name: "in_nested_module", start: 37, end: 40, ignored: false }, + { name: "tests::plain", start: 19, end: 22, ignored: false }, + { name: "tests::needs_infra", start: 24, end: 30, ignored: true }, + { name: "tests::also_ignored", start: 32, end: 34, ignored: true }, + { + name: "tests::postgres_tests::in_nested_module", + start: 37, + end: 40, + ignored: false, + }, ]); assert.deepEqual(rustTests("fn main() {}\n"), []); }); +test("same-named Rust tests in different modules are distinct, and a hunk selects only its own", () => { + const source = [ + "mod alpha {", + " #[test]", + " fn same() {}", + "}", + "pub(crate) mod beta {", + " mod tests {", + " #[test]", + " fn same() { let _ = 1; }", + " }", + "}", + "", + ].join("\n"); + assert.deepEqual( + rustTests(source).map((test) => test.name), + ["alpha::same", "beta::tests::same"], + ); + assert.deepEqual( + selectRust(diff([8, 1, 8, 1]), source, { previous: source }), + { + lines: [7], + names: ["beta::tests::same"], + }, + ); + // The file's path predicts the rest of the name; cargo's layout rules. + for (const [file, modules] of [ + ["crates/buzz-relay/src/lib.rs", []], + ["crates/buzz-relay/src/main.rs", []], + ["crates/buzz-relay/src/api/mod.rs", ["api"]], + ["crates/buzz-relay/src/handlers/auth.rs", ["handlers", "auth"]], + ["crates/buzz-test-client/src/bin/mention.rs", []], + ["crates/buzz-test-client/tests/e2e_relay.rs", []], + ["crates/x/tests/suite/main.rs", []], + ["crates/x/tests/suite/helpers.rs", ["helpers"]], + ]) + assert.deepEqual(rustModulePath(file), modules, file); + // Listed candidates: the predicted name wins; without it every suffix + // match is kept rather than nothing. + const candidates = [ + { binary: "buzz-relay", name: "api::relay_members::tests::same" }, + { binary: "buzz-relay", name: "handlers::auth::tests::same" }, + { binary: "buzz-relay", name: "handlers::auth::tests::other" }, + { + binary: "buzz-relay::bin/buzz-relay", + name: "handlers::auth::tests::same", + }, + ]; + assert.deepEqual( + chooseRust( + [{ path: "tests::same", exact: "handlers::auth::tests::same" }], + candidates, + ), + [candidates[1], candidates[3]], + ); + assert.deepEqual( + chooseRust( + [ + { path: "tests::same", exact: "elsewhere::tests::same" }, + { path: "tests::other", exact: "handlers::auth::tests::other" }, + ], + candidates, + ), + [candidates[0], candidates[1], candidates[3], candidates[2]], + ); +}); + +test("Rust files build only the targets they can be part of", () => { + 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-agent/tests/common/mod.rs", []], + ]) + assert.deepEqual(rustTarget(file), flags, file); +}); + test("Rust selection: a hunk inside a test selects it, elsewhere selects every runnable test; ignored tests never run", () => { const select = (text, options) => selectRust(text, rustSource, { previous: rustSource, ...options }); assert.deepEqual(select(diff([21, 1, 21, 1])), { lines: [19], - names: ["plain"], + names: ["tests::plain"], }); // Inside an ignored test: owned, so not shared code, but nothing to run. assert.deepEqual(select(diff([28, 0, 29, 1])), { lines: [24], names: [] }); @@ -194,7 +285,7 @@ test("Rust selection: a hunk inside a test selects it, elsewhere selects every r const whole = (reason) => ({ whole: true, reason, - names: ["plain", "in_nested_module"], + names: ["tests::plain", "tests::postgres_tests::in_nested_module"], }); assert.deepEqual(select(diff([4, 0, 5, 1])), whole("shared code changed")); assert.deepEqual( @@ -281,25 +372,84 @@ test("hunks inside tests select those tests; shared code, new or unlisted files ); }); -test("Playwright lists smoke journeys per spec; integration-only specs list none", () => { +test("Playwright lists smoke journeys per spec once each; integration-only specs list none", () => { const smoke = "desktop/tests/e2e/channel-sort.spec.ts"; + const looped = "desktop/tests/e2e/claude-effort-picker.spec.ts"; const integration = "desktop/tests/e2e/agents.spec.ts"; - const listed = listPlaywright([smoke, integration]); + // Alone, an integration spec makes Playwright exit 1 with "No tests found". + assert.deepEqual(listPlaywright([integration]), new Map([[integration, []]])); + const listed = listPlaywright([smoke, looped, integration]); assert.deepEqual(listed.get(integration), []); const lines = listed.get(smoke); assert.ok(lines.length > 0, "a smoke spec must list its journeys"); assert.equal( testBodies(testCalls(read(`../${smoke}`), smoke), lines).length, - new Set(lines).size, + lines.length, "every listed journey must be owned by a test call", ); + // A looped declaration is listed once per iteration at the same line. + const loopedLines = listed.get(looped); + assert.deepEqual(loopedLines, [...new Set(loopedLines)]); + assert.ok( + /^\s*for \(const .+\) \{\n\s+test\(/m.test(read(`../${looped}`)), + "the looped spec must still declare a test inside a loop", + ); }); -test("Playwright repetitions get one shard per five journeys, at most ten", () => { +test("Playwright repetitions get one shard per five journeys, at most ten, and a selection past ~19 minutes per shard skips", () => { assert.deepEqual( [0, 1, 5, 6, 18, 40, 41, 50, 51, 200].map(playwrightShards), [1, 1, 1, 2, 4, 8, 9, 10, 10, 10], ); + // 250 journey-runs per shard at ~4.5 s each, over ten shards. + assert.equal(MAX_PLAYWRIGHT_JOURNEYS, 250); + assert.equal(ITERATIONS, 10); +}); + +test("nextest's JUnit classname is the binary id its list reports", () => { + // Captured from cargo-nextest 0.9.136 on one buzz-core test: `cargo nextest + // list -p buzz-core --lib -E 'test(=kind::tests::no_duplicate_kind_values)' + // --message-format json` (trimmed to the matching testcase) and the + // target/nextest/flake/junit.xml of `cargo nextest run --profile flake` with + // the same arguments. repeatRust keys expectations as `<binary-id> <name>` + // from the list and reads statuses as `<classname> <name>` from the report, + // so the two must agree. + const list = { + "rust-suites": { + "buzz-core": { + "package-name": "buzz-core", + "binary-id": "buzz-core", + "binary-name": "buzz_core", + kind: "lib", + status: "listed", + testcases: { + "kind::tests::no_duplicate_kind_values": { + kind: "test", + ignored: false, + "filter-match": { status: "matches" }, + }, + }, + }, + }, + }; + const junit = [ + '<?xml version="1.0" encoding="UTF-8"?>', + '<testsuites name="nextest-run" tests="1" failures="0" errors="0" uuid="01abf963-07b0-46d9-8f00-e212969d85a8" timestamp="2026-10-27T00:32:44.093+11:00" time="0.042">', + ' <testsuite name="buzz-core" tests="1" disabled="0" errors="0" failures="0">', + ' <testcase name="kind::tests::no_duplicate_kind_values" classname="buzz-core" timestamp="2026-10-27T00:32:44.094+11:00" time="0.040">', + " </testcase>", + " </testsuite>", + "</testsuites>", + "", + ].join("\n"); + const expected = Object.entries(list["rust-suites"]).flatMap( + ([binary, suite]) => + Object.keys(suite.testcases).map((name) => `${binary} ${name}`), + ); + assert.deepEqual( + parseJunit(junit), + new Map(expected.map((test) => [test, "passed"])), + ); }); test("reports give per-test statuses per iteration", () => { diff --git a/scripts/test-rust-cache-contract-regressions.sh b/scripts/test-rust-cache-contract-regressions.sh index 0275b873415..a184c3d7454 100755 --- a/scripts/test-rust-cache-contract-regressions.sh +++ b/scripts/test-rust-cache-contract-regressions.sh @@ -139,13 +139,28 @@ YAML run_contract >/dev/null rm -rf "$tmp/.github/actions" "$tmp/.github/workflows/new-cache-user.yaml" +cp "$root/.github/workflows/_ci-flake.yml" "$tmp/flake.orig" +python3 - "$tmp/.github/workflows/_ci-flake.yml" <<'PY' +import pathlib +import sys + +path = pathlib.Path(sys.argv[1]) +workflow = path.read_text() +needle = " shared-key: unit-tests-sherpa-cache-v1\n save-if: false\n" +if workflow.count(needle) != 1: + raise SystemExit("expected one flake-rust cache restore key") +path.write_text(workflow.replace(needle, " shared-key: unit-tests\n save-if: false\n")) +PY +expect_failure 'flake-rust rust-cache action must restore with.shared-key unit-tests-sherpa-cache-v1' +mv "$tmp/flake.orig" "$tmp/.github/workflows/_ci-flake.yml" + python3 - "$tmp/.github/workflows/_ci-rust.yml" <<'PY' import pathlib import sys path = pathlib.Path(sys.argv[1]) workflow = path.read_text() -needle = " key: sherpa-cache-v1\n save-if:" +needle = " shared-key: unit-tests-sherpa-cache-v1\n save-if:" replacement = " save-if:" if workflow.count(needle) != 1: raise SystemExit("expected one Unit Tests cache generation key") @@ -153,9 +168,9 @@ workflow = workflow.replace(needle, replacement) job = " unit-tests:\n" if workflow.count(job) != 1: raise SystemExit("expected one Unit Tests job") -workflow = workflow.replace(job, job + " env:\n key: sherpa-cache-v1\n", 1) +workflow = workflow.replace(job, job + " env:\n shared-key: unit-tests-sherpa-cache-v1\n", 1) path.write_text(workflow) PY -expect_failure 'rust-cache action must keep with.key set to sherpa-cache-v1' +expect_failure 'rust-cache action must keep with.shared-key set to unit-tests-sherpa-cache-v1' echo "rust cache contract regressions passed" diff --git a/scripts/test-rust-cache-contract.sh b/scripts/test-rust-cache-contract.sh index cf69c8aadc6..18cc7c8c868 100755 --- a/scripts/test-rust-cache-contract.sh +++ b/scripts/test-rust-cache-contract.sh @@ -104,9 +104,30 @@ cache_steps = steps.select do |step| end abort "Unit Tests must contain exactly one rust-cache action step" unless cache_steps.length == 1 +# With shared-key set, rust-cache ignores `key`, so the cache-bust suffix +# lives in the shared key; the advisory flake lane restores the same cache +# and must name it identically. +shared_key = "unit-tests-sherpa-cache-v1" cache_with = cache_steps.first["with"] -unless cache_with.is_a?(Hash) && cache_with["key"] == "sherpa-cache-v1" - abort "Unit Tests rust-cache action must keep with.key set to sherpa-cache-v1" +unless cache_with.is_a?(Hash) && cache_with["shared-key"] == shared_key && !cache_with.key?("key") + abort "Unit Tests rust-cache action must keep with.shared-key set to #{shared_key} and no with.key" +end + +flake_ci_path = root / ".github" / "workflows" / "_ci-flake.yml" +flake_ci = workflows.fetch(flake_ci_path) { load_workflow.call(flake_ci_path) } +flake_jobs = flake_ci["jobs"] +flake_rust = flake_jobs.is_a?(Hash) ? flake_jobs["flake-rust"] : nil +abort "flake-rust job missing" unless flake_rust.is_a?(Hash) +flake_steps = flake_rust["steps"] +abort "flake-rust steps missing" unless flake_steps.is_a?(Array) +flake_cache_steps = flake_steps.select do |step| + step.is_a?(Hash) && step["uses"].is_a?(String) && + step["uses"].match?(/\ASwatinem\/rust-cache@/i) +end +abort "flake-rust must contain exactly one rust-cache action step" unless flake_cache_steps.length == 1 +flake_with = flake_cache_steps.first["with"] +unless flake_with.is_a?(Hash) && flake_with["shared-key"] == shared_key && flake_with["save-if"] == false + abort "flake-rust rust-cache action must restore with.shared-key #{shared_key} with save-if false" end renovate = JSON.parse(Pathname(renovate_name).binread.force_encoding("UTF-8")) From df954981683082affc4a6a01bc963d913ff7d4f2 Mon Sep 17 00:00:00 2001 From: Matt Toohey <contact@matttoohey.com> Date: Tue, 27 Oct 2026 08:41:13 +1100 Subject: [PATCH 3/7] fix(ci): emit one --test flag per Rust integration target and correct action pin comments Address the Codex security review and the zizmor code-scanning alerts (386-390) on #8613. repeatRust merged each package's cargo target flags through a Set of argv tokens, so two integration-test files in one package collapsed `--test codec --test connection` into `--test codec connection`. nextest reads the trailing `connection` as a test-name filter, lists nothing, and the lane took the "nothing to repeat" branch and reported success without running a test (verified against buzz-audio-client: the collapsed argv lists no tests and exits 0). rustTarget now returns one flag per entry and the new rustGroups keys each whole flag, so a `--test <name>` is emitted once per name while `--lib`/`--bins` stay single; the regression test asserts the exact argv for two integration files and for a src/ plus tests/ file in the same package, through the seam repeatRust uses. The `-p`, `-E` and filterset arguments were re-read for the same class of bug and pass single values. In _ci-flake.yml, actions/upload-artifact 043fb46d is v7.0.1 (the `v7` tag has since moved to v7.0.2), so its four comments now say so, as the other workflows do. rui314/setup-mold 7e4f20ad is a commit the `v1` tag no longer points at (339e96b4, mold 3.0.0), so the flake workflow pins the current `v1`; _ci-rust.yml, _ci-relay.yml and _ci-desktop.yml carry the same stale pin and are left for a separate change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com> --- .github/workflows/_ci-flake.yml | 10 +++---- scripts/flake-check.mjs | 53 ++++++++++++++++++++++----------- scripts/flake-check.test.mjs | 40 ++++++++++++++++++++++--- 3 files changed, 76 insertions(+), 27 deletions(-) diff --git a/.github/workflows/_ci-flake.yml b/.github/workflows/_ci-flake.yml index 37f6d228e13..f2b4f8c23ef 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 e49d5c88e7e..fcb1b8a27a7 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/<stem>.rs and -// tests/<dir>/*.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 <name>`: src/** is the lib (when the crate has one) or a bin, +// tests/<stem>.rs and tests/<dir>/*.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 <crate>` 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 <file>::<module::test>: ${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 21b4006ca23..fa868c993a7 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", () => { From 5568e2680fceedd4774c61ecabdccf9f47c4f3c5 Mon Sep 17 00:00:00 2001 From: Matt Toohey <contact@matttoohey.com> Date: Tue, 27 Oct 2026 11:47:07 +1100 Subject: [PATCH 4/7] fix(ci): repeat desktop Tauri tests and helper-dependent Rust binaries Address the two High findings of review 5659898932 on #8613: a changed Rust test under desktop/src-tauri, or a changed helper module under a crate's tests/, produced an empty flake selection and every repeat lane skipped, a false "nothing selected" green. Desktop Tauri Rust is now a runner of its own rather than an unclassified path. rustFile places a Rust file in its workspace (the root crates, or desktop/src-tauri and its crates/*), and classify, cratePackage, rustTarget, rustModulePath and rustGroups read that instead of assuming crates/<crate>/. The new flake-tauri lane mirrors the Desktop Tauri core lane's setup (its apt packages, sidecar stubs and desktop-core rust-cache, restored read-only) and runs the same script against desktop/src-tauri/Cargo.toml. nextest reads its configuration from the workspace root, so that workspace gets its own .config/nextest.toml with the flake profile, and the Postgres exclusion applies only in the root workspace: nextest rejects a binary(/^postgres_/) predicate where no such binary exists, which the Tauri workspace's first live run showed. Supporting the surface for real was straightforward, since the workspace is an ordinary cargo workspace that nextest lists and runs by manifest path, so no out-of-scope declaration was needed. A tests/** file without test functions is a helper that integration binaries compile in with `mod common;`. select now finds the package's integration targets that include it (rustIncludes reads `mod <first>;` declarations with comments and strings blanked, `#[path]` attributes, and a tests/<dir>/main.rs owning its directory; a helper no target names falls back to every target) and selects each of those binaries whole, so the ignored and infrastructure exclusions still apply, with a row naming the importers. A helper and its importer changed together select each test once. A src/** file without test functions still selects nothing; a comment beside the row records that this is deliberate, since the regular lanes run what depends on it. Regression tests go through the real select entry point over a synthetic head tree (HEAD with a comment appended, as the reviewer probed): a changed desktop/src-tauri/tests/rodio_mixer_diagnostic.rs yields its three tests under tauri=, and a comment-only change to crates/buzz-agent/tests/common/mod.rs yields rust= entries in exactly anthropic_continuation, fake_llm, permission_boundary and regressions, the four files declaring `mod common;`. Verified live: cargo nextest list with the produced -p/--test flags and filterset matches all 85 entries, and `flake-check.mjs tauri --iterations=2` on the Tauri selection ran green end to end with its JUnit report under desktop/src-tauri/target/nextest. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com> --- .github/workflows/_ci-flake.yml | 66 ++++++ CONTRIBUTING.md | 12 +- desktop/src-tauri/.config/nextest.toml | 14 ++ scripts/flake-check.mjs | 260 ++++++++++++++++++----- scripts/flake-check.test.mjs | 274 ++++++++++++++++++++++++- 5 files changed, 564 insertions(+), 62 deletions(-) create mode 100644 desktop/src-tauri/.config/nextest.toml diff --git a/.github/workflows/_ci-flake.yml b/.github/workflows/_ci-flake.yml index f2b4f8c23ef..2ab3542e4bc 100644 --- a/.github/workflows/_ci-flake.yml +++ b/.github/workflows/_ci-flake.yml @@ -23,6 +23,7 @@ jobs: contents: read outputs: rust: ${{ steps.select.outputs.rust }} + tauri: ${{ steps.select.outputs.tauri }} node: ${{ steps.select.outputs.node }} playwright: ${{ steps.select.outputs.playwright }} playwright_shards: ${{ steps.select.outputs.playwright_shards }} @@ -106,6 +107,71 @@ jobs: if-no-files-found: ignore retention-days: 7 + flake-tauri: + name: Flake check (Desktop Tauri) + needs: [flake-select] + if: needs.flake-select.outputs.tauri != '' + runs-on: ubuntu-latest + # Same bound as a Desktop Tauri lane (_ci-desktop.yml): ci-apt-retry.sh + # bounds the apt step at ~34m worst case and a cold build needs ~15m. + timeout-minutes: 55 + permissions: + contents: read + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + - uses: cashapp/activate-hermit@cea9af7913204a965fd488637a8d1811bba2e616 # v1 + - uses: rui314/setup-mold@339e96b4ff1f3bd865856ba4cbbe8818a7acb038 # v1 + # Restores the Desktop Tauri core lane's cache (_ci-desktop.yml) + # read-only, under the same constraints as flake-rust's restore above: + # the workspace, shared-key and environment hash must stay identical to + # that job's. + - uses: Swatinem/rust-cache@e18b497796c12c097a38f9edb9d0641fb99eee32 + with: + workspaces: desktop/src-tauri + shared-key: desktop-core + save-if: false + - name: Install Tauri dependencies (Linux) + env: + DEBIAN_FRONTEND: noninteractive + run: | + CI_APT_ATTEMPT_SECONDS=90 scripts/ci-apt-retry.sh sudo apt-get update + scripts/ci-apt-retry.sh sudo apt-get install -y --no-install-recommends \ + build-essential \ + curl \ + file \ + libasound2-dev \ + libayatana-appindicator3-dev \ + libgtk-3-dev \ + librsvg2-dev \ + libssl-dev \ + libwebkit2gtk-4.1-dev \ + libxdo-dev \ + patchelf \ + wget + - name: Install cargo-nextest + uses: taiki-e/install-action@0fd46367812ee04360509b4169d9f659d6892bb2 # v2.79.15 + with: + tool: cargo-nextest@0.9.136 + # tauri-build validates the sidecar binaries at compile time; the Desktop + # Tauri recipes create the same empty stubs. + - name: Create sidecar stubs + run: just _ensure-sidecar-stubs + # Same script as flake-rust, against the Tauri workspace's manifest and + # its own .config/nextest.toml `flake` profile. + - name: Repeat changed desktop Tauri tests + env: + CMAKE_POLICY_VERSION_MINIMUM: "3.5" + SELECTED: ${{ needs.flake-select.outputs.tauri }} + run: node scripts/flake-check.mjs tauri --iterations=10 $SELECTED + - name: Desktop Tauri iteration reports + if: ${{ !cancelled() }} + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + with: + name: flake-tauri + path: test-results/flake + if-no-files-found: ignore + retention-days: 7 + flake-node: name: Flake check (node:test) needs: [flake-select] diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6d020a7f9ab..95260d2ed56 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -305,11 +305,13 @@ This is the same check that runs in CI. PRs that fail `just ci` will not be merged. If `just ci` fails on formatting, `just fix-all` fixes it in one shot (`rustfmt` + Tauri fmt + desktop, web, and mobile formatters). Pull requests also get an advisory **flake check**. `scripts/flake-check.mjs -select` maps the diff against the base branch to the Rust tests, desktop -`node --test` files, Playwright smoke journeys and Flutter test files it -changed: a hunk inside a test body selects that test; a change anywhere else -in a test file (imports, helpers, hooks, the code under test beside Rust -tests) or a new file selects the whole file. The `Flake check (…)` lanes then +select` maps the diff against the base branch to the Rust tests (in the root +and desktop Tauri workspaces), desktop `node --test` files, Playwright smoke +journeys and Flutter test files it changed: a hunk inside a test body selects +that test; a change anywhere else in a test file (imports, helpers, hooks, +the code under test beside Rust tests) or a new file selects the whole file, +and a change to a helper module under a crate's `tests/` selects every +integration test binary that includes it. The `Flake check (…)` lanes then run that selection ten times with retries off; a repetition that fails is usually a race in the test or the code under it, worth fixing before it lands, though a test that needs infrastructure or the regular lane's diff --git a/desktop/src-tauri/.config/nextest.toml b/desktop/src-tauri/.config/nextest.toml new file mode 100644 index 00000000000..d398fdc2eab --- /dev/null +++ b/desktop/src-tauri/.config/nextest.toml @@ -0,0 +1,14 @@ +# nextest reads its configuration from the workspace root, so the desktop +# Tauri workspace carries its own. Only the advisory pull-request flake lane +# (scripts/flake-check.mjs tauri) runs nextest here; the Desktop Tauri lanes +# use `cargo test`. + +[profile.flake] +# Mirrors the root .config/nextest.toml profile: the lane repeats a selection +# and must see every failed repetition, so no retries; the JUnit report gives +# it per-test statuses. +retries = 0 +fail-fast = false + +[profile.flake.junit] +path = "junit.xml" diff --git a/scripts/flake-check.mjs b/scripts/flake-check.mjs index fcb1b8a27a7..224296c4c05 100644 --- a/scripts/flake-check.mjs +++ b/scripts/flake-check.mjs @@ -8,10 +8,12 @@ import { copyFileSync, existsSync, mkdirSync, + readdirSync, readFileSync, rmSync, } from "node:fs"; import { createRequire } from "node:module"; +import { posix as path } from "node:path"; import { fileURLToPath } from "node:url"; const root = fileURLToPath(new URL("../", import.meta.url)); @@ -38,7 +40,43 @@ export const MAX_PLAYWRIGHT_JOURNEYS = Math.floor( // one past the journey cap, is a sweeping refactor rather than a test change // worth ten repetitions, so the lane skips rather than overrunning its timeout. export const MAX_OUTPUT_BYTES = 64 * 1024; -export const runners = ["rust", "node", "playwright", "dart"]; +export const runners = ["rust", "tauri", "node", "playwright", "dart"]; +// The two cargo workspaces, by runner: the root workspace's crates, and the +// desktop Tauri workspace (desktop/src-tauri and its crates/*), which the +// Desktop Tauri lane (_ci-desktop.yml) builds with its own manifest, system +// dependencies and cache. Each is repeated by its own lane with nextest run +// against that manifest. +export const rustWorkspaces = { + rust: "Cargo.toml", + tauri: "desktop/src-tauri/Cargo.toml", +}; +const rustLayouts = [ + { + runner: "rust", + pattern: /^(crates\/[^/]+)\/(src|tests)\/((?!fixtures\/).*)\.rs$/, + }, + { + runner: "tauri", + pattern: + /^(desktop\/src-tauri(?:\/crates\/[^/]+)?)\/(src|tests)\/((?!fixtures\/).*)\.rs$/, + }, +]; + +// A Rust file's place in its workspace: the runner, its package directory, +// whether it is under src or tests, and its path below that without `.rs`. +export function rustFile(file) { + for (const { runner, pattern } of rustLayouts) { + const match = pattern.exec(file); + if (match) + return { + runner, + dir: match[1], + kind: match[2], + parts: match[3].split("/"), + }; + } + return undefined; +} // Mirrors desktop/package.json "test": plain helpers run under the TS loader; // jsdom tests also preload the DOM shim and force-exit past their timers. export const nodeTestArgs = (file) => @@ -58,19 +96,34 @@ export const dartDefine = "--dart-define=BUZZ_PUSH_GATEWAY_URL=https://push.example"; // Mirrors the postgres-ci default-filter in .config/nextest.toml: those tests // are #[ignore]d and need Postgres, Redis, S3 or MinIO, so a selection that -// reached one would fail every repetition without being flaky. -const infraTests = - "test(/postgres_tests::/) | binary(/^postgres_/) | test(/(^|::)external_infra[^:]*::/)"; +// reached one would fail every repetition without being flaky. The convention +// is the root workspace's; nextest rejects a `binary()` predicate matching no +// test binary in the workspace, and the Tauri workspace has none, so it gets +// no exclusion. +const infraTests = { + rust: "test(/postgres_tests::/) | binary(/^postgres_/) | test(/(^|::)external_infra[^:]*::/)", +}; + +// The nextest filterset listing a workspace's candidates for the selected +// in-file test paths, less its infrastructure-backed tests. +export function rustFilterset(runner, paths) { + const suffixes = [...new Set(paths)] + .map((path) => `test(/(^|::)${path}$/)`) + .join(" | "); + return infraTests[runner] + ? `(${suffixes}) & !(${infraTests[runner]})` + : `(${suffixes})`; +} // Mirrors desktop/package.json "test", desktop/playwright.config.ts testDir, -// the root Cargo workspace's crates and mobile's test directory. Paths are +// the two cargo workspaces' packages and mobile's test directory. Paths are // later split on whitespace in a shell, so admit only plain names. The // desktop `*.perf.ts` measurements are deliberately not specs here: they time // the app rather than assert on it, so a slow repetition is not a flake. export function classify(file) { if (!/^[\w./@-]+$/.test(file)) return undefined; - if (/^crates\/[^/]+\/(?:src|tests)\/(?!fixtures\/).*\.rs$/.test(file)) - return "rust"; + const rust = rustFile(file); + if (rust) return rust.runner; if ( /^desktop\/(?:src\/.*\.(?:test|jsdom-test)\.mjs|scripts\/[^/]+\.test\.mjs)$/.test( file, @@ -350,7 +403,8 @@ export function selectTests( // Rust tests a diff selects: hunks inside a test body select that test, any // other hunk selects every test in the file (the module's code under test // lives beside them), and `#[ignore]`d tests own their lines but never run. -// A file without test functions is not a test file and selects nothing. +// A file without test functions selects nothing of its own; `select` decides +// what that means by where the file lives. export function selectRust( diff, source, @@ -394,9 +448,9 @@ const summarize = (text) => { }; const cell = (value) => String(value).replaceAll("|", "\\|"); -const crateDir = (file) => `${root}crates/${file.split("/")[1]}`; +const crateDir = (file) => `${root}${rustFile(file).dir}`; -// The `[package]` name of the crate owning a file under crates/. +// The `[package]` name of the package owning a Rust file. export function cratePackage(file) { const manifest = readFileSync(`${crateDir(file)}/Cargo.toml`, "utf8"); return /^\[package\][\s\S]*?^name\s*=\s*"([^"]+)"/m.exec(manifest)[1]; @@ -409,16 +463,16 @@ export function cratePackage(file) { // 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("/"); + const { kind, parts } = rustFile(file); 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"]]; } - if (second === undefined) return [["--test", first.replace(/\.rs$/, "")]]; - return existsSync(`${crateDir(file)}/tests/${first}/main.rs`) - ? [["--test", first]] + if (parts.length === 1) return [["--test", parts[0]]]; + return existsSync(`${crateDir(file)}/tests/${parts[0]}/main.rs`) + ? [["--test", parts[0]]] : []; } @@ -428,7 +482,8 @@ export function rustTarget(file) { // is x. A `#[path]` attribute can break this; repeatRust then falls back to the // in-file path alone. export function rustModulePath(file) { - const [, , kind, ...parts] = file.replace(/\.rs$/, "").split("/"); + const { kind, parts: below } = rustFile(file); + const parts = [...below]; if (kind === "tests") parts.shift(); else if (parts[0] === "bin") parts.splice(0, 2); if (["mod", "main"].includes(parts.at(-1)) || parts.join() === "lib") @@ -436,6 +491,60 @@ export function rustModulePath(file) { return parts; } +// Whether an integration-test target (tests/<stem>.rs or tests/<dir>/main.rs) +// compiles a helper file under tests/ into its binary: a main.rs owns its +// directory, and any target's source declares `mod <first>;` for the +// helper's first path component under tests/ (a crate root resolves +// `mod common;` to tests/common/mod.rs or tests/common.rs), or a `#[path]` +// attribute names the helper's file or the mod.rs of a directory holding it. +// Declarations are read with comments and strings blanked; `#[path]` strings +// are read raw, since blanking would erase them. +export function rustIncludes(target, source, helper) { + const targetDir = path.dirname(target); + const [first] = rustFile(helper).parts; + if (path.basename(target) === "main.rs" && helper.startsWith(`${targetDir}/`)) + return true; + if (new RegExp(`\\bmod\\s+${first}\\s*;`).test(blankRust(source))) + return true; + for (const [, named] of source.matchAll(/#\[\s*path\s*=\s*"([^"]+)"\s*\]/g)) { + const resolved = path.normalize(path.join(targetDir, named)); + if (resolved === helper) return true; + if ( + path.basename(resolved) === "mod.rs" && + helper.startsWith(`${path.dirname(resolved)}/`) + ) + return true; + } + return false; +} + +// The integration-test targets of a helper's package that include it, as +// files. A helper no target is found to include (a layout rustIncludes does +// not read) falls back to every target, so a helper change is never left +// unrepeated by the scan. +export function rustImporters(helper) { + const { dir } = rustFile(helper); + const tests = `${dir}/tests`; + const targets = readdirSync(`${root}${tests}`, { withFileTypes: true }) + .flatMap((entry) => + entry.isFile() && entry.name.endsWith(".rs") + ? [`${tests}/${entry.name}`] + : entry.isDirectory() && + existsSync(`${root}${tests}/${entry.name}/main.rs`) + ? [`${tests}/${entry.name}/main.rs`] + : [], + ) + .filter((target) => target !== helper) + .sort(); + const importers = targets.filter((target) => + rustIncludes(target, readFileSync(`${root}${target}`, "utf8"), helper), + ); + return { + importers: importers.length ? importers : targets, + scanned: importers.length > 0, + }; +} + // Smoke journeys per spec file as Playwright lists them, each line once (a // looped declaration is listed once per iteration). Integration-only specs // list nothing for the smoke project, which Playwright reports as its "No @@ -502,11 +611,16 @@ function select(base = "HEAD^1", head = "HEAD") { added.has(file) ? "" : git("show", `${base}:${file}`); const diff = (file) => git("diff", "-U0", "--no-color", base, head, "--", file); + // Every runnable test of a Rust file, as entries. + const wholeRust = (file) => + rustTests(readFileSync(`${root}${file}`, "utf8")) + .filter((test) => !test.ignored) + .map((test) => `${file}::${test.name}`); for (const runner of runners) { for (const file of files[runner]) { let entries = [file]; let note = "whole file"; - if (runner === "rust") { + if (rustWorkspaces[runner]) { const selection = selectRust( diff(file), readFileSync(`${root}${file}`, "utf8"), @@ -515,17 +629,37 @@ function select(base = "HEAD^1", head = "HEAD") { previous: previous(file), }, ); - if (!selection) { + if (selection) { + entries = selection.names.map((name) => `${file}::${name}`); + note = + selection.names.length === 0 + ? "excluded (ignored tests only)" + : selection.whole + ? `whole file (${selection.reason}): ${selection.names.length} test(s)` + : `${selection.names.length} changed test(s)`; + } else if (rustFile(file).kind === "tests") { + // A tests/** file without test functions is a helper module that + // integration binaries compile in (`mod common;`), so a change to + // it can change every test of each binary that includes it: select + // those binaries whole. The ignored and infrastructure exclusions + // still apply downstream, as for any whole-file selection. + const { importers, scanned } = rustImporters(file); + entries = importers.flatMap(wholeRust); + const names = importers.map( + (importer) => rustFile(importer).parts[0], + ); + note = + importers.length === 0 + ? "helper with no integration tests to repeat" + : `helper for${scanned ? "" : " (no importer found; every integration test)"}: ${names.join(", ")}: ${entries.length} test(s)`; + } else { + // A src/** file without test functions is the code under test, not + // a test file. This is deliberate: the regular lanes build and run + // everything that depends on it, and repeating its crate's whole + // suite for any source change would not be a changed-test check. rows.push(`| ${cell(file)} | ${runner} | no test functions |`); continue; } - entries = selection.names.map((name) => `${file}::${name}`); - note = - selection.names.length === 0 - ? "excluded (ignored tests only)" - : selection.whole - ? `whole file (${selection.reason}): ${selection.names.length} test(s)` - : `${selection.names.length} changed test(s)`; } else if (runner === "playwright") { const listed = journeys.get(file) ?? []; const source = readFileSync(`${root}${file}`, "utf8"); @@ -549,6 +683,9 @@ function select(base = "HEAD^1", head = "HEAD") { outputs[runner].push(...entries); rows.push(`| ${cell(file)} | ${runner} | ${cell(note)} |`); } + // A helper and a file that includes it, changed together, select the + // same tests twice. + outputs[runner] = [...new Set(outputs[runner])]; const exceeded = Buffer.byteLength(outputs[runner].join(" ")) > MAX_OUTPUT_BYTES ? `selection exceeds ${MAX_OUTPUT_BYTES} bytes` @@ -569,7 +706,9 @@ function select(base = "HEAD^1", head = "HEAD") { "", ...(rows.length ? ["| File | Runner | Selection |", "| --- | --- | --- |", ...rows] - : ["No Rust, node:test, Playwright or Flutter test files changed."]), + : [ + "No Rust, desktop Tauri, node:test, Playwright or Flutter test files changed.", + ]), "", "", ].join("\n"), @@ -678,53 +817,65 @@ export function chooseRust(tests, candidates) { // 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. +// to list in those targets. Every entry must belong to one workspace, whose +// runner is returned with the groups. export function rustGroups(entries) { const groups = new Map(); + const workspaces = new Set(); for (const entry of entries) { - const [, file, path] = - /^(crates\/[^:]+\.rs)::((?:\w+::)*\w+)$/.exec(entry) ?? []; - if (!file) + const [, file, test] = /^([^:]+\.rs)::((?:\w+::)*\w+)$/.exec(entry) ?? []; + const place = file && rustFile(file); + if (!place) throw new Error(`rust entry must be <file>::<module::test>: ${entry}`); + workspaces.add(place.runner); const crate = cratePackage(file); 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.set(target.join(" "), target); group.tests.push({ - path, - exact: [...rustModulePath(file), path].join("::"), + path: test, + exact: [...rustModulePath(file), test].join("::"), }); groups.set(crate, group); } + if (workspaces.size > 1) + throw new Error( + `rust entries span workspaces: ${[...workspaces].join(", ")}`, + ); for (const [crate, group] of groups) group.args = [ "-p", crate, ...(group.every ? [] : [...group.targets.values()].flat()), ]; - return groups; + return { groups, runner: [...workspaces][0] }; } // 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. +// the `flake` profile (the workspace's .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. The Tauri workspace is addressed by its manifest; the root one is +// cargo's default. function repeatRust(entries, iterations) { - const groups = rustGroups(entries); + const { groups, runner } = rustGroups(entries); + const manifest = rustWorkspaces[runner]; + const cargoArgs = runner === "rust" ? [] : ["--manifest-path", manifest]; const runs = []; for (const group of groups.values()) { - const suffixes = [...new Set(group.tests.map(({ path }) => path))] - .map((path) => `test(/(^|::)${path}$/)`) - .join(" | "); const listed = run("cargo", [ "nextest", "list", + ...cargoArgs, ...group.args, "-E", - `(${suffixes}) & !(${infraTests})`, + rustFilterset( + runner, + group.tests.map(({ path }) => path), + ), "--message-format", "json", ]); @@ -756,8 +907,12 @@ function repeatRust(entries, iterations) { } // nextest keeps its store under the workspace root's target/nextest, not // CARGO_TARGET_DIR; the `flake` profile writes its JUnit report there. - const junit = `${root}target/nextest/flake/junit.xml`; - const dir = resultsDir("rust"); + const junit = path.join( + root, + path.dirname(manifest), + "target/nextest/flake/junit.xml", + ); + const dir = resultsDir(runner); for (let i = 1; i <= iterations; i += 1) { const statuses = new Map(); let ok = true; @@ -769,6 +924,7 @@ function repeatRust(entries, iterations) { [ "nextest", "run", + ...cargoArgs, "--profile", "flake", ...group.args, @@ -790,7 +946,10 @@ function repeatRust(entries, iterations) { runs.push({ ok, statuses }); progress(i, iterations, ok); } - report("Flake check (Rust)", runs); + report( + runner === "rust" ? "Flake check (Rust)" : "Flake check (Desktop Tauri)", + runs, + ); } // node:test has no repeat flag either. Each file runs alone, so a failure is @@ -931,9 +1090,12 @@ function main() { if (command === "select" && args.length <= 2) return select(...args); if (command === "playwright-summary" && args.length === 2) return playwrightSummary(...args); - const repeat = { rust: repeatRust, node: repeatNode, dart: repeatDart }[ - command - ]; + const repeat = { + rust: repeatRust, + tauri: repeatRust, + node: repeatNode, + dart: repeatDart, + }[command]; if (repeat) { const iterations = Number( args @@ -945,7 +1107,7 @@ function main() { return repeat(entries, iterations); } throw new Error( - "usage: flake-check select [base] [head] | rust|node|dart [--iterations=N] <entry…> | playwright-summary <report.json> <title>", + "usage: flake-check select [base] [head] | rust|tauri|node|dart [--iterations=N] <entry…> | playwright-summary <report.json> <title>", ); } diff --git a/scripts/flake-check.test.mjs b/scripts/flake-check.test.mjs index fa868c993a7..439b76d0667 100644 --- a/scripts/flake-check.test.mjs +++ b/scripts/flake-check.test.mjs @@ -1,6 +1,10 @@ import assert from "node:assert/strict"; -import { readFileSync } from "node:fs"; +import { execFileSync, spawnSync } from "node:child_process"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; import test from "node:test"; +import { fileURLToPath } from "node:url"; import { chooseRust, classify, @@ -15,10 +19,15 @@ import { parseJunit, playwrightRepetitions, playwrightShards, + runners, + rustFilterset, rustGroups, + rustImporters, + rustIncludes, rustModulePath, rustTarget, rustTests, + rustWorkspaces, selectRust, selectTests, testBodies, @@ -26,6 +35,47 @@ import { } from "./flake-check.mjs"; const read = (path) => readFileSync(new URL(path, import.meta.url), "utf8"); +const root = fileURLToPath(new URL("../", import.meta.url)); +const git = (args, env = {}) => + execFileSync("git", ["-c", "core.hooksPath=/dev/null", ...args], { + cwd: root, + encoding: "utf8", + env: { ...process.env, ...env }, + }); +// `select` over a synthetic head tree: HEAD's tree with a comment appended to +// each given file, so only those files differ and no test body changes. The +// outputs are keyed per runner as the workflow reads them. +function selectAppending(...files) { + const dir = mkdtempSync(join(tmpdir(), "flake-select-")); + try { + const env = { GIT_INDEX_FILE: join(dir, "index") }; + git(["read-tree", "HEAD"], env); + for (const file of files) { + const blob = execFileSync("git", ["hash-object", "-w", "--stdin"], { + cwd: root, + encoding: "utf8", + input: `${git(["show", `HEAD:${file}`])}// flake-check probe\n`, + }).trim(); + git(["update-index", "--cacheinfo", `100644,${blob},${file}`], env); + } + const tree = git(["write-tree"], env).trim(); + const result = spawnSync( + process.execPath, + [join(root, "scripts/flake-check.mjs"), "select", "HEAD", tree], + { cwd: root, encoding: "utf8" }, + ); + assert.equal(result.status, 0, result.stderr); + const outputs = Object.fromEntries( + [...result.stdout.matchAll(/^(\w+)=(.*)$/gm)].map(([, key, value]) => [ + key, + value.split(" ").filter(Boolean), + ]), + ); + return { summary: result.stdout, outputs }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} // A hunk writing new-side lines start..end; by default it removed nothing. const hunk = (start, end, removed = { start: start + 1, end: start }) => ({ start, @@ -40,6 +90,10 @@ test("classification follows each runner's test file layout", () => { for (const [file, runner] of [ ["crates/buzz-core/src/filter.rs", "rust"], ["crates/buzz-test-client/tests/e2e_relay.rs", "rust"], + ["desktop/src-tauri/src/lib.rs", "tauri"], + ["desktop/src-tauri/tests/rodio_mixer_diagnostic.rs", "tauri"], + ["desktop/src-tauri/crates/buzz-terminal/src/lib.rs", "tauri"], + ["desktop/src-tauri/crates/buzz-terminal/tests/latency.rs", "tauri"], ["desktop/src/features/channels/lib/channelDescription.test.mjs", "node"], ["desktop/src/shared/ui/thing.jsdom-test.mjs", "node"], ["desktop/scripts/demo-build-config.test.mjs", "node"], @@ -50,7 +104,9 @@ test("classification follows each runner's test file layout", () => { for (const file of [ "crates/buzz-core/Cargo.toml", "crates/buzz-feature-flags/tests/fixtures/relay-feature-selection/src/main.rs", - "desktop/src-tauri/src/lib.rs", + "desktop/src-tauri/build.rs", + "desktop/src-tauri/tests/fixtures/x.rs", + "desktop/src-tauri/crates/buzz-terminal/Cargo.toml", "desktop/src/features/channels/lib/channelDescription.ts", "desktop/tests/e2e/helpers/animations.ts", "desktop/tests/e2e/typing-latency.perf.ts", @@ -64,6 +120,9 @@ test("classification follows each runner's test file layout", () => { "crates/buzz-core/src/$(id).rs", ]) assert.equal(classify(file), undefined, file); + // Each Rust runner names the cargo workspace its lane repeats. + assert.deepEqual(Object.keys(rustWorkspaces), ["rust", "tauri"]); + assert.ok(runners.includes("tauri")); }); test("-U0 hunks map to new-side ranges, with deletions attached to the preceding line and their old-side lines kept", () => { @@ -230,6 +289,12 @@ test("same-named Rust tests in different modules are distinct, and a hunk select ["crates/buzz-test-client/tests/e2e_relay.rs", []], ["crates/x/tests/suite/main.rs", []], ["crates/x/tests/suite/helpers.rs", ["helpers"]], + ["desktop/src-tauri/src/app_state.rs", ["app_state"]], + ["desktop/src-tauri/tests/rodio_mixer_diagnostic.rs", []], + [ + "desktop/src-tauri/crates/buzz-terminal/src/grid/cell.rs", + ["grid", "cell"], + ], ]) assert.deepEqual(rustModulePath(file), modules, file); // Listed candidates: the predicted name wins; without it every suffix @@ -268,12 +333,21 @@ test("Rust files build only the targets they can be part of, merged per package ["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", []], + ["desktop/src-tauri/src/lib.rs", [["--lib"], ["--bins"]]], + [ + "desktop/src-tauri/tests/rodio_mixer_diagnostic.rs", + [["--test", "rodio_mixer_diagnostic"]], + ], + [ + "desktop/src-tauri/crates/buzz-terminal/tests/latency.rs", + [["--test", "latency"]], + ], ]) 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); + [...rustGroups(entries).groups.values()].map((group) => group.args); assert.deepEqual( args( "crates/buzz-audio-client/tests/codec.rs::roundtrip", @@ -301,6 +375,167 @@ test("Rust files build only the targets they can be part of, merged per package ["-p", "buzz-admin", "--bins"], ], ); + // The Tauri workspace's packages are addressed by its manifest, and never + // mixed into a root-workspace invocation. + const tauri = rustGroups([ + "desktop/src-tauri/tests/rodio_mixer_diagnostic.rs::single_queue_serializes_sources", + "desktop/src-tauri/crates/buzz-terminal/tests/latency.rs::g3_renderer_acquire_stays_within_frame_budget", + ]); + assert.equal(tauri.runner, "tauri"); + assert.equal(rustWorkspaces[tauri.runner], "desktop/src-tauri/Cargo.toml"); + assert.deepEqual( + [...tauri.groups.values()].map((group) => group.args), + [ + ["-p", "buzz-desktop", "--test", "rodio_mixer_diagnostic"], + ["-p", "buzz-terminal", "--test", "latency"], + ], + ); + assert.equal( + rustGroups(["crates/buzz-core/src/filter.rs::tests::a"]).runner, + "rust", + ); + assert.throws( + () => + rustGroups([ + "crates/buzz-core/src/filter.rs::tests::a", + "desktop/src-tauri/src/lib.rs::tests::b", + ]), + /span workspaces/, + ); + assert.throws( + () => rustGroups(["desktop/src/x.rs::tests::a"]), + /rust entry must be/, + ); + // The root workspace's listing excludes its Postgres-backed tests by name + // and binary; nextest rejects `binary(/^postgres_/)` in the Tauri + // workspace, where no such binary exists, so that listing carries no + // exclusion. + assert.equal( + rustFilterset("rust", ["tests::a", "b", "tests::a"]), + "(test(/(^|::)tests::a$/) | test(/(^|::)b$/)) & !(test(/postgres_tests::/) | binary(/^postgres_/) | test(/(^|::)external_infra[^:]*::/))", + ); + assert.equal(rustFilterset("tauri", ["a"]), "(test(/(^|::)a$/))"); + assert.doesNotMatch(rustFilterset("tauri", ["a"]), /binary\(/); +}); + +test("a helper under tests/ selects the integration binaries that include it", () => { + const helper = "crates/x/tests/common/mod.rs"; + const includes = (target, ...lines) => + rustIncludes(target, `${lines.join("\n")}\n`, helper); + // `mod common;` in a crate root, in any form; not in a comment or string. + assert.ok( + includes("crates/x/tests/a.rs", "mod common;", "#[test] fn t() {}"), + ); + assert.ok(includes("crates/x/tests/a.rs", "pub(crate) mod common ;")); + assert.ok(!includes("crates/x/tests/a.rs", "// uses mod common; elsewhere")); + assert.ok(!includes("crates/x/tests/a.rs", 'const S: &str = "mod common;";')); + assert.ok(!includes("crates/x/tests/a.rs", "mod commonplace;", "mod other;")); + // `#[path]` reaching the file itself or its directory's mod.rs. + assert.ok( + includes("crates/x/tests/a.rs", '#[path = "common/mod.rs"]', "mod shared;"), + ); + assert.ok( + includes( + "crates/x/tests/a.rs", + '#[path="../tests/common/mod.rs"] mod shared;', + ), + ); + assert.ok( + !includes("crates/x/tests/a.rs", '#[path = "other/mod.rs"]', "mod shared;"), + ); + assert.ok( + rustIncludes( + "crates/x/tests/a.rs", + '#[path = "common/mod.rs"] mod shared;\n', + "crates/x/tests/common/util.rs", + ), + ); + // A tests/<dir>/main.rs target owns every file under its directory. + assert.ok( + rustIncludes( + "crates/x/tests/suite/main.rs", + "", + "crates/x/tests/suite/util.rs", + ), + ); + assert.ok(!rustIncludes("crates/x/tests/suite/main.rs", "", helper)); + assert.ok( + rustIncludes( + "crates/x/tests/suite/main.rs", + '#[path = "../common/mod.rs"]\nmod common;\n', + helper, + ), + ); + // The real importers of buzz-agent's shared harness: the files declaring + // `mod common;`, not the suites that do not. + assert.deepEqual(rustImporters("crates/buzz-agent/tests/common/mod.rs"), { + importers: [ + "crates/buzz-agent/tests/anthropic_continuation.rs", + "crates/buzz-agent/tests/fake_llm.rs", + "crates/buzz-agent/tests/permission_boundary.rs", + "crates/buzz-agent/tests/regressions.rs", + ], + scanned: true, + }); + for (const importer of rustImporters("crates/buzz-agent/tests/common/mod.rs") + .importers) + assert.match(read(`../${importer}`), /^mod common;$/m, importer); + assert.doesNotMatch( + read("../crates/buzz-agent/tests/golden_transcripts.rs"), + /^mod common;$/m, + ); +}); + +test("select, through its entry point, repeats a changed Tauri test and the binaries including a changed helper", () => { + // Reviewer probes on #8613: before, both produced an empty selection and + // every repeat lane skipped. + const tauri = selectAppending( + "desktop/src-tauri/tests/rodio_mixer_diagnostic.rs", + ); + assert.match( + tauri.summary, + /\| desktop\/src-tauri\/tests\/rodio_mixer_diagnostic\.rs \| tauri \| whole file \(shared code changed\): 3 test\(s\) \|/, + ); + assert.deepEqual(tauri.outputs.rust, []); + assert.deepEqual( + tauri.outputs.tauri, + rustTests(read("../desktop/src-tauri/tests/rodio_mixer_diagnostic.rs")).map( + (test) => + `desktop/src-tauri/tests/rodio_mixer_diagnostic.rs::${test.name}`, + ), + ); + assert.equal(tauri.outputs.tauri.length, 3); + + const helper = selectAppending("crates/buzz-agent/tests/common/mod.rs"); + assert.match( + helper.summary, + /\| crates\/buzz-agent\/tests\/common\/mod\.rs \| rust \| helper for: anthropic_continuation, fake_llm, permission_boundary, regressions: \d+ test\(s\) \|/, + ); + const binaries = new Set( + helper.outputs.rust.map((entry) => entry.split("::")[0]), + ); + assert.deepEqual([...binaries].sort(), [ + "crates/buzz-agent/tests/anthropic_continuation.rs", + "crates/buzz-agent/tests/fake_llm.rs", + "crates/buzz-agent/tests/permission_boundary.rs", + "crates/buzz-agent/tests/regressions.rs", + ]); + assert.ok(helper.outputs.rust.length > 0); + // Changed together, the helper and an importer select each test once. + const both = selectAppending( + "crates/buzz-agent/tests/common/mod.rs", + "crates/buzz-agent/tests/fake_llm.rs", + ); + assert.deepEqual(both.outputs.rust, [...new Set(both.outputs.rust)]); + assert.deepEqual(both.outputs.rust.length, helper.outputs.rust.length); + // A src/** file without test functions still selects nothing: the regular + // lanes run what depends on it. + const source = selectAppending("crates/buzz-core/src/lib.rs"); + assert.match( + source.summary, + /\| crates\/buzz-core\/src\/lib\.rs \| rust \| no test functions \|/, + ); + assert.deepEqual(source.outputs.rust, []); }); test("Rust selection: a hunk inside a test selects it, elsewhere selects every runnable test; ignored tests never run", () => { @@ -614,16 +849,39 @@ test("repeat invocations mirror the lanes they stand in for", () => { `scripts/mobile-test-shard.sh \${{ matrix.shard }} 3 ${dartDefine}`, ), ); - assert.match( - read("../.config/nextest.toml"), - /^\[profile\.flake\]\n(?:#.*\n)*retries = 0\nfail-fast = false\n\n\[profile\.flake\.junit\]\npath = "junit\.xml"\n/m, - ); + // nextest reads its config from the workspace root, so each Rust workspace + // carries the flake profile. + for (const config of [ + "../.config/nextest.toml", + "../desktop/src-tauri/.config/nextest.toml", + ]) + assert.match( + read(config), + /^\[profile\.flake\]\n(?:#.*\n)*retries = 0\nfail-fast = false\n\n\[profile\.flake\.junit\]\npath = "junit\.xml"\n/m, + config, + ); const flake = read("../.github/workflows/_ci-flake.yml"); - for (const runner of ["rust", "node", "dart"]) + for (const runner of ["rust", "tauri", "node", "dart"]) assert.ok( flake.includes(`flake-check.mjs ${runner} --iterations=10 $SELECTED`), runner, ); + // The Tauri lane builds what the Desktop Tauri core lane does: the same + // workspace cache, system packages and sidecar stubs. + const desktop = read("../.github/workflows/_ci-desktop.yml"); + const packages = (text) => + /apt-get install -y --no-install-recommends \\\n((?: {12}\S+(?: \\)?\n)+)/.exec( + text, + )?.[1]; + assert.ok(packages(desktop), "Desktop Tauri apt packages"); + assert.equal(packages(flake), packages(desktop)); + assert.match( + flake, + /flake-tauri:\n(?: {4}.*\n)*? {6}- uses: Swatinem\/rust-cache@\w+\n {8}with:\n {10}workspaces: desktop\/src-tauri\n {10}shared-key: desktop-core\n {10}save-if: false\n/, + ); + assert.match(desktop, /- lane: core\n {12}cache: desktop-core\n/); + assert.ok(flake.includes("run: just _ensure-sidecar-stubs")); + assert.ok(read("../Justfile").includes("\n_ensure-sidecar-stubs:\n")); assert.ok(flake.includes("--repeat-each=10 --retries=0")); // The lanes are advisory: ci.yml calls the workflow on pull requests only, // and no required gate depends on it. From eb18c44ad4e11d49a0b309dab97e5a6d06a9d8ac Mon Sep 17 00:00:00 2001 From: Matt Toohey <contact@matttoohey.com> Date: Tue, 27 Oct 2026 14:54:05 +1100 Subject: [PATCH 5/7] fix(ci): keep flake-check tests hermetic to the Actions env and select #[path] wrapper targets Address Jude's integrated review of f02a4235a on #8613. The required Desktop JS gate was red: scripts/flake-check.test.mjs spawns the real selector, which writes its summary to $GITHUB_STEP_SUMMARY and its outputs to $GITHUB_OUTPUT when those are set, as they are under Actions, so the regression test read an empty stdout there. selectAppending now strips both from the child's environment when it asserts on stdout, and its `github` mode sets them to temp files and reads those instead; the Tauri selection is asserted both ways, equal, with nothing on stdout in the routed mode. The suite passes with and without the two variables set. A tests/<stem>.rs root with no test function of its own was treated as a helper: buzz-acp's pool_lifecycle_state compiles src/pool_lifecycle.rs in by `#[path]`, and a change to it selected git_bootstrap, run_task and stdio_contract and never pool_lifecycle_state. rustTargetRoot now tells a binary's root (tests/<stem>.rs, tests/<dir>/main.rs) from a module of one; rustModules resolves the file modules a file declares by cargo's conventions (`#[path]` relative to the declaring file, else x.rs or x/mod.rs under the module's directory, with comments and strings blanked); rustIncludedTests collects their tests transitively under their module path, the names nextest lists for the binary. selectRust takes these as `included`: a whole selection of a root carries them, a hunk inside one of the root's own tests does not, and a root with none of its own selects them as shared code changed. A root with no tests at all gets a row and selects nothing; only a non-root tests/** file without tests takes the helper branch, and the helper importers' whole-binary expansion uses the same test set. Regression tests go through the real select entry point: a comment appended to pool_lifecycle_state.rs selects exactly the nine pool_lifecycle::tests::* entries under that file (and fails if the included tests are dropped), and rustGroups builds `-p buzz-acp --test pool_lifecycle_state` with those exact names. Verified live: `cargo nextest list` with the produced filterset matches all nine in buzz-acp::pool_lifecycle_state, and `flake-check.mjs rust --iterations=2` over the selection ran green end to end. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com> --- CONTRIBUTING.md | 8 +- scripts/flake-check.mjs | 128 +++++++++++++++++++++++------ scripts/flake-check.test.mjs | 154 ++++++++++++++++++++++++++++++++--- 3 files changed, 249 insertions(+), 41 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 95260d2ed56..b0c73556f47 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -309,9 +309,11 @@ select` maps the diff against the base branch to the Rust tests (in the root and desktop Tauri workspaces), desktop `node --test` files, Playwright smoke journeys and Flutter test files it changed: a hunk inside a test body selects that test; a change anywhere else in a test file (imports, helpers, hooks, -the code under test beside Rust tests) or a new file selects the whole file, -and a change to a helper module under a crate's `tests/` selects every -integration test binary that includes it. The `Flake check (…)` lanes then +the code under test beside Rust tests) or a new file selects the whole file. +An integration test binary's tests include those of the file modules its +root declares (`mod x;`, `#[path = "…"] mod x;`), and a change to a helper +module under a crate's `tests/` selects every integration test binary that +includes it. The `Flake check (…)` lanes then run that selection ten times with retries off; a repetition that fails is usually a race in the test or the code under it, worth fixing before it lands, though a test that needs infrastructure or the regular lane's diff --git a/scripts/flake-check.mjs b/scripts/flake-check.mjs index 224296c4c05..c9ef8a52cbb 100644 --- a/scripts/flake-check.mjs +++ b/scripts/flake-check.mjs @@ -402,23 +402,28 @@ export function selectTests( // Rust tests a diff selects: hunks inside a test body select that test, any // other hunk selects every test in the file (the module's code under test -// lives beside them), and `#[ignore]`d tests own their lines but never run. -// A file without test functions selects nothing of its own; `select` decides -// what that means by where the file lives. +// lives beside them) and, for an integration-test root, every test of the +// file modules it declares (`included`: they are compiled into the same +// binary, so a root with no test of its own still selects them). `#[ignore]`d +// tests own their lines but never run. A file with no tests at all selects +// nothing of its own; `select` decides what that means by where it lives. export function selectRust( diff, source, - { added = false, previous = "" } = {}, + { added = false, previous = "", included = [] } = {}, ) { const tests = rustTests(source); - if (tests.length === 0) return undefined; - const selection = selectTests( - hunkRanges(diff), - tests.map((test) => ({ ...test, line: test.start })), - { added, previous: added ? [] : rustTests(previous) }, - ); + if (tests.length === 0 && included.length === 0) return undefined; + const selection = + tests.length === 0 + ? { whole: true, reason: added ? "new file" : "shared code changed" } + : selectTests( + hunkRanges(diff), + tests.map((test) => ({ ...test, line: test.start })), + { added, previous: added ? [] : rustTests(previous) }, + ); const chosen = selection.whole - ? tests + ? [...tests, ...included] : tests.filter((test) => selection.lines.includes(test.start)); return { ...selection, @@ -545,6 +550,65 @@ export function rustImporters(helper) { }; } +// Whether a tests/** file is the root of an integration-test binary +// (tests/<stem>.rs or tests/<dir>/main.rs) rather than a module of one. +export function rustTargetRoot(file) { + const place = rustFile(file); + if (place?.kind !== "tests") return false; + const { parts } = place; + return parts.length === 1 || (parts.length === 2 && parts[1] === "main"); +} + +// The file modules a Rust file declares, as cargo compiles them into its +// binary: `#[path = "…"] mod x;` names a file relative to the declaring +// file's directory; a plain `mod x;` is x.rs or x/mod.rs under the module's +// directory, the file's own for a root (lib.rs, main.rs, mod.rs, a +// tests/<stem>.rs) and <dir>/<stem>/ otherwise. Declarations are read with +// comments and strings blanked; `#[path]` strings raw, since blanking erases +// them. A module no file backs is left out. +export function rustModules(file, source) { + const dir = path.dirname(file); + const stem = path.basename(file, ".rs"); + const owned = + ["lib", "main", "mod"].includes(stem) || rustTargetRoot(file) + ? dir + : path.join(dir, stem); + const pathed = new Map(); + for (const [, named, name] of source.matchAll( + /#\[\s*path\s*=\s*"([^"]+)"\s*\]\s*(?:#\[[^\]]*\]\s*)*(?:pub(?:\([^)]*\))?\s+)?mod\s+(\w+)\s*;/g, + )) + pathed.set(name, path.normalize(path.join(dir, named))); + const modules = []; + for (const [, name] of blankRust(source).matchAll( + /\b(?:pub(?:\([^)]*\))?\s+)?mod\s+(\w+)\s*;/g, + )) { + const found = ( + pathed.has(name) + ? [pathed.get(name)] + : [`${owned}/${name}.rs`, `${owned}/${name}/mod.rs`] + ).find((candidate) => existsSync(`${root}${candidate}`)); + if (found) modules.push({ name, file: found }); + } + return modules; +} + +// The tests a file compiles in from the file modules it declares, +// transitively, named by their module path from it +// (`pool_lifecycle::tests::name` for `#[path = "../src/pool_lifecycle.rs"] +// mod pool_lifecycle;`), which is how nextest lists them for the binary. An +// integration-test root with no test function of its own (a wrapper around a +// source module, as buzz-acp's pool_lifecycle_state) has only these. +export function rustIncludedTests(file, source, seen = new Set([file])) { + return rustModules(file, source).flatMap(({ name, file: module }) => { + if (seen.has(module)) return []; + seen.add(module); + const text = readFileSync(`${root}${module}`, "utf8"); + return [...rustTests(text), ...rustIncludedTests(module, text, seen)].map( + (test) => ({ ...test, name: `${name}::${test.name}` }), + ); + }); +} + // Smoke journeys per spec file as Playwright lists them, each line once (a // looped declaration is listed once per iteration). Integration-only specs // list nothing for the smoke project, which Playwright reports as its "No @@ -611,24 +675,28 @@ function select(base = "HEAD^1", head = "HEAD") { added.has(file) ? "" : git("show", `${base}:${file}`); const diff = (file) => git("diff", "-U0", "--no-color", base, head, "--", file); + // The tests an integration-test root compiles in from its file modules; + // any other file's tests are its own. + const includedRust = (file, source) => + rustTargetRoot(file) ? rustIncludedTests(file, source) : []; // Every runnable test of a Rust file, as entries. - const wholeRust = (file) => - rustTests(readFileSync(`${root}${file}`, "utf8")) + const wholeRust = (file) => { + const source = readFileSync(`${root}${file}`, "utf8"); + return [...rustTests(source), ...includedRust(file, source)] .filter((test) => !test.ignored) .map((test) => `${file}::${test.name}`); + }; for (const runner of runners) { for (const file of files[runner]) { let entries = [file]; let note = "whole file"; if (rustWorkspaces[runner]) { - const selection = selectRust( - diff(file), - readFileSync(`${root}${file}`, "utf8"), - { - added: added.has(file), - previous: previous(file), - }, - ); + const source = readFileSync(`${root}${file}`, "utf8"); + const selection = selectRust(diff(file), source, { + added: added.has(file), + previous: previous(file), + included: includedRust(file, source), + }); if (selection) { entries = selection.names.map((name) => `${file}::${name}`); note = @@ -637,12 +705,20 @@ function select(base = "HEAD^1", head = "HEAD") { : selection.whole ? `whole file (${selection.reason}): ${selection.names.length} test(s)` : `${selection.names.length} changed test(s)`; + } else if (rustTargetRoot(file)) { + // An integration-test root with no test of its own and none in + // the modules it declares: a binary with nothing to repeat. + rows.push( + `| ${cell(file)} | ${runner} | integration test with no tests |`, + ); + continue; } else if (rustFile(file).kind === "tests") { - // A tests/** file without test functions is a helper module that - // integration binaries compile in (`mod common;`), so a change to - // it can change every test of each binary that includes it: select - // those binaries whole. The ignored and infrastructure exclusions - // still apply downstream, as for any whole-file selection. + // A tests/** file that is not a binary's root and has no test + // functions is a helper module that integration binaries compile + // in (`mod common;`), so a change to it can change every test of + // each binary that includes it: select those binaries whole. The + // ignored and infrastructure exclusions still apply downstream, as + // for any whole-file selection. const { importers, scanned } = rustImporters(file); entries = importers.flatMap(wholeRust); const names = importers.map( diff --git a/scripts/flake-check.test.mjs b/scripts/flake-check.test.mjs index 439b76d0667..4f5f226c32e 100644 --- a/scripts/flake-check.test.mjs +++ b/scripts/flake-check.test.mjs @@ -23,9 +23,12 @@ import { rustFilterset, rustGroups, rustImporters, + rustIncludedTests, rustIncludes, rustModulePath, + rustModules, rustTarget, + rustTargetRoot, rustTests, rustWorkspaces, selectRust, @@ -44,8 +47,12 @@ const git = (args, env = {}) => }); // `select` over a synthetic head tree: HEAD's tree with a comment appended to // each given file, so only those files differ and no test body changes. The -// outputs are keyed per runner as the workflow reads them. -function selectAppending(...files) { +// outputs are keyed per runner as the workflow reads them. The selector +// writes its summary and outputs to $GITHUB_STEP_SUMMARY and $GITHUB_OUTPUT +// when they are set (as under Actions, where this suite also runs) and to +// stdout otherwise, so the child gets one contract or the other by `github`, +// never whichever the parent happens to have. +function selectAppending(files, { github = false } = {}) { const dir = mkdtempSync(join(tmpdir(), "flake-select-")); try { const env = { GIT_INDEX_FILE: join(dir, "index") }; @@ -59,19 +66,34 @@ function selectAppending(...files) { git(["update-index", "--cacheinfo", `100644,${blob},${file}`], env); } const tree = git(["write-tree"], env).trim(); + const { GITHUB_STEP_SUMMARY, GITHUB_OUTPUT, ...inherited } = process.env; + const routed = { + GITHUB_STEP_SUMMARY: join(dir, "summary.md"), + GITHUB_OUTPUT: join(dir, "output.txt"), + }; const result = spawnSync( process.execPath, [join(root, "scripts/flake-check.mjs"), "select", "HEAD", tree], - { cwd: root, encoding: "utf8" }, + { + cwd: root, + encoding: "utf8", + env: github ? { ...inherited, ...routed } : inherited, + }, ); assert.equal(result.status, 0, result.stderr); + const summary = github + ? readFileSync(routed.GITHUB_STEP_SUMMARY, "utf8") + : result.stdout; + const output = github + ? readFileSync(routed.GITHUB_OUTPUT, "utf8") + : result.stdout; const outputs = Object.fromEntries( - [...result.stdout.matchAll(/^(\w+)=(.*)$/gm)].map(([, key, value]) => [ + [...output.matchAll(/^(\w+)=(.*)$/gm)].map(([, key, value]) => [ key, value.split(" ").filter(Boolean), ]), ); - return { summary: result.stdout, outputs }; + return { summary, outputs, stdout: result.stdout }; } finally { rmSync(dir, { recursive: true, force: true }); } @@ -484,14 +506,55 @@ test("a helper under tests/ selects the integration binaries that include it", ( read("../crates/buzz-agent/tests/golden_transcripts.rs"), /^mod common;$/m, ); + // A binary's root is tests/<stem>.rs or tests/<dir>/main.rs; anything else + // under tests/ is a module of one. + for (const file of ["crates/x/tests/a.rs", "crates/x/tests/suite/main.rs"]) + assert.ok(rustTargetRoot(file), file); + for (const file of [ + "crates/x/tests/common/mod.rs", + "crates/x/tests/suite/util.rs", + "crates/x/src/lib.rs", + "crates/x/src/main.rs", + ]) + assert.ok(!rustTargetRoot(file), file); + // File modules resolve by cargo's conventions against the real tree: a + // root's `mod common;` is tests/common/mod.rs, a non-root's `mod launch;` + // is <stem>/launch.rs, a `#[path]` names its file, and a declaration in a + // comment or one no file backs is left out. + assert.deepEqual( + rustModules( + "crates/buzz-agent/tests/fake_llm.rs", + "mod common;\n// mod missing;\nmod nowhere;\n", + ), + [{ name: "common", file: "crates/buzz-agent/tests/common/mod.rs" }], + ); + assert.deepEqual(rustModules("crates/buzz-acp/src/acp.rs", "mod launch;\n"), [ + { name: "launch", file: "crates/buzz-acp/src/acp/launch.rs" }, + ]); + const wrapper = "crates/buzz-acp/tests/pool_lifecycle_state.rs"; + assert.deepEqual(rustModules(wrapper, read(`../${wrapper}`)), [ + { name: "pool_lifecycle", file: "crates/buzz-acp/src/pool_lifecycle.rs" }, + ]); + // The wrapper's tests are the included module's, under its module name: + // the names nextest lists for the pool_lifecycle_state binary. + const included = rustTests(read("../crates/buzz-acp/src/pool_lifecycle.rs")); + assert.ok(included.length > 0); + assert.deepEqual( + rustIncludedTests(wrapper, read(`../${wrapper}`)).map((test) => test.name), + included.map((test) => `pool_lifecycle::${test.name}`), + ); + assert.deepEqual( + rustIncludedTests("crates/buzz-agent/tests/fake_llm.rs", "mod common;\n"), + [], + ); }); test("select, through its entry point, repeats a changed Tauri test and the binaries including a changed helper", () => { // Reviewer probes on #8613: before, both produced an empty selection and // every repeat lane skipped. - const tauri = selectAppending( + const tauri = selectAppending([ "desktop/src-tauri/tests/rodio_mixer_diagnostic.rs", - ); + ]); assert.match( tauri.summary, /\| desktop\/src-tauri\/tests\/rodio_mixer_diagnostic\.rs \| tauri \| whole file \(shared code changed\): 3 test\(s\) \|/, @@ -505,8 +568,21 @@ test("select, through its entry point, repeats a changed Tauri test and the bina ), ); assert.equal(tauri.outputs.tauri.length, 3); + // Under Actions, where the Desktop JS lane runs this suite, the selector + // routes the summary and outputs to $GITHUB_STEP_SUMMARY and + // $GITHUB_OUTPUT: the same selection arrives there, and nothing on stdout. + const routed = selectAppending( + ["desktop/src-tauri/tests/rodio_mixer_diagnostic.rs"], + { github: true }, + ); + assert.equal(routed.stdout, ""); + assert.match( + routed.summary, + /\| desktop\/src-tauri\/tests\/rodio_mixer_diagnostic\.rs \| tauri \| whole file \(shared code changed\): 3 test\(s\) \|/, + ); + assert.deepEqual(routed.outputs, tauri.outputs); - const helper = selectAppending("crates/buzz-agent/tests/common/mod.rs"); + const helper = selectAppending(["crates/buzz-agent/tests/common/mod.rs"]); assert.match( helper.summary, /\| crates\/buzz-agent\/tests\/common\/mod\.rs \| rust \| helper for: anthropic_continuation, fake_llm, permission_boundary, regressions: \d+ test\(s\) \|/, @@ -522,15 +598,45 @@ test("select, through its entry point, repeats a changed Tauri test and the bina ]); assert.ok(helper.outputs.rust.length > 0); // Changed together, the helper and an importer select each test once. - const both = selectAppending( + const both = selectAppending([ "crates/buzz-agent/tests/common/mod.rs", "crates/buzz-agent/tests/fake_llm.rs", - ); + ]); assert.deepEqual(both.outputs.rust, [...new Set(both.outputs.rust)]); assert.deepEqual(both.outputs.rust.length, helper.outputs.rust.length); + // A tests/<stem>.rs root with no test of its own is a binary, not a + // helper: buzz-acp's pool_lifecycle_state compiles src/pool_lifecycle.rs + // in by `#[path]`, and a change to it repeats that binary's tests, not its + // siblings' (before, the selection was git_bootstrap, run_task and + // stdio_contract, and pool_lifecycle_state was not repeated at all). + const wrapper = "crates/buzz-acp/tests/pool_lifecycle_state.rs"; + assert.deepEqual(rustTests(read(`../${wrapper}`)), []); + const included = rustTests(read("../crates/buzz-acp/src/pool_lifecycle.rs")); + const wrapped = selectAppending([wrapper]); + assert.match( + wrapped.summary, + new RegExp( + `\\| crates/buzz-acp/tests/pool_lifecycle_state\\.rs \\| rust \\| whole file \\(shared code changed\\): ${included.length} test\\(s\\) \\|`, + ), + ); + assert.deepEqual( + wrapped.outputs.rust, + included.map((test) => `${wrapper}::pool_lifecycle::${test.name}`), + ); + const { groups } = rustGroups(wrapped.outputs.rust); + assert.deepEqual(groups.get("buzz-acp").args, [ + "-p", + "buzz-acp", + "--test", + "pool_lifecycle_state", + ]); + assert.deepEqual( + groups.get("buzz-acp").tests.map(({ exact }) => exact), + included.map((test) => `pool_lifecycle::${test.name}`), + ); // A src/** file without test functions still selects nothing: the regular // lanes run what depends on it. - const source = selectAppending("crates/buzz-core/src/lib.rs"); + const source = selectAppending(["crates/buzz-core/src/lib.rs"]); assert.match( source.summary, /\| crates\/buzz-core\/src\/lib\.rs \| rust \| no test functions \|/, @@ -564,7 +670,31 @@ test("Rust selection: a hunk inside a test selects it, elsewhere selects every r select(diff([21, 1, 21, 1]), { added: true, previous: "" }), whole("new file"), ); - // A file without test functions is not a test file. + // Tests of the file modules an integration-test root declares ride along + // with a whole selection, not with a hunk inside one of the root's own + // tests; a root with no test of its own selects only them, and one with no + // tests at all is not a test file. + const included = [ + { name: "m::a", ignored: false }, + { name: "m::b", ignored: true }, + ]; + assert.deepEqual(select(diff([4, 0, 5, 1]), { included }), { + ...whole("shared code changed"), + names: [...whole().names, "m::a"], + }); + assert.deepEqual(select(diff([21, 1, 21, 1]), { included }), { + lines: [19], + names: ["tests::plain"], + }); + assert.deepEqual(selectRust(diff([1, 1, 1, 1]), "mod m;\n", { included }), { + whole: true, + reason: "shared code changed", + names: ["m::a"], + }); + assert.deepEqual( + selectRust(diff([0, 0, 1, 1]), "mod m;\n", { added: true, included }), + { whole: true, reason: "new file", names: ["m::a"] }, + ); assert.equal(selectRust(diff([1, 1, 1, 1]), "fn main() {}\n"), undefined); assert.equal(cratePackage("crates/buzz-core/src/filter.rs"), "buzz-core"); }); From fdee846d20bdb9bd2bcb02580c70a3cdf2e52b3b Mon Sep 17 00:00:00 2001 From: Matt Toohey <contact@matttoohey.com> Date: Wed, 28 Oct 2026 22:24:57 +1100 Subject: [PATCH 6/7] fix(ci): repeat feature-gated Rust tests in their CI feature graph and fail on unlisted selections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Rust and Desktop Tauri repeats listed only each package's default feature graph and treated an entry nextest did not list as excluded, so a selected `#[cfg(feature = …)]` test (buzz-feature-flags' LaunchDarkly, buzz-push-gateway's App Attest development feature, the desktop crate's mesh-llm) passed as "nothing to repeat" or was silently dropped from a mixed selection. Each package is now listed in the feature graphs the regular lanes test it in, in order, and each entry repeats in the first graph that lists it, so the mesh-llm graph is built only for tests the default graph lacks. Ignored and infrastructure-backed tests are classified from the whole listing (nextest reports an ignored test as a filter mismatch whatever -E says), and an entry no graph lists fails the lane by name. A fake-cargo regression drives the script's entry point through each case, and the graph table is pinned to the Justfile recipes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com> --- .github/workflows/_ci-flake.yml | 5 +- CONTRIBUTING.md | 5 + scripts/flake-check.mjs | 193 +++++++++++-------- scripts/flake-check.test.mjs | 322 ++++++++++++++++++++++++++++++-- 4 files changed, 435 insertions(+), 90 deletions(-) diff --git a/.github/workflows/_ci-flake.yml b/.github/workflows/_ci-flake.yml index 2ab3542e4bc..94d98c82c90 100644 --- a/.github/workflows/_ci-flake.yml +++ b/.github/workflows/_ci-flake.yml @@ -157,7 +157,10 @@ jobs: - name: Create sidecar stubs run: just _ensure-sidecar-stubs # Same script as flake-rust, against the Tauri workspace's manifest and - # its own .config/nextest.toml `flake` profile. + # its own .config/nextest.toml `flake` profile. A selected test the + # default graph does not list is looked for under mesh-llm, whose + # dependency graph the restored desktop-core cache does not hold, so + # only such a selection builds it cold. - name: Repeat changed desktop Tauri tests env: CMAKE_POLICY_VERSION_MINIMUM: "3.5" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b0c73556f47..9c155a5d170 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -322,6 +322,11 @@ are deliberately not repeated: `#[ignore]`d and Postgres-backed Rust tests, Playwright specs outside the `smoke` project, `*.perf.ts` measurements, and a selection too large for its lane (over 64 KiB of entries, or more Playwright journeys than ten repetitions can finish within the lane's timeout). +A Rust test repeats in the first feature graph the regular lanes test its +package in that lists it (LaunchDarkly, the App Attest development feature, +the desktop crate's `mesh-llm`); a selected test that no such graph lists on +the Linux runner, such as a macOS-only one, fails its lane by name rather +than passing unrepeated. The lanes skip when no test changed and are not required to merge; their step summaries show the selection and the test × iteration table. diff --git a/scripts/flake-check.mjs b/scripts/flake-check.mjs index c9ef8a52cbb..aa26b0c4d31 100644 --- a/scripts/flake-check.mjs +++ b/scripts/flake-check.mjs @@ -94,26 +94,26 @@ export const nodeTestArgs = (file) => // Mirrors the mobile test shards in .github/workflows/_ci-clients.yml. export const dartDefine = "--dart-define=BUZZ_PUSH_GATEWAY_URL=https://push.example"; -// Mirrors the postgres-ci default-filter in .config/nextest.toml: those tests -// are #[ignore]d and need Postgres, Redis, S3 or MinIO, so a selection that -// reached one would fail every repetition without being flaky. The convention -// is the root workspace's; nextest rejects a `binary()` predicate matching no -// test binary in the workspace, and the Tauri workspace has none, so it gets -// no exclusion. -const infraTests = { - rust: "test(/postgres_tests::/) | binary(/^postgres_/) | test(/(^|::)external_infra[^:]*::/)", -}; +// Mirrors the postgres-ci default-filter in .config/nextest.toml +// (`test(/postgres_tests::/)`, `binary(/^postgres_/)` and +// `test(/(^|::)external_infra[^:]*::/)`): those tests need Postgres, Redis, +// S3 or MinIO, so a selection that reached one would fail every repetition +// without being flaky. Only the root workspace has such tests. +export const infraTest = (binaryName, name) => + /postgres_tests::/.test(name) || + /^postgres_/.test(binaryName) || + /(^|::)external_infra[^:]*::/.test(name); -// The nextest filterset listing a workspace's candidates for the selected -// in-file test paths, less its infrastructure-backed tests. -export function rustFilterset(runner, paths) { - const suffixes = [...new Set(paths)] - .map((path) => `test(/(^|::)${path}$/)`) - .join(" | "); - return infraTests[runner] - ? `(${suffixes}) & !(${infraTests[runner]})` - : `(${suffixes})`; -} +// The feature graphs the regular lanes test a package in, in order, where +// that is not just its default graph: the Justfile's test-unit runs +// buzz-push-gateway without and with its feature and buzz-feature-flags only +// with LaunchDarkly, and desktop-tauri-test-mesh runs the desktop crate under +// mesh-llm. A test behind `#[cfg(feature = …)]` is listed only in its graph. +export const rustFeatureGraphs = { + "buzz-feature-flags": [["launchdarkly"]], + "buzz-push-gateway": [[], ["personal-dev-app-attest"]], + "buzz-desktop": [[], ["mesh-llm"]], +}; // Mirrors desktop/package.json "test", desktop/playwright.config.ts testDir, // the two cargo workspaces' packages and mobile's test directory. Paths are @@ -875,26 +875,30 @@ export function parseJunit(xml) { // The listed tests an entry stands for: a test the file's module path predicts // exactly when nextest lists one, otherwise every test whose name ends in the // in-file path (a `#[path]` module breaks the prediction; a namesake in another -// module is then repeated too rather than nothing). +// module is then repeated too rather than nothing). `unlisted` keeps the +// entries no candidate stands for. export function chooseRust(tests, candidates) { const chosen = new Map(); - for (const { path, exact } of tests) { + const unlisted = []; + for (const test of tests) { + const { path, exact } = test; const suffix = candidates.filter( ({ name }) => name === path || name.endsWith(`::${path}`), ); const predicted = suffix.filter(({ name }) => name === exact); + if (suffix.length === 0) unlisted.push(test); for (const test of predicted.length ? predicted : suffix) chosen.set(`${test.binary} ${test.name}`, test); } - return [...chosen.values()]; + return { chosen: [...chosen.values()], unlisted }; } // The repeat's cargo invocation per package: `-p <crate>` 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. Every entry must belong to one workspace, whose -// runner is returned with the groups. +// nextest reads as a test-name filter that matches nothing), the feature +// graphs to list them in, and the tests to list in those targets. Every entry +// must belong to one workspace, whose runner is returned with the groups. export function rustGroups(entries) { const groups = new Map(); const workspaces = new Set(); @@ -910,6 +914,7 @@ export function rustGroups(entries) { if (targets.length === 0) group.every = true; for (const target of targets) group.targets.set(target.join(" "), target); group.tests.push({ + entry, path: test, exact: [...rustModulePath(file), test].join("::"), }); @@ -919,66 +924,104 @@ export function rustGroups(entries) { throw new Error( `rust entries span workspaces: ${[...workspaces].join(", ")}`, ); - for (const [crate, group] of groups) + for (const [crate, group] of groups) { group.args = [ "-p", crate, ...(group.every ? [] : [...group.targets.values()].flat()), ]; + group.graphs = rustFeatureGraphs[crate] ?? [[]]; + } return { groups, runner: [...workspaces][0] }; } // nextest has no repeat flag, so run the selection once per iteration under // the `flake` profile (the workspace's .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. The Tauri workspace is addressed by its manifest; the root one is -// cargo's default. +// per run), one cargo invocation per package and feature graph so each builds +// only the targets its files can be in. Each package's targets are listed +// whole in its feature graphs in turn (nextest reports an ignored test as a +// filter mismatch whatever `-E` says, so a filtered listing could not tell an +// ignored entry from an unlisted one), and every entry is narrowed with +// chooseRust to exact (binary, test) pairs in the first graph listing it, so +// a later graph (and its build) is reached only by tests behind its feature. +// An entry standing only for ignored or infrastructure-backed tests is +// excluded; one no graph lists (a test behind a cfg the regular lanes do not +// build here) fails the lane rather than passing unrepeated. The Tauri +// workspace is addressed by its manifest; the root one is cargo's default. function repeatRust(entries, iterations) { const { groups, runner } = rustGroups(entries); + const title = + runner === "rust" ? "Flake check (Rust)" : "Flake check (Desktop Tauri)"; const manifest = rustWorkspaces[runner]; const cargoArgs = runner === "rust" ? [] : ["--manifest-path", manifest]; - const runs = []; - for (const group of groups.values()) { - const listed = run("cargo", [ - "nextest", - "list", - ...cargoArgs, - ...group.args, - "-E", - rustFilterset( - runner, - group.tests.map(({ path }) => path), - ), - "--message-format", - "json", - ]); - if (listed.status !== 0) - throw new Error(`cargo nextest list: ${listed.stderr}`); - const candidates = Object.entries( - JSON.parse(listed.stdout)["rust-suites"], - ).flatMap(([binary, suite]) => - Object.entries(suite.testcases) - .filter( - ([, test]) => - !test.ignored && test["filter-match"].status === "matches", - ) - .map(([name]) => ({ binary, name })), + const invocations = []; + const unlisted = []; + for (const [crate, group] of groups) { + let tests = group.tests; + for (const features of group.graphs) { + if (tests.length === 0) break; + const args = [ + ...group.args, + ...(features.length ? ["--features", features.join(",")] : []), + ]; + const listed = run("cargo", [ + "nextest", + "list", + ...cargoArgs, + ...args, + "--message-format", + "json", + ]); + if (listed.status !== 0) + throw new Error(`cargo nextest list: ${listed.stderr}`); + const candidates = Object.entries( + JSON.parse(listed.stdout)["rust-suites"], + ).flatMap(([binary, suite]) => + Object.entries(suite.testcases).map(([name, test]) => ({ + binary, + name, + runnable: !test.ignored && !infraTest(suite["binary-name"], name), + })), + ); + const choice = chooseRust(tests, candidates); + const chosen = choice.chosen.filter((test) => test.runnable); + if (chosen.length) + invocations.push({ + label: [crate, ...features].join("+"), + args, + chosen, + filterset: chosen + .map( + ({ binary, name }) => `(binary_id(=${binary}) & test(=${name}))`, + ) + .join(" | "), + }); + tests = choice.unlisted; + } + unlisted.push(...tests.map((test) => test.entry)); + } + if (unlisted.length) { + summarize( + [ + `## ${title}: ${unlisted.length} selected test(s) not listed`, + "", + "No feature graph the regular lanes test these packages in lists them on this runner, so they were not repeated:", + "", + ...unlisted.map((entry) => `- \`${entry}\``), + "", + "", + ].join("\n"), ); - group.chosen = chooseRust(group.tests, candidates); - group.filterset = group.chosen - .map(({ binary, name }) => `(binary_id(=${binary}) & test(=${name}))`) - .join(" | "); + process.exitCode = 1; } - const expected = [...groups.values()].flatMap((group) => - group.chosen.map(({ binary, name }) => `${binary} ${name}`), + const expected = invocations.flatMap((invocation) => + invocation.chosen.map(({ binary, name }) => `${binary} ${name}`), ); if (expected.length === 0) { - summarize( - "## Flake check (Rust): nothing to repeat\n\nEvery selected test is ignored or infrastructure-backed.\n\n", - ); + if (!unlisted.length) + summarize( + `## ${title}: nothing to repeat\n\nEvery selected test is ignored or infrastructure-backed.\n\n`, + ); return; } // nextest keeps its store under the workspace root's target/nextest, not @@ -989,11 +1032,11 @@ function repeatRust(entries, iterations) { "target/nextest/flake/junit.xml", ); const dir = resultsDir(runner); + const runs = []; for (let i = 1; i <= iterations; i += 1) { const statuses = new Map(); let ok = true; - for (const [crate, group] of groups) { - if (group.chosen.length === 0) continue; + for (const { label, args, filterset } of invocations) { rmSync(junit, { force: true }); const result = run( "cargo", @@ -1003,9 +1046,9 @@ function repeatRust(entries, iterations) { ...cargoArgs, "--profile", "flake", - ...group.args, + ...args, "-E", - group.filterset, + filterset, ], { stdio: "inherit" }, ); @@ -1013,7 +1056,7 @@ function repeatRust(entries, iterations) { try { for (const [test, status] of parseJunit(readFileSync(junit, "utf8"))) statuses.set(test, status); - copyFileSync(junit, `${dir}/iter-${i}-${crate}.xml`); + copyFileSync(junit, `${dir}/iter-${i}-${label}.xml`); } catch { ok = false; } @@ -1022,10 +1065,8 @@ function repeatRust(entries, iterations) { runs.push({ ok, statuses }); progress(i, iterations, ok); } - report( - runner === "rust" ? "Flake check (Rust)" : "Flake check (Desktop Tauri)", - runs, - ); + report(title, runs); + if (unlisted.length) process.exitCode = 1; } // node:test has no repeat flag either. Each file runs alone, so a failure is diff --git a/scripts/flake-check.test.mjs b/scripts/flake-check.test.mjs index 4f5f226c32e..bbc56cf785d 100644 --- a/scripts/flake-check.test.mjs +++ b/scripts/flake-check.test.mjs @@ -1,6 +1,14 @@ import assert from "node:assert/strict"; import { execFileSync, spawnSync } from "node:child_process"; -import { mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { + copyFileSync, + mkdirSync, + mkdtempSync, + readFileSync, + realpathSync, + rmSync, + writeFileSync, +} from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import test from "node:test"; @@ -12,6 +20,7 @@ import { dartDefine, hunkRanges, ITERATIONS, + infraTest, listPlaywright, MAX_PLAYWRIGHT_JOURNEYS, nodeTestArgs, @@ -20,7 +29,7 @@ import { playwrightRepetitions, playwrightShards, runners, - rustFilterset, + rustFeatureGraphs, rustGroups, rustImporters, rustIncludedTests, @@ -335,17 +344,23 @@ test("same-named Rust tests in different modules are distinct, and a hunk select [{ path: "tests::same", exact: "handlers::auth::tests::same" }], candidates, ), - [candidates[1], candidates[3]], + { chosen: [candidates[1], candidates[3]], unlisted: [] }, ); + // An entry no candidate stands for is kept apart, not dropped. + const gated = { path: "tests::gated", exact: "handlers::auth::tests::gated" }; assert.deepEqual( chooseRust( [ { path: "tests::same", exact: "elsewhere::tests::same" }, { path: "tests::other", exact: "handlers::auth::tests::other" }, + gated, ], candidates, ), - [candidates[0], candidates[1], candidates[3], candidates[2]], + { + chosen: [candidates[0], candidates[1], candidates[3], candidates[2]], + unlisted: [gated], + }, ); }); @@ -428,16 +443,28 @@ test("Rust files build only the targets they can be part of, merged per package () => rustGroups(["desktop/src/x.rs::tests::a"]), /rust entry must be/, ); - // The root workspace's listing excludes its Postgres-backed tests by name - // and binary; nextest rejects `binary(/^postgres_/)` in the Tauri - // workspace, where no such binary exists, so that listing carries no - // exclusion. - assert.equal( - rustFilterset("rust", ["tests::a", "b", "tests::a"]), - "(test(/(^|::)tests::a$/) | test(/(^|::)b$/)) & !(test(/postgres_tests::/) | binary(/^postgres_/) | test(/(^|::)external_infra[^:]*::/))", + // Packages the regular lanes test under a feature list in those graphs, in + // order; the rest in their default graph alone. + assert.deepEqual( + [ + "crates/buzz-feature-flags/src/launchdarkly.rs::tests::a", + "crates/buzz-push-gateway/src/lib.rs::tests::b", + "crates/buzz-core/src/filter.rs::tests::c", + ].map((entry) => [...rustGroups([entry]).groups.values()][0].graphs), + [[["launchdarkly"]], [[], ["personal-dev-app-attest"]], [[]]], ); - assert.equal(rustFilterset("tauri", ["a"]), "(test(/(^|::)a$/))"); - assert.doesNotMatch(rustFilterset("tauri", ["a"]), /binary\(/); + assert.deepEqual( + [ + ...rustGroups(["desktop/src-tauri/src/lib.rs::tests::d"]).groups.values(), + ][0].graphs, + [[], ["mesh-llm"]], + ); + // Postgres-backed tests are excluded by name and binary, as the postgres-ci + // default-filter selects them. + assert.ok(infraTest("buzz_db", "event_store::postgres_tests::insert")); + assert.ok(infraTest("postgres_relay", "admits")); + assert.ok(infraTest("buzz_media", "tests::external_infra_s3::upload")); + assert.ok(!infraTest("buzz_db", "tests::postgres_free")); }); test("a helper under tests/ selects the integration binaries that include it", () => { @@ -990,6 +1017,43 @@ test("repeat invocations mirror the lanes they stand in for", () => { /^\[profile\.flake\]\n(?:#.*\n)*retries = 0\nfail-fast = false\n\n\[profile\.flake\.junit\]\npath = "junit\.xml"\n/m, config, ); + // The infrastructure exclusion mirrors the postgres-ci default-filter. + for (const predicate of [ + "test(/postgres_tests::/)", + "binary(/^postgres_/)", + "test(/(^|::)external_infra[^:]*::/)", + ]) + assert.ok(read("../.config/nextest.toml").includes(predicate), predicate); + // Feature graphs mirror the Justfile recipes the regular lanes run: every + // package test-unit runs under a feature, in each graph it runs there, and + // the desktop crate's default and mesh-llm lanes. + const justfile = read("../Justfile"); + const graphs = {}; + for (const [, crate, features] of justfile.matchAll( + /^ +cargo nextest run -p ([\w-]+)(?: --features ([\w,-]+))?$/gm, + )) { + graphs[crate] ??= []; + graphs[crate].push(features?.split(",") ?? []); + } + assert.ok( + justfile.includes( + "\ndesktop-tauri-test: _ensure-sidecar-stubs\n cd desktop/src-tauri && cargo test --workspace\n", + ), + ); + assert.ok( + justfile.includes( + "\ndesktop-tauri-test-mesh: _ensure-sidecar-stubs\n cargo test --manifest-path {{desktop_tauri_manifest}} --features mesh-llm --lib\n", + ), + ); + graphs["buzz-desktop"] = [[], ["mesh-llm"]]; + assert.deepEqual( + rustFeatureGraphs, + Object.fromEntries( + Object.entries(graphs).filter(([, list]) => + list.some((features) => features.length), + ), + ), + ); const flake = read("../.github/workflows/_ci-flake.yml"); for (const runner of ["rust", "tauri", "node", "dart"]) assert.ok( @@ -1022,3 +1086,235 @@ test("repeat invocations mirror the lanes they stand in for", () => { ); assert.doesNotMatch(ci, /needs: \[[^\]]*flake/); }); + +// The repeat, through its entry point, in a scratch tree holding the script +// and the packages its entries name, with a fake `cargo` on PATH listing +// `tests` ({pkg, binary, binaryName, name, feature?, ignored?}; a feature +// test only under that feature) and passing every test a run selects. +function repeatWithFakeCargo(runner, entries, tests) { + const dir = realpathSync(mkdtempSync(join(tmpdir(), "flake-repeat-"))); + try { + mkdirSync(join(dir, "scripts")); + copyFileSync( + join(root, "scripts/flake-check.mjs"), + join(dir, "scripts/flake-check.mjs"), + ); + for (const [crateDir, name] of [ + ["crates/buzz-feature-flags", "buzz-feature-flags"], + ["desktop/src-tauri", "buzz-desktop"], + ]) { + mkdirSync(join(dir, crateDir, "src"), { recursive: true }); + writeFileSync( + join(dir, crateDir, "Cargo.toml"), + `[package]\nname = "${name}"\n`, + ); + writeFileSync(join(dir, crateDir, "src/lib.rs"), ""); + } + mkdirSync(join(dir, "bin")); + writeFileSync( + join(dir, "fake-cargo.mjs"), + `import { appendFileSync, mkdirSync, writeFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +const args = process.argv.slice(2); +appendFileSync(process.env.FAKE_CARGO_LOG, JSON.stringify(args) + "\\n"); +const value = (flag) => args[args.indexOf(flag) + 1]; +const features = args.includes("--features") ? value("--features").split(",") : []; +const tests = JSON.parse(process.env.FAKE_CARGO_TESTS).filter( + (test) => test.pkg === value("-p") && (!test.feature || features.includes(test.feature)), +); +if (args[1] === "list") { + const suites = {}; + for (const test of tests) { + suites[test.binary] ??= { "binary-name": test.binaryName, testcases: {} }; + suites[test.binary].testcases[test.name] = { + ignored: Boolean(test.ignored), + "filter-match": test.ignored ? { status: "mismatch", reason: "ignored" } : { status: "matches" }, + }; + } + process.stdout.write(JSON.stringify({ "rust-suites": suites })); +} else { + const junit = join( + args.includes("--manifest-path") ? dirname(value("--manifest-path")) : ".", + "target/nextest/flake/junit.xml", + ); + mkdirSync(dirname(junit), { recursive: true }); + const cases = [...value("-E").matchAll(/binary_id\\(=([^)]+)\\) & test\\(=([^)]+)\\)/g)]; + writeFileSync(junit, "<testsuites>" + cases.map(([, binary, name]) => '<testcase name="' + name + '" classname="' + binary + '"/>').join("") + "</testsuites>"); +} +`, + ); + writeFileSync( + join(dir, "bin/cargo"), + `#!/bin/sh\nexec "${process.execPath}" "${join(dir, "fake-cargo.mjs")}" "$@"\n`, + { mode: 0o755 }, + ); + const { GITHUB_STEP_SUMMARY, GITHUB_OUTPUT, ...inherited } = process.env; + const log = join(dir, "cargo.log"); + const summary = join(dir, "summary.md"); + writeFileSync(summary, ""); + const result = spawnSync( + process.execPath, + [ + join(dir, "scripts/flake-check.mjs"), + runner, + "--iterations=2", + ...entries, + ], + { + cwd: dir, + encoding: "utf8", + env: { + ...inherited, + PATH: `${join(dir, "bin")}:${process.env.PATH}`, + GITHUB_STEP_SUMMARY: summary, + FAKE_CARGO_LOG: log, + FAKE_CARGO_TESTS: JSON.stringify(tests), + }, + }, + ); + assert.equal(result.signal, null, result.stderr); + return { + status: result.status, + summary: readFileSync(summary, "utf8"), + calls: readFileSync(log, "utf8") + .trim() + .split("\n") + .map((line) => JSON.parse(line)), + }; + } finally { + rmSync(dir, { recursive: true, force: true }); + } +} + +test("feature-gated Rust tests repeat in the graph the regular lanes test them in, and unlisted ones fail the lane", () => { + // Reviewer probe on #8613: the repeat listed only the default graph, so a + // selected `#[cfg(feature = …)]` test was dropped and the lane passed as + // "nothing to repeat", or reported only its ungated neighbours. + const features = (call) => + call.includes("--features") ? call[call.indexOf("--features") + 1] : ""; + const flags = { + pkg: "buzz-feature-flags", + binary: "buzz-feature-flags", + binaryName: "buzz_feature_flags", + }; + const root = repeatWithFakeCargo( + "rust", + [ + "crates/buzz-feature-flags/src/lib.rs::tests::plain", + "crates/buzz-feature-flags/src/launchdarkly.rs::tests::null_events", + ], + [ + { ...flags, name: "tests::plain" }, + { + ...flags, + name: "launchdarkly::tests::null_events", + feature: "launchdarkly", + }, + ], + ); + assert.equal(root.status, 0, root.summary); + // buzz-feature-flags is only tested with LaunchDarkly, so it is listed and + // run in that graph alone. + assert.deepEqual( + root.calls.map((call) => [call[1], features(call)]), + [ + ["list", "launchdarkly"], + ["run", "launchdarkly"], + ["run", "launchdarkly"], + ], + ); + assert.match( + root.summary, + /## Flake check \(Rust\): 0 of 2 iterations failed/, + ); + assert.match( + root.summary, + /\| buzz-feature-flags launchdarkly::tests::null_events \| ✅ \| ✅ \| 0\/2 \|/, + ); + assert.match( + root.summary, + /\| buzz-feature-flags tests::plain \| ✅ \| ✅ \| 0\/2 \|/, + ); + + // The desktop crate is tested in its default graph, then under mesh-llm: + // only tests the default graph does not list reach the mesh one, a mesh + // test repeats there, and one no graph lists fails the lane by name while + // the rest still repeat. An ignored test is excluded, not unlisted. + const desktop = { + pkg: "buzz-desktop", + binary: "buzz-desktop", + binaryName: "buzz_lib", + }; + const tauriTests = [ + { ...desktop, name: "tests::plain" }, + { ...desktop, name: "tests::skipped", ignored: true }, + { + ...desktop, + name: "managed_agents::relay_mesh::tests::mesh_only", + feature: "mesh-llm", + }, + ]; + const tauri = repeatWithFakeCargo( + "tauri", + [ + "desktop/src-tauri/src/lib.rs::tests::plain", + "desktop/src-tauri/src/lib.rs::tests::skipped", + "desktop/src-tauri/src/managed_agents/relay_mesh.rs::tests::mesh_only", + "desktop/src-tauri/src/lib.rs::tests::macos_only", + ], + tauriTests, + ); + assert.equal(tauri.status, 1, tauri.summary); + assert.deepEqual( + tauri.calls.map((call) => [call[1], features(call)]), + [ + ["list", ""], + ["list", "mesh-llm"], + ["run", ""], + ["run", "mesh-llm"], + ["run", ""], + ["run", "mesh-llm"], + ], + ); + assert.match( + tauri.summary, + /## Flake check \(Desktop Tauri\): 1 selected test\(s\) not listed\n[^#]*- `desktop\/src-tauri\/src\/lib\.rs::tests::macos_only`\n/, + ); + assert.doesNotMatch(tauri.summary, /skipped`/); + assert.match( + tauri.summary, + /## Flake check \(Desktop Tauri\): 0 of 2 iterations failed/, + ); + assert.match( + tauri.summary, + /\| buzz-desktop managed_agents::relay_mesh::tests::mesh_only \| ✅ \| ✅ \| 0\/2 \|/, + ); + assert.match( + tauri.summary, + /\| buzz-desktop tests::plain \| ✅ \| ✅ \| 0\/2 \|/, + ); + + // A selection of only ignored tests is excluded, and passes as such; one + // of only unlisted tests fails without running anything. + const ignored = repeatWithFakeCargo( + "tauri", + ["desktop/src-tauri/src/lib.rs::tests::skipped"], + tauriTests, + ); + assert.equal(ignored.status, 0, ignored.summary); + assert.match( + ignored.summary, + /## Flake check \(Desktop Tauri\): nothing to repeat/, + ); + const unlisted = repeatWithFakeCargo( + "tauri", + ["desktop/src-tauri/src/lib.rs::tests::macos_only"], + tauriTests, + ); + assert.equal(unlisted.status, 1, unlisted.summary); + assert.deepEqual( + unlisted.calls.map((call) => call[1]), + ["list", "list"], + ); + assert.doesNotMatch(unlisted.summary, /nothing to repeat/); +}); From ba859ae1ecf54fdef66b6f9fdc38e6be05dc7b09 Mon Sep 17 00:00:00 2001 From: Matt Toohey <contact@matttoohey.com> Date: Thu, 29 Oct 2026 11:13:11 +1100 Subject: [PATCH 7/7] fix(ci): repeat Rust tests in every CI feature graph that lists them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address Jude's review 5684023609 on #8613: repeatRust narrowed each selected test to the first feature graph that listed it, so a test the default graph lists was never looked for in a later graph. buzz-push-gateway is tested without and with personal-dev-app-attest, and config.rs's attestation_environment_is_explicit_and_build_gated asserts differently under `#[cfg(feature = …)]` and `#[cfg(not(feature = …))]`; a change to the feature arm selected the test, repeated it in the default graph alone and passed without compiling the changed arm. Each package is now listed in every graph the regular lanes test it in, every selected test repeats in each graph that lists it, and only an entry no graph lists fails the lane. Report rows and the expected-status keys carry the graph's features (`… [personal-dev-app-attest]`), so one graph's pass cannot stand for another's failure. rustGroups builds each graph's full cargo arguments; the desktop crate's mesh-llm graph is lib-only, as desktop-tauri-test-mesh runs `--lib`, so an integration-test selection never builds it and a lib selection builds only the lib under it. The flake-tauri lane's timeout grows to 70 minutes for that second cold build. The fake-cargo regression now covers the gateway test listed in both graphs with a failure only under the feature: the lane runs both graphs and exits 1 with the feature row red and the default row green; restoring the first-graph narrowing or dropping the per-graph keys fails it. Verified live with cargo-nextest: the real selection runs in both graphs and passes, and with the feature arm's expected value changed the default graph passes while the feature graph fails every iteration. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Matt Toohey <contact@matttoohey.com> --- .github/workflows/_ci-flake.yml | 15 +-- CONTRIBUTING.md | 10 +- scripts/flake-check.mjs | 64 +++++++----- scripts/flake-check.test.mjs | 176 +++++++++++++++++++++++++------- 4 files changed, 193 insertions(+), 72 deletions(-) diff --git a/.github/workflows/_ci-flake.yml b/.github/workflows/_ci-flake.yml index 94d98c82c90..ff527ddb7d1 100644 --- a/.github/workflows/_ci-flake.yml +++ b/.github/workflows/_ci-flake.yml @@ -112,9 +112,10 @@ jobs: needs: [flake-select] if: needs.flake-select.outputs.tauri != '' runs-on: ubuntu-latest - # Same bound as a Desktop Tauri lane (_ci-desktop.yml): ci-apt-retry.sh - # bounds the apt step at ~34m worst case and a cold build needs ~15m. - timeout-minutes: 55 + # A Desktop Tauri lane's bound (_ci-desktop.yml: ci-apt-retry.sh bounds + # the apt step at ~34m worst case and a cold build needs ~15m), plus a + # second cold build: a lib selection also builds the mesh-llm graph. + timeout-minutes: 70 permissions: contents: read steps: @@ -157,10 +158,10 @@ jobs: - name: Create sidecar stubs run: just _ensure-sidecar-stubs # Same script as flake-rust, against the Tauri workspace's manifest and - # its own .config/nextest.toml `flake` profile. A selected test the - # default graph does not list is looked for under mesh-llm, whose - # dependency graph the restored desktop-core cache does not hold, so - # only such a selection builds it cold. + # its own .config/nextest.toml `flake` profile. A selected lib test also + # repeats under mesh-llm, as desktop-tauri-test-mesh runs it; the + # restored desktop-core cache does not hold that dependency graph, so a + # lib selection builds it cold, and an integration-test one never does. - name: Repeat changed desktop Tauri tests env: CMAKE_POLICY_VERSION_MINIMUM: "3.5" diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 9c155a5d170..e392a8b252f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -322,11 +322,13 @@ are deliberately not repeated: `#[ignore]`d and Postgres-backed Rust tests, Playwright specs outside the `smoke` project, `*.perf.ts` measurements, and a selection too large for its lane (over 64 KiB of entries, or more Playwright journeys than ten repetitions can finish within the lane's timeout). -A Rust test repeats in the first feature graph the regular lanes test its +A Rust test repeats in every feature graph the regular lanes test its package in that lists it (LaunchDarkly, the App Attest development feature, -the desktop crate's `mesh-llm`); a selected test that no such graph lists on -the Linux runner, such as a macOS-only one, fails its lane by name rather -than passing unrepeated. +the desktop crate's lib under `mesh-llm`), since a `#[cfg(feature = …)]` arm +compiles only in its own graph; the step summary marks a row from a feature +graph with that feature. A selected test that no such graph lists on the +Linux runner, such as a macOS-only one, fails its lane by name rather than +passing unrepeated. The lanes skip when no test changed and are not required to merge; their step summaries show the selection and the test × iteration table. diff --git a/scripts/flake-check.mjs b/scripts/flake-check.mjs index aa26b0c4d31..3dd4fe570d0 100644 --- a/scripts/flake-check.mjs +++ b/scripts/flake-check.mjs @@ -107,12 +107,17 @@ export const infraTest = (binaryName, name) => // The feature graphs the regular lanes test a package in, in order, where // that is not just its default graph: the Justfile's test-unit runs // buzz-push-gateway without and with its feature and buzz-feature-flags only -// with LaunchDarkly, and desktop-tauri-test-mesh runs the desktop crate under -// mesh-llm. A test behind `#[cfg(feature = …)]` is listed only in its graph. +// with LaunchDarkly, and desktop-tauri-test-mesh runs the desktop crate's lib +// tests (`lib`: no other target) under mesh-llm. A test behind +// `#[cfg(feature = …)]` is listed only in its graph, and one listed in several +// can compile differently in each (`#[cfg(not(feature = …))]` arms). export const rustFeatureGraphs = { - "buzz-feature-flags": [["launchdarkly"]], - "buzz-push-gateway": [[], ["personal-dev-app-attest"]], - "buzz-desktop": [[], ["mesh-llm"]], + "buzz-feature-flags": [{ features: ["launchdarkly"] }], + "buzz-push-gateway": [ + { features: [] }, + { features: ["personal-dev-app-attest"] }, + ], + "buzz-desktop": [{ features: [] }, { features: ["mesh-llm"], lib: true }], }; // Mirrors desktop/package.json "test", desktop/playwright.config.ts testDir, @@ -897,8 +902,9 @@ export function chooseRust(tests, candidates) { // 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), the feature -// graphs to list them in, and the tests to list in those targets. Every entry -// must belong to one workspace, whose runner is returned with the groups. +// graphs to list them in with each graph's full arguments, and the tests to +// list in those targets. Every entry must belong to one workspace, whose +// runner is returned with the groups. export function rustGroups(entries) { const groups = new Map(); const workspaces = new Set(); @@ -930,7 +936,16 @@ export function rustGroups(entries) { crate, ...(group.every ? [] : [...group.targets.values()].flat()), ]; - group.graphs = rustFeatureGraphs[crate] ?? [[]]; + // A lib-only graph applies only where the selection can reach the lib. + group.graphs = (rustFeatureGraphs[crate] ?? [{ features: [] }]) + .filter(({ lib }) => !lib || group.every || group.targets.has("--lib")) + .map(({ features, lib }) => ({ + features, + args: [ + ...(lib ? ["-p", crate, "--lib"] : group.args), + ...(features.length ? ["--features", features.join(",")] : []), + ], + })); } return { groups, runner: [...workspaces][0] }; } @@ -939,11 +954,13 @@ export function rustGroups(entries) { // the `flake` profile (the workspace's .config/nextest.toml: no retries, JUnit // per run), one cargo invocation per package and feature graph so each builds // only the targets its files can be in. Each package's targets are listed -// whole in its feature graphs in turn (nextest reports an ignored test as a +// whole in each of its feature graphs (nextest reports an ignored test as a // filter mismatch whatever `-E` says, so a filtered listing could not tell an // ignored entry from an unlisted one), and every entry is narrowed with -// chooseRust to exact (binary, test) pairs in the first graph listing it, so -// a later graph (and its build) is reached only by tests behind its feature. +// chooseRust to exact (binary, test) pairs in every graph listing it, as the +// regular lanes run it in each: a `#[cfg(feature = …)]` arm inside a test +// the default graph lists compiles only in its feature's graph. Statuses are +// keyed per graph, so one graph's pass cannot stand for another's failure. // An entry standing only for ignored or infrastructure-backed tests is // excluded; one no graph lists (a test behind a cfg the regular lanes do not // build here) fails the lane rather than passing unrepeated. The Tauri @@ -957,13 +974,8 @@ function repeatRust(entries, iterations) { const invocations = []; const unlisted = []; for (const [crate, group] of groups) { - let tests = group.tests; - for (const features of group.graphs) { - if (tests.length === 0) break; - const args = [ - ...group.args, - ...(features.length ? ["--features", features.join(",")] : []), - ]; + const missing = new Set(group.tests); + for (const { features, args } of group.graphs) { const listed = run("cargo", [ "nextest", "list", @@ -983,11 +995,14 @@ function repeatRust(entries, iterations) { runnable: !test.ignored && !infraTest(suite["binary-name"], name), })), ); - const choice = chooseRust(tests, candidates); + const choice = chooseRust(group.tests, candidates); + for (const test of group.tests) + if (!choice.unlisted.includes(test)) missing.delete(test); const chosen = choice.chosen.filter((test) => test.runnable); if (chosen.length) invocations.push({ label: [crate, ...features].join("+"), + tag: features.length ? ` [${features.join(",")}]` : "", args, chosen, filterset: chosen @@ -996,9 +1011,8 @@ function repeatRust(entries, iterations) { ) .join(" | "), }); - tests = choice.unlisted; } - unlisted.push(...tests.map((test) => test.entry)); + unlisted.push(...[...missing].map((test) => test.entry)); } if (unlisted.length) { summarize( @@ -1015,7 +1029,9 @@ function repeatRust(entries, iterations) { process.exitCode = 1; } const expected = invocations.flatMap((invocation) => - invocation.chosen.map(({ binary, name }) => `${binary} ${name}`), + invocation.chosen.map( + ({ binary, name }) => `${binary} ${name}${invocation.tag}`, + ), ); if (expected.length === 0) { if (!unlisted.length) @@ -1036,7 +1052,7 @@ function repeatRust(entries, iterations) { for (let i = 1; i <= iterations; i += 1) { const statuses = new Map(); let ok = true; - for (const { label, args, filterset } of invocations) { + for (const { label, tag, args, filterset } of invocations) { rmSync(junit, { force: true }); const result = run( "cargo", @@ -1055,7 +1071,7 @@ function repeatRust(entries, iterations) { ok &&= result.status === 0; try { for (const [test, status] of parseJunit(readFileSync(junit, "utf8"))) - statuses.set(test, status); + statuses.set(`${test}${tag}`, status); copyFileSync(junit, `${dir}/iter-${i}-${label}.xml`); } catch { ok = false; diff --git a/scripts/flake-check.test.mjs b/scripts/flake-check.test.mjs index bbc56cf785d..5450e484a17 100644 --- a/scripts/flake-check.test.mjs +++ b/scripts/flake-check.test.mjs @@ -444,20 +444,33 @@ test("Rust files build only the targets they can be part of, merged per package /rust entry must be/, ); // Packages the regular lanes test under a feature list in those graphs, in - // order; the rest in their default graph alone. + // order; the rest in their default graph alone. The desktop crate's + // mesh-llm graph builds only its lib, and only for a selection reaching it. + const graphs = (entries) => + [...rustGroups(entries).groups.values()][0].graphs.map(({ args }) => + args.join(" "), + ); assert.deepEqual( [ "crates/buzz-feature-flags/src/launchdarkly.rs::tests::a", "crates/buzz-push-gateway/src/lib.rs::tests::b", "crates/buzz-core/src/filter.rs::tests::c", - ].map((entry) => [...rustGroups([entry]).groups.values()][0].graphs), - [[["launchdarkly"]], [[], ["personal-dev-app-attest"]], [[]]], - ); - assert.deepEqual( + "desktop/src-tauri/src/lib.rs::tests::d", + "desktop/src-tauri/tests/rodio_mixer_diagnostic.rs::e", + ].map((entry) => graphs([entry])), [ - ...rustGroups(["desktop/src-tauri/src/lib.rs::tests::d"]).groups.values(), - ][0].graphs, - [[], ["mesh-llm"]], + ["-p buzz-feature-flags --lib --bins --features launchdarkly"], + [ + "-p buzz-push-gateway --lib --bins", + "-p buzz-push-gateway --lib --bins --features personal-dev-app-attest", + ], + ["-p buzz-core --lib --bins"], + [ + "-p buzz-desktop --lib --bins", + "-p buzz-desktop --lib --features mesh-llm", + ], + ["-p buzz-desktop --test rodio_mixer_diagnostic"], + ], ); // Postgres-backed tests are excluded by name and binary, as the postgres-ci // default-filter selects them. @@ -1026,14 +1039,14 @@ test("repeat invocations mirror the lanes they stand in for", () => { assert.ok(read("../.config/nextest.toml").includes(predicate), predicate); // Feature graphs mirror the Justfile recipes the regular lanes run: every // package test-unit runs under a feature, in each graph it runs there, and - // the desktop crate's default and mesh-llm lanes. + // the desktop crate's default and lib-only mesh-llm lanes. const justfile = read("../Justfile"); const graphs = {}; for (const [, crate, features] of justfile.matchAll( /^ +cargo nextest run -p ([\w-]+)(?: --features ([\w,-]+))?$/gm, )) { graphs[crate] ??= []; - graphs[crate].push(features?.split(",") ?? []); + graphs[crate].push({ features: features?.split(",") ?? [] }); } assert.ok( justfile.includes( @@ -1045,12 +1058,15 @@ test("repeat invocations mirror the lanes they stand in for", () => { "\ndesktop-tauri-test-mesh: _ensure-sidecar-stubs\n cargo test --manifest-path {{desktop_tauri_manifest}} --features mesh-llm --lib\n", ), ); - graphs["buzz-desktop"] = [[], ["mesh-llm"]]; + graphs["buzz-desktop"] = [ + { features: [] }, + { features: ["mesh-llm"], lib: true }, + ]; assert.deepEqual( rustFeatureGraphs, Object.fromEntries( Object.entries(graphs).filter(([, list]) => - list.some((features) => features.length), + list.some(({ features }) => features.length), ), ), ); @@ -1089,8 +1105,9 @@ test("repeat invocations mirror the lanes they stand in for", () => { // The repeat, through its entry point, in a scratch tree holding the script // and the packages its entries name, with a fake `cargo` on PATH listing -// `tests` ({pkg, binary, binaryName, name, feature?, ignored?}; a feature -// test only under that feature) and passing every test a run selects. +// `tests` ({pkg, binary, binaryName, name, feature?, ignored?, failsWith?}; a +// feature test only under that feature) and passing every test a run selects +// but one failing in the graph enabling its `failsWith` feature. function repeatWithFakeCargo(runner, entries, tests) { const dir = realpathSync(mkdtempSync(join(tmpdir(), "flake-repeat-"))); try { @@ -1101,6 +1118,7 @@ function repeatWithFakeCargo(runner, entries, tests) { ); for (const [crateDir, name] of [ ["crates/buzz-feature-flags", "buzz-feature-flags"], + ["crates/buzz-push-gateway", "buzz-push-gateway"], ["desktop/src-tauri", "buzz-desktop"], ]) { mkdirSync(join(dir, crateDir, "src"), { recursive: true }); @@ -1139,7 +1157,10 @@ if (args[1] === "list") { ); mkdirSync(dirname(junit), { recursive: true }); const cases = [...value("-E").matchAll(/binary_id\\(=([^)]+)\\) & test\\(=([^)]+)\\)/g)]; - writeFileSync(junit, "<testsuites>" + cases.map(([, binary, name]) => '<testcase name="' + name + '" classname="' + binary + '"/>').join("") + "</testsuites>"); + const fails = ([, binary, name]) => + tests.some((test) => test.binary === binary && test.name === name && features.includes(test.failsWith)); + writeFileSync(junit, "<testsuites>" + cases.map((match) => '<testcase name="' + match[2] + '" classname="' + match[1] + '">' + (fails(match) ? "<failure/>" : "") + "</testcase>").join("") + "</testsuites>"); + if (cases.some(fails)) process.exitCode = 100; } `, ); @@ -1186,12 +1207,16 @@ if (args[1] === "list") { } } -test("feature-gated Rust tests repeat in the graph the regular lanes test them in, and unlisted ones fail the lane", () => { - // Reviewer probe on #8613: the repeat listed only the default graph, so a +test("feature-gated Rust tests repeat in every graph the regular lanes test them in, and unlisted ones fail the lane", () => { + // Reviewer probes on #8613: the repeat listed only the default graph, so a // selected `#[cfg(feature = …)]` test was dropped and the lane passed as - // "nothing to repeat", or reported only its ungated neighbours. + // "nothing to repeat"; then it repeated each test in the first graph listing + // it alone, so a changed `#[cfg(feature = …)]` arm inside a test the default + // graph lists too was never compiled, and the lane passed. const features = (call) => call.includes("--features") ? call[call.indexOf("--features") + 1] : ""; + const targets = (call) => + call.filter((arg) => arg === "--lib" || arg === "--bins").join(" "); const flags = { pkg: "buzz-feature-flags", binary: "buzz-feature-flags", @@ -1229,17 +1254,65 @@ test("feature-gated Rust tests repeat in the graph the regular lanes test them i ); assert.match( root.summary, - /\| buzz-feature-flags launchdarkly::tests::null_events \| ✅ \| ✅ \| 0\/2 \|/, + /\| buzz-feature-flags launchdarkly::tests::null_events \[launchdarkly\] \| ✅ \| ✅ \| 0\/2 \|/, ); assert.match( root.summary, - /\| buzz-feature-flags tests::plain \| ✅ \| ✅ \| 0\/2 \|/, + /\| buzz-feature-flags tests::plain \[launchdarkly\] \| ✅ \| ✅ \| 0\/2 \|/, ); - // The desktop crate is tested in its default graph, then under mesh-llm: - // only tests the default graph does not list reach the mesh one, a mesh - // test repeats there, and one no graph lists fails the lane by name while - // the rest still repeat. An ignored test is excluded, not unlisted. + // buzz-push-gateway is tested without and with personal-dev-app-attest, and + // config.rs's attestation_environment_is_explicit_and_build_gated asserts + // differently in each: it repeats in both graphs, and a change breaking only + // the feature's arm fails the lane on that graph's row while the default + // graph's row passes. + const gateway = { + pkg: "buzz-push-gateway", + binary: "buzz-push-gateway", + binaryName: "buzz_push_gateway", + }; + const gated = repeatWithFakeCargo( + "rust", + [ + "crates/buzz-push-gateway/src/config.rs::tests::attestation_environment_is_explicit_and_build_gated", + ], + [ + { + ...gateway, + name: "config::tests::attestation_environment_is_explicit_and_build_gated", + failsWith: "personal-dev-app-attest", + }, + ], + ); + assert.equal(gated.status, 1, gated.summary); + assert.deepEqual( + gated.calls.map((call) => [call[1], features(call)]), + [ + ["list", ""], + ["list", "personal-dev-app-attest"], + ["run", ""], + ["run", "personal-dev-app-attest"], + ["run", ""], + ["run", "personal-dev-app-attest"], + ], + ); + assert.match( + gated.summary, + /## Flake check \(Rust\): 2 of 2 iterations failed/, + ); + assert.match( + gated.summary, + /\| buzz-push-gateway config::tests::attestation_environment_is_explicit_and_build_gated \| ✅ \| ✅ \| 0\/2 \|/, + ); + assert.match( + gated.summary, + /\| buzz-push-gateway config::tests::attestation_environment_is_explicit_and_build_gated \[personal-dev-app-attest\] \| ❌ \| ❌ \| 2\/2 \|/, + ); + + // The desktop crate's lib is tested in its default graph and under + // mesh-llm, which builds the lib alone: a lib test repeats in both, a mesh + // test in the mesh graph, and one no graph lists fails the lane by name + // while the rest still repeat. An ignored test is excluded, not unlisted. const desktop = { pkg: "buzz-desktop", binary: "buzz-desktop", @@ -1253,6 +1326,12 @@ test("feature-gated Rust tests repeat in the graph the regular lanes test them i name: "managed_agents::relay_mesh::tests::mesh_only", feature: "mesh-llm", }, + { + ...desktop, + binary: "buzz-desktop::rodio_mixer_diagnostic", + binaryName: "rodio_mixer_diagnostic", + name: "mixes", + }, ]; const tauri = repeatWithFakeCargo( "tauri", @@ -1266,14 +1345,14 @@ test("feature-gated Rust tests repeat in the graph the regular lanes test them i ); assert.equal(tauri.status, 1, tauri.summary); assert.deepEqual( - tauri.calls.map((call) => [call[1], features(call)]), + tauri.calls.map((call) => [call[1], features(call), targets(call)]), [ - ["list", ""], - ["list", "mesh-llm"], - ["run", ""], - ["run", "mesh-llm"], - ["run", ""], - ["run", "mesh-llm"], + ["list", "", "--lib --bins"], + ["list", "mesh-llm", "--lib"], + ["run", "", "--lib --bins"], + ["run", "mesh-llm", "--lib"], + ["run", "", "--lib --bins"], + ["run", "mesh-llm", "--lib"], ], ); assert.match( @@ -1285,13 +1364,36 @@ test("feature-gated Rust tests repeat in the graph the regular lanes test them i tauri.summary, /## Flake check \(Desktop Tauri\): 0 of 2 iterations failed/, ); - assert.match( - tauri.summary, - /\| buzz-desktop managed_agents::relay_mesh::tests::mesh_only \| ✅ \| ✅ \| 0\/2 \|/, + for (const row of [ + "managed_agents::relay_mesh::tests::mesh_only [mesh-llm]", + "tests::plain", + "tests::plain [mesh-llm]", + ]) + assert.ok( + tauri.summary.includes(`| buzz-desktop ${row} | ✅ | ✅ | 0/2 |`), + row, + ); + assert.doesNotMatch(tauri.summary, /mesh_only \|/); + + // An integration test selection never reaches the lib-only mesh graph. + const integration = repeatWithFakeCargo( + "tauri", + ["desktop/src-tauri/tests/rodio_mixer_diagnostic.rs::mixes"], + tauriTests, ); - assert.match( - tauri.summary, - /\| buzz-desktop tests::plain \| ✅ \| ✅ \| 0\/2 \|/, + assert.equal(integration.status, 0, integration.summary); + assert.deepEqual( + integration.calls.map((call) => [call[1], features(call)]), + [ + ["list", ""], + ["run", ""], + ["run", ""], + ], + ); + assert.ok( + integration.summary.includes( + "| buzz-desktop::rodio_mixer_diagnostic mixes | ✅ | ✅ | 0/2 |", + ), ); // A selection of only ignored tests is excluded, and passes as such; one