mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 16:35:48 +01:00
fix(ci): split release chat verification into test shards (#13198)
Split release chat verification into three validated test-line shards and balance other server suites across five runners using the measured native Runner integration cost. Retire each chat case's fixtures after assertions, preserve complete test coverage, and exercise the real shard CLI in PR tests. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
9e970df4c5
commit
42961b6ef1
9 files changed
+286
-40
No files matched your search
@@ -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", 5], ["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: 5 }, (_, index) => dryRunJson([
|
||||
"--mode", "general", "--group", "general-server-without-chat",
|
||||
"--shard-index", String(index), "--shard-count", "5",
|
||||
]));
|
||||
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,7 +1,9 @@
|
||||
{
|
||||
"$comment": "Per-suite wall-clock durations (ms) for the general-server vitest lane, used by scripts/general-server-shard.mjs to balance suites across the PR shard matrix. Sampled from a real PR run of .github/workflows/pr.yml (actions run 32708351172, 2026-08-24) by diffing consecutive per-suite completion timestamps in the 'Run grouped general test suites' logs \u2014 that captures each suite's true serial cost (import + collect + tests), not just the vitest-reported test time. Suites missing here get the median weight, so the manifest only needs occasional refreshes.",
|
||||
"$chatSample": "chat-channels.integration.test.ts: actions run 34405038082, job 102646337040, 2026-09-09. The first suite completed at 21:19:52.9026416Z after Vitest RUN at 21:09:07.5491992Z: 645354ms rounded up, including startup/import/collection; the 985 tests themselves took 629654ms. All 2972 tests in the shard passed, but the job exceeded its unchanged 20-minute bound during cleanup. Recording this missing heavy-suite weight lets the existing LPT partition reserve one of the existing five shards without changing test coverage, isolation, or deadlines.",
|
||||
"$nativeRunnerSample": "native-codex-runner.integration.test.ts: actions run 34555686996, job 103127786254, 2026-09-11. Consecutive suite completions at 02:50:34.2830368Z and 02:55:07.9837588Z give 273701ms including import/collection (test body 270773ms). This previously unweighted suite made one four-way server shard take 14m38s; recording its cost lets the existing LPT partition balance it in both PR and release runs.",
|
||||
"durations": {
|
||||
"server/src/services/native-runtime/native-codex-runner.integration.test.ts": 273701,
|
||||
"server/src/__tests__/access-service.test.ts": 4757,
|
||||
"server/src/__tests__/access-validators.test.ts": 645,
|
||||
"server/src/__tests__/activity-log-responsible-user.test.ts": 4407,
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
Reference in new issue
Block a user