mirror of
https://github.com/block/buzz.git
synced 2026-10-28 21:11:48 +00:00
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>
This commit is contained in:
10
.github/workflows/_ci-flake.yml
vendored
10
.github/workflows/_ci-flake.yml
vendored
@@ -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
|
||||
|
||||
@@ -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(" | ");
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
Reference in New Issue
Block a user