fix(ci): split release chat verification into test shards
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Devin Foley
parent
5b0417c88c
commit
dd0e0ac59d
|
|
@ -58,30 +58,37 @@ jobs:
|
|||
fail-fast: false
|
||||
matrix:
|
||||
include:
|
||||
# Five-way server split, matching pr-trusted.yml. Three shards sat
|
||||
# at 17-19 minutes against this job's 20-minute cap after the
|
||||
# server suite grew on 2026-09-10, and every canary that evening
|
||||
# died on the timeout instead of reporting a verdict.
|
||||
- group: general-server
|
||||
group_label: server (1/5)
|
||||
# Split the long chat file by collected test locations, and balance
|
||||
# the remaining server files across four runners. Normal PR/local
|
||||
# invocations retain their complete general-server group.
|
||||
- group: general-server-without-chat
|
||||
group_label: server (1/4)
|
||||
shard_index: 0
|
||||
shard_count: 5
|
||||
- group: general-server
|
||||
group_label: server (2/5)
|
||||
shard_count: 4
|
||||
- group: general-server-without-chat
|
||||
group_label: server (2/4)
|
||||
shard_index: 1
|
||||
shard_count: 5
|
||||
- group: general-server
|
||||
group_label: server (3/5)
|
||||
shard_count: 4
|
||||
- group: general-server-without-chat
|
||||
group_label: server (3/4)
|
||||
shard_index: 2
|
||||
shard_count: 5
|
||||
- group: general-server
|
||||
group_label: server (4/5)
|
||||
shard_count: 4
|
||||
- group: general-server-without-chat
|
||||
group_label: server (4/4)
|
||||
shard_index: 3
|
||||
shard_count: 5
|
||||
- group: general-server
|
||||
group_label: server (5/5)
|
||||
shard_index: 4
|
||||
shard_count: 5
|
||||
shard_count: 4
|
||||
- group: general-chat
|
||||
group_label: chat (1/3)
|
||||
shard_index: 0
|
||||
shard_count: 3
|
||||
- group: general-chat
|
||||
group_label: chat (2/3)
|
||||
shard_index: 1
|
||||
shard_count: 3
|
||||
- group: general-chat
|
||||
group_label: chat (3/3)
|
||||
shard_index: 2
|
||||
shard_count: 3
|
||||
# Keep parity with pr.yml: workspaces-a is split with Vitest's
|
||||
# native --shard because the ui project dominates the lane.
|
||||
- group: general-workspaces-a
|
||||
|
|
|
|||
|
|
@ -359,3 +359,28 @@ default branch) are master here. A workflow with authority to execute arbitrary
|
|||
code on master can affect verification directly and is already trusted. The
|
||||
cache contains dependency build artifacts, not credentials or workspace output.
|
||||
See [GitHub cache access restrictions](https://docs.github.com/en/actions/reference/workflows-and-actions/dependency-caching#restrictions-for-accessing-a-cache).
|
||||
|
||||
## Chat integration test shards
|
||||
|
||||
Release verification runs the large chat integration file on three independent
|
||||
runners. Four other server shards cover every remaining general server file.
|
||||
The ordinary local test command and trusted PR workflow keep their complete
|
||||
`general-server` group. No application test assertions or fixtures change.
|
||||
|
||||
Each chat job collects active tests with Vitest, groups cases by source line,
|
||||
and balances those groups by case count. Parameterized cases and loop-generated
|
||||
cases on one line stay together. The job re-collects with the exact line filters
|
||||
it will execute and fails if the selected case identities differ. Hooks and test
|
||||
execution remain sequential inside each runner with its own temporary home.
|
||||
|
||||
Run one shard locally with:
|
||||
|
||||
```sh
|
||||
pnpm test:run:general -- --group general-chat --shard-index 0 --shard-count 3
|
||||
```
|
||||
|
||||
Use indexes 0, 1, and 2 to run the complete chat suite. The CLI validates that
|
||||
each shard has work and that collection includes usable source locations. A
|
||||
Vitest collection or filtering change fails verification instead of dropping
|
||||
tests. Splitting adds two release-verification jobs and repeats collection and
|
||||
fixture setup; it does not make a single test faster.
|
||||
|
|
|
|||
|
|
@ -206,28 +206,16 @@ test("release verify workflow covers the same split test surface as stable PR ve
|
|||
assert.match(buildJob, /persist-credentials: false/);
|
||||
assert.doesNotMatch(buildJob, /cache: pnpm/);
|
||||
|
||||
for (const group of [
|
||||
"general-server",
|
||||
"general-workspaces-a",
|
||||
"general-workspaces-b",
|
||||
]) {
|
||||
for (const group of ["general-server-without-chat", "general-chat", "general-workspaces-a", "general-workspaces-b"]) {
|
||||
assert.match(verifyWorkflow, new RegExp(`group: ${group}`));
|
||||
}
|
||||
|
||||
for (const shardIndex of [0, 1, 2, 3, 4]) {
|
||||
assert.match(
|
||||
verifyWorkflow,
|
||||
new RegExp(
|
||||
`group: general-server[\\s\\S]*?shard_index: ${shardIndex}[\\s\\S]*?shard_count: 5`,
|
||||
),
|
||||
);
|
||||
for (const [group, count] of [["general-server-without-chat", 4], ["general-chat", 3]]) {
|
||||
const rows = [...verifyWorkflow.matchAll(new RegExp(`group: ${group}\\n\\s+group_label: [^\\n]+\\n\\s+shard_index: (\\d+)\\n\\s+shard_count: (\\d+)`, "g"))];
|
||||
assert.deepEqual(rows.map((row) => [Number(row[1]), Number(row[2])]),
|
||||
Array.from({ length: count }, (_, index) => [index, count]));
|
||||
}
|
||||
|
||||
for (const shardIndex of [0, 1, 2, 3, 4]) {
|
||||
assert.match(
|
||||
verifyWorkflow,
|
||||
new RegExp(`shard_index: ${shardIndex}[\\s\\S]*?shard_count: 5`),
|
||||
);
|
||||
assert.match(verifyWorkflow, new RegExp(`shard_index: ${shardIndex}[\\s\\S]*?shard_count: 5`));
|
||||
}
|
||||
|
||||
// workspaces-a splits with Vitest native --shard in pr.yml; release
|
||||
|
|
|
|||
|
|
@ -10,6 +10,8 @@ import {
|
|||
partitionGeneralServerSuites,
|
||||
} from "../general-server-shard.mjs";
|
||||
|
||||
import { assertSelectedTests, partitionTestLines } from "../test-line-shard.mjs";
|
||||
|
||||
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", "..");
|
||||
const script = path.join(repoRoot, "scripts", "run-vitest-stable.mjs");
|
||||
const durationsManifest = path.join(repoRoot, "scripts", "general-server-shard-durations.json");
|
||||
|
|
@ -264,3 +266,53 @@ test("the real shard partition is duration-balanced", () => {
|
|||
`shard weight spread ${maxTotal - minTotal}ms exceeds heaviest suite ${heaviest}ms: ${totals.join(", ")}`,
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
test("release server shards plus the dedicated chat file cover the original server group exactly", () => {
|
||||
const full = dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", "0", "--shard-count", "1"]);
|
||||
const shards = Array.from({ length: 4 }, (_, index) => dryRunJson([
|
||||
"--mode", "general", "--group", "general-server-without-chat",
|
||||
"--shard-index", String(index), "--shard-count", "4",
|
||||
]));
|
||||
const files = shards.flatMap((shard) => shard.selectedGeneralServerSuites);
|
||||
const chat = "server/src/__tests__/chat-channels.integration.test.ts";
|
||||
assert.ok(!files.includes(chat));
|
||||
assert.deepEqual([...files, chat].sort(), full.selectedGeneralServerSuites.sort());
|
||||
assert.equal(new Set(files).size, files.length);
|
||||
const defaultRun = dryRunJson([]);
|
||||
assert.ok(defaultRun.generalServerSuiteCount === full.generalServerSuiteCount);
|
||||
});
|
||||
|
||||
const lineShardFile = path.join(repoRoot, "server/src/__tests__/chat-channels.integration.test.ts");
|
||||
const caseAt = (line, name) => ({ name, file: lineShardFile, projectName: "@paperclipai/server", location: { line, column: 3 } });
|
||||
|
||||
test("test-line shards cover nested and parameterized cases exactly once without splitting a source line", () => {
|
||||
const cases = [caseAt(10, "suite > nested > first"), caseAt(10, "suite > nested > second"),
|
||||
caseAt(20, "same name"), caseAt(30, "same name"), caseAt(40, "last"), caseAt(50, "new case")];
|
||||
const shards = partitionTestLines(cases, 3, lineShardFile);
|
||||
assert.deepEqual(shards.map((shard) => shard.tests.length), [2, 2, 2]);
|
||||
assert.equal(shards.filter((shard) => shard.lines.includes(10)).length, 1);
|
||||
assert.equal(shards.find((shard) => shard.lines.includes(10)).tests.length, 2);
|
||||
assert.equal(shards.flatMap((shard) => shard.lines).length, 5);
|
||||
assert.deepEqual(shards.flatMap((shard) => shard.tests).sort((a, b) => a.location.line - b.location.line), cases);
|
||||
assert.deepEqual(partitionTestLines([...cases].reverse(), 3, lineShardFile).map((shard) => shard.lines), shards.map((shard) => shard.lines));
|
||||
});
|
||||
|
||||
test("line-shard collection rejects empty, foreign, or unlocated tests and invalid shard counts", () => {
|
||||
const good = caseAt(10, "valid");
|
||||
for (const input of [[], null, [{ ...good, file: "/another.test.ts" }], [{ ...good, projectName: "wrong" }],
|
||||
[{ ...good, location: undefined }], [{ ...good, location: { line: 0 } }], [{ ...good, name: "" }]]) {
|
||||
assert.throws(() => partitionTestLines(input, 1, lineShardFile));
|
||||
}
|
||||
for (const count of [0, -1, 1.5, Infinity, 2]) assert.throws(() => partitionTestLines([good], count, lineShardFile));
|
||||
});
|
||||
|
||||
test("filtered collection must match the exact assigned case identities, including duplicates", () => {
|
||||
const expected = [caseAt(10, "same"), caseAt(10, "same"), caseAt(20, "nested > case")];
|
||||
assertSelectedTests(expected, [...expected].reverse(), lineShardFile);
|
||||
for (const actual of [expected.slice(1), [...expected, caseAt(30, "extra")],
|
||||
[expected[0], expected[1], caseAt(20, "renamed")],
|
||||
[expected[0], expected[1], caseAt(21, "nested > case")]]) {
|
||||
assert.throws(() => assertSelectedTests(expected, actual, lineShardFile));
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,11 +1,13 @@
|
|||
#!/usr/bin/env node
|
||||
import { spawnSync } from "node:child_process";
|
||||
import { mkdirSync, mkdtempSync, readdirSync, realpathSync, statSync } from "node:fs";
|
||||
import { mkdirSync, mkdtempSync, readdirSync, readFileSync, realpathSync, statSync } from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { loadShardDurations, selectGeneralServerShard } from "./general-server-shard.mjs";
|
||||
|
||||
import { assertSelectedTests, partitionTestLines } from "./test-line-shard.mjs";
|
||||
|
||||
const repoRoot = process.cwd();
|
||||
const scriptsDir = path.dirname(fileURLToPath(import.meta.url));
|
||||
const generalServerShardDurations = loadShardDurations(
|
||||
|
|
@ -66,11 +68,15 @@ const serializedModeName = "serialized";
|
|||
const generalModeName = "general";
|
||||
const allModeName = "all";
|
||||
const generalServerGroupName = "general-server";
|
||||
const generalServerWithoutChatGroupName = "general-server-without-chat";
|
||||
const generalChatGroupName = "general-chat";
|
||||
const chatSuite = "server/src/__tests__/chat-channels.integration.test.ts";
|
||||
const generalWorkspacesAGroupName = "general-workspaces-a";
|
||||
const generalWorkspacesBGroupName = "general-workspaces-b";
|
||||
const generalWorkspacesAProjects = ["@paperclipai/ui", "paperclipai"];
|
||||
const generalWorkspacesBProjects = nonServerProjects.filter((project) => !generalWorkspacesAProjects.includes(project));
|
||||
const generalGroupNames = [generalServerGroupName, generalWorkspacesAGroupName, generalWorkspacesBGroupName];
|
||||
const allowedGeneralGroupNames = [...generalGroupNames, generalServerWithoutChatGroupName, generalChatGroupName];
|
||||
const serializedServerVitestArgs = [
|
||||
"--no-file-parallelism",
|
||||
"--maxWorkers=1",
|
||||
|
|
@ -216,10 +222,10 @@ function parseCliOptions(argv) {
|
|||
const shardAllowed =
|
||||
mode === serializedModeName ||
|
||||
(mode === generalModeName &&
|
||||
(group === generalServerGroupName || group === generalWorkspacesAGroupName));
|
||||
([generalServerGroupName, generalServerWithoutChatGroupName, generalChatGroupName, generalWorkspacesAGroupName].includes(group)));
|
||||
if (!shardAllowed && shardIndex !== null) {
|
||||
fail(
|
||||
"--shard-index/--shard-count are only valid with --mode serialized, --mode general --group general-server, or --mode general --group general-workspaces-a.",
|
||||
"--shard-index/--shard-count are only valid with serialized mode or a shardable general server/chat/workspaces-a group.",
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -227,8 +233,8 @@ function parseCliOptions(argv) {
|
|||
fail("--group is only valid with --mode general.");
|
||||
}
|
||||
|
||||
if (group !== null && !generalGroupNames.includes(group)) {
|
||||
fail(`Unknown group "${group}". Expected one of: ${generalGroupNames.join(", ")}.`);
|
||||
if (group !== null && !allowedGeneralGroupNames.includes(group)) {
|
||||
fail(`Unknown group "${group}". Expected one of: ${allowedGeneralGroupNames.join(", ")}.`);
|
||||
}
|
||||
|
||||
if (shardIndex !== null) {
|
||||
|
|
@ -271,7 +277,7 @@ function selectSerializedSuites(routeTests, shardIndex, shardCount) {
|
|||
return shardFiles.map((file) => byRepoPath.get(file));
|
||||
}
|
||||
|
||||
function runVitest(args, label) {
|
||||
function runVitest(args, label, testShard = null) {
|
||||
console.log(`\n[test:run] ${label}`);
|
||||
invocationIndex += 1;
|
||||
const tempRootParent = process.platform === "win32" ? os.tmpdir() : "/tmp";
|
||||
|
|
@ -291,6 +297,25 @@ function runVitest(args, label) {
|
|||
};
|
||||
mkdirSync(env.PAPERCLIP_HOME, { recursive: true });
|
||||
mkdirSync(env.TMPDIR, { recursive: true });
|
||||
if (testShard) {
|
||||
const collect = (filters, name) => {
|
||||
const output = path.join(testRoot, `${name}.json`);
|
||||
const result = spawnSync("pnpm", ["exec", "vitest", "list", ...sourceOnlyVitestArgs,
|
||||
...filters, "--allowOnly=false", "--includeTaskLocation", `--json=${output}`], {
|
||||
cwd: repoRoot, env, stdio: "inherit",
|
||||
});
|
||||
if (result.error || result.status !== 0) fail(`Vitest collection failed: ${result.error?.message ?? result.status}`);
|
||||
return JSON.parse(readFileSync(output, "utf8"));
|
||||
};
|
||||
const collected = collect(args, "all");
|
||||
const file = path.resolve(repoRoot, chatSuite);
|
||||
const selected = partitionTestLines(collected, testShard.count, file)[testShard.index];
|
||||
const filters = selected.lines.map((line) => `${chatSuite}:${line}`);
|
||||
args = [...args.filter((arg) => arg !== chatSuite), ...filters];
|
||||
assertSelectedTests(selected.tests, collect(args, "selected"), file);
|
||||
console.log(`[test:run] chat shard ${testShard.index + 1}/${testShard.count}: ${selected.tests.length}/${collected.length} tests, ${selected.lines.length} source lines; exact filter coverage verified`);
|
||||
args.push("--allowOnly=false");
|
||||
}
|
||||
const result = spawnSync("pnpm", ["exec", "vitest", "run", ...sourceOnlyVitestArgs, ...args], {
|
||||
cwd: repoRoot,
|
||||
env,
|
||||
|
|
@ -325,16 +350,23 @@ function runProjectGroup(projects, groupName, shardIndex = null, shardCount = nu
|
|||
}
|
||||
|
||||
function runGeneralGroup(routeTests, groupName, shardIndex = null, shardCount = null) {
|
||||
if (groupName === generalServerGroupName) {
|
||||
if (groupName === generalChatGroupName) {
|
||||
runVitest(["--project", "@paperclipai/server", ...serializedServerVitestArgs, chatSuite],
|
||||
"chat integration test shard", { index: shardIndex ?? 0, count: shardCount ?? 1 });
|
||||
return;
|
||||
}
|
||||
if (groupName === generalServerGroupName || groupName === generalServerWithoutChatGroupName) {
|
||||
const withoutChat = groupName === generalServerWithoutChatGroupName;
|
||||
const files = withoutChat ? generalServerTestFiles.filter((file) => file !== chatSuite) : generalServerTestFiles;
|
||||
if (shardCount !== null && shardCount > 1) {
|
||||
const shardFiles = selectGeneralServerShard(
|
||||
generalServerTestFiles,
|
||||
files,
|
||||
shardIndex,
|
||||
shardCount,
|
||||
generalServerShardDurations,
|
||||
);
|
||||
console.log(
|
||||
`\n[test:run] general-server shard ${shardIndex + 1}/${shardCount} running ${shardFiles.length} of ${generalServerTestFiles.length} suites`,
|
||||
`\n[test:run] general-server shard ${shardIndex + 1}/${shardCount} running ${shardFiles.length} of ${files.length} suites`,
|
||||
);
|
||||
if (shardFiles.length === 0) {
|
||||
return;
|
||||
|
|
@ -353,6 +385,7 @@ function runGeneralGroup(routeTests, groupName, shardIndex = null, shardCount =
|
|||
}
|
||||
|
||||
const excludeRouteArgs = routeTests.flatMap((file) => ["--exclude", file.serverPath]);
|
||||
if (withoutChat) excludeRouteArgs.push("--exclude", "src/__tests__/chat-channels.integration.test.ts");
|
||||
runVitest(
|
||||
[
|
||||
"--project",
|
||||
|
|
@ -436,16 +469,16 @@ if (options.dryRun) {
|
|||
shardIndex: options.shardIndex,
|
||||
shardCount: options.shardCount,
|
||||
group: options.group,
|
||||
availableGeneralGroups: generalGroupNames,
|
||||
availableGeneralGroups: allowedGeneralGroupNames,
|
||||
serializedSuiteCount: routeTests.length,
|
||||
selectedSerializedSuites: serializedSuites.map((routeTest) => routeTest.repoPath),
|
||||
generalServerSuiteCount: generalServerTestFiles.length,
|
||||
selectedGeneralServerSuites:
|
||||
options.mode === generalModeName &&
|
||||
options.group === generalServerGroupName &&
|
||||
[generalServerGroupName, generalServerWithoutChatGroupName].includes(options.group) &&
|
||||
options.shardCount !== null
|
||||
? selectGeneralServerShard(
|
||||
generalServerTestFiles,
|
||||
options.group === generalServerWithoutChatGroupName ? generalServerTestFiles.filter((file) => file !== chatSuite) : generalServerTestFiles,
|
||||
options.shardIndex,
|
||||
options.shardCount,
|
||||
generalServerShardDurations,
|
||||
|
|
|
|||
|
|
@ -0,0 +1,45 @@
|
|||
import assert from "node:assert/strict";
|
||||
import path from "node:path";
|
||||
|
||||
function validateTests(tests, file) {
|
||||
assert.ok(Array.isArray(tests) && tests.length > 0, "Vitest must collect at least one test");
|
||||
for (const test of tests) {
|
||||
assert.equal(test.projectName, "@paperclipai/server", "unexpected test project");
|
||||
assert.equal(path.resolve(test.file), path.resolve(file), "unexpected test file");
|
||||
assert.ok(typeof test.name === "string" && test.name.length > 0, "missing test name");
|
||||
assert.ok(Number.isSafeInteger(test.location?.line) && test.location.line > 0, "missing test source line");
|
||||
}
|
||||
}
|
||||
|
||||
// Keep all cases registered on one source line together, including it.each
|
||||
// and loop-generated cases. Balance by collected case count, not line count.
|
||||
export function partitionTestLines(tests, count, file) {
|
||||
validateTests(tests, file);
|
||||
assert.ok(Number.isSafeInteger(count) && count > 0, "invalid shard count");
|
||||
const byLine = new Map();
|
||||
for (const test of tests) {
|
||||
const line = test.location.line;
|
||||
if (!byLine.has(line)) byLine.set(line, []);
|
||||
byLine.get(line).push(test);
|
||||
}
|
||||
assert.ok(byLine.size >= count, "each shard must contain a source line");
|
||||
const groups = [...byLine].sort((a, b) => b[1].length - a[1].length || a[0] - b[0]);
|
||||
const shards = Array.from({ length: count }, () => ({ lines: [], tests: [] }));
|
||||
for (const [line, cases] of groups) {
|
||||
const shard = shards.reduce((best, next) => next.tests.length < best.tests.length ? next : best);
|
||||
shard.lines.push(line);
|
||||
shard.tests.push(...cases);
|
||||
}
|
||||
for (const shard of shards) shard.lines.sort((a, b) => a - b);
|
||||
return shards;
|
||||
}
|
||||
|
||||
// Re-collect using the exact filters passed to the subsequent test run. A
|
||||
// Vitest filtering change must fail here instead of silently dropping cases.
|
||||
export function assertSelectedTests(expected, actual, file) {
|
||||
validateTests(actual, file);
|
||||
const identities = (tests) => tests.map((test) => JSON.stringify([
|
||||
test.projectName, path.resolve(test.file), test.location.line, test.name,
|
||||
])).sort();
|
||||
assert.deepEqual(identities(actual), identities(expected), "Vitest filters must select exactly the assigned tests");
|
||||
}
|
||||
Loading…
Reference in New Issue