jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-10-27 04:30:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ci): keep flake-check tests hermetic to the Actions env and select #[path] wrapper targets

Browse Source 
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>
This commit is contained in:
Matt Toohey
2026-10-27 14:54:05 +11:00
parent f02a4235a9
commit 29858b2aff
3 changed files with 249 additions and 41 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
CONTRIBUTING.md
View File

@@ -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

128
scripts/flake-check.mjs
View File

@@ -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(

154
scripts/flake-check.test.mjs
View File

@@ -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");
});