mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 02:26:12 +02:00
perf(ci): shard the general-server test lane across 3 runners
The general-server group ran every server suite (213 files) serially in a single job because server/vitest.config.ts pins maxWorkers to 1. At ~7.2m of test execution it was the PR critical path. Server tests can only be parallelized across runners, so shard the lane into 3 even partitions (71/71/71) the same way the serialized route suites are sharded, cutting the lane to ~2.4m. Add a node:test that asserts the shards form a complete, non-overlapping partition of the general-server suite set (guarding against silent coverage gaps) and run it in the policy job. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
713b3b08f0
commit
b967076c24
3 files changed
+149
-15
No files matched your search
@@ -55,6 +55,9 @@ jobs:
|
||||
- name: Test no-git-push check
|
||||
run: node --test ./scripts/check-no-git-push.test.mjs
|
||||
|
||||
- name: Test general-server shard partition
|
||||
run: node --test ./scripts/__tests__/run-vitest-stable-shard.test.mjs
|
||||
|
||||
- name: Validate release package manifest
|
||||
run: node ./scripts/release-package-map.mjs check
|
||||
|
||||
@@ -135,8 +138,21 @@ jobs:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
include:
|
||||
# The server suite is pinned to maxWorkers=1 (server/vitest.config.ts),
|
||||
# so it can only be parallelized across runners. Shard it to keep this
|
||||
# lane off the PR critical path.
|
||||
- group: general-server
|
||||
group_label: server
|
||||
group_label: server (1/3)
|
||||
shard_index: 0
|
||||
shard_count: 3
|
||||
- group: general-server
|
||||
group_label: server (2/3)
|
||||
shard_index: 1
|
||||
shard_count: 3
|
||||
- group: general-server
|
||||
group_label: server (3/3)
|
||||
shard_index: 2
|
||||
shard_count: 3
|
||||
- group: general-workspaces-a
|
||||
group_label: workspaces-a
|
||||
- group: general-workspaces-b
|
||||
@@ -168,7 +184,13 @@ jobs:
|
||||
run: pnpm install --frozen-lockfile
|
||||
|
||||
- name: Run grouped general test suites
|
||||
run: pnpm test:run:general -- --group '${{ matrix.group }}'
|
||||
run: |
|
||||
if [ -n "${{ matrix.shard_count }}" ]; then
|
||||
pnpm test:run:general -- --group '${{ matrix.group }}' \
|
||||
--shard-index ${{ matrix.shard_index }} --shard-count ${{ matrix.shard_count }}
|
||||
else
|
||||
pnpm test:run:general -- --group '${{ matrix.group }}'
|
||||
fi
|
||||
|
||||
verify:
|
||||
# Preserve the legacy required-check name while the underlying work runs in parallel.
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
import assert from "node:assert/strict";
|
||||
import { spawnSync } from "node:child_process";
|
||||
import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import test from "node:test";
|
||||
|
||||
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", "..");
|
||||
const script = path.join(repoRoot, "scripts", "run-vitest-stable.mjs");
|
||||
|
||||
function dryRun(args) {
|
||||
const result = spawnSync(process.execPath, [script, ...args, "--dry-run"], {
|
||||
cwd: repoRoot,
|
||||
encoding: "utf8",
|
||||
});
|
||||
return result;
|
||||
}
|
||||
|
||||
function dryRunJson(args) {
|
||||
const result = dryRun(args);
|
||||
assert.equal(result.status, 0, `expected success for ${args.join(" ")}: ${result.stderr}`);
|
||||
return JSON.parse(result.stdout);
|
||||
}
|
||||
|
||||
const SHARD_COUNT = 3;
|
||||
|
||||
test("the general-server shards form a complete, non-overlapping partition", () => {
|
||||
const shards = Array.from({ length: SHARD_COUNT }, (_, index) =>
|
||||
dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", String(index), "--shard-count", String(SHARD_COUNT)]),
|
||||
);
|
||||
|
||||
const total = shards[0].generalServerSuiteCount;
|
||||
assert.ok(total > 0, "expected a non-empty general-server suite set");
|
||||
|
||||
const seen = new Set();
|
||||
let selectedTotal = 0;
|
||||
for (const shard of shards) {
|
||||
assert.equal(shard.generalServerSuiteCount, total, "suite count must be stable across shards");
|
||||
for (const file of shard.selectedGeneralServerSuites) {
|
||||
assert.ok(!seen.has(file), `suite assigned to more than one shard: ${file}`);
|
||||
seen.add(file);
|
||||
selectedTotal += 1;
|
||||
}
|
||||
}
|
||||
|
||||
// Every suite runs exactly once: union covers the whole set with no overlap.
|
||||
assert.equal(selectedTotal, total, "every suite must be selected exactly once");
|
||||
assert.equal(seen.size, total, "union of shards must cover the whole suite set");
|
||||
});
|
||||
|
||||
test("a route/authz suite never leaks into the general-server shards", () => {
|
||||
const shard = dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", "0", "--shard-count", SHARD_COUNT.toString()]);
|
||||
for (const file of shard.selectedGeneralServerSuites) {
|
||||
assert.ok(
|
||||
!/[^/]*(?:route|routes|authz)[^/]*\.test\.ts$/.test(file),
|
||||
`route/authz suite must stay in the serialized lane, not general-server: ${file}`,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
test("shard flags are rejected for the parallel workspace groups", () => {
|
||||
const result = dryRun(["--mode", "general", "--group", "general-workspaces-a", "--shard-index", "0", "--shard-count", "3"]);
|
||||
assert.notEqual(result.status, 0, "workspace groups must not accept shard flags");
|
||||
});
|
||||
@@ -6,6 +6,7 @@ import path from "node:path";
|
||||
|
||||
const repoRoot = process.cwd();
|
||||
const serverRoot = path.join(repoRoot, "server");
|
||||
const serverSrcDir = path.join(repoRoot, "server", "src");
|
||||
const serverTestsDir = path.join(repoRoot, "server", "src", "__tests__");
|
||||
const nonServerProjects = [
|
||||
"@paperclipai/shared",
|
||||
@@ -198,8 +199,13 @@ function parseCliOptions(argv) {
|
||||
fail("--shard-index and --shard-count must be provided together.");
|
||||
}
|
||||
|
||||
if (mode !== serializedModeName && shardIndex !== null) {
|
||||
fail("--shard-index/--shard-count are only valid with --mode serialized.");
|
||||
const shardAllowed =
|
||||
mode === serializedModeName ||
|
||||
(mode === generalModeName && group === generalServerGroupName);
|
||||
if (!shardAllowed && shardIndex !== null) {
|
||||
fail(
|
||||
"--shard-index/--shard-count are only valid with --mode serialized or --mode general --group general-server.",
|
||||
);
|
||||
}
|
||||
|
||||
if (group !== null && mode !== generalModeName) {
|
||||
@@ -210,17 +216,17 @@ function parseCliOptions(argv) {
|
||||
fail(`Unknown group "${group}". Expected one of: ${generalGroupNames.join(", ")}.`);
|
||||
}
|
||||
|
||||
if (mode === serializedModeName) {
|
||||
const resolvedShardCount = shardCount ?? 1;
|
||||
const resolvedShardIndex = shardIndex ?? 0;
|
||||
if (resolvedShardIndex >= resolvedShardCount) {
|
||||
fail(`--shard-index must be less than --shard-count. Received ${resolvedShardIndex} of ${resolvedShardCount}.`);
|
||||
if (shardIndex !== null) {
|
||||
if (shardIndex >= shardCount) {
|
||||
fail(`--shard-index must be less than --shard-count. Received ${shardIndex} of ${shardCount}.`);
|
||||
}
|
||||
}
|
||||
|
||||
if (mode === serializedModeName) {
|
||||
return {
|
||||
mode,
|
||||
shardIndex: resolvedShardIndex,
|
||||
shardCount: resolvedShardCount,
|
||||
shardIndex: shardIndex ?? 0,
|
||||
shardCount: shardCount ?? 1,
|
||||
group: null,
|
||||
dryRun,
|
||||
};
|
||||
@@ -228,8 +234,8 @@ function parseCliOptions(argv) {
|
||||
|
||||
return {
|
||||
mode,
|
||||
shardIndex: null,
|
||||
shardCount: null,
|
||||
shardIndex,
|
||||
shardCount,
|
||||
group,
|
||||
dryRun,
|
||||
};
|
||||
@@ -280,8 +286,31 @@ function runProjectGroup(projects, groupName) {
|
||||
}
|
||||
}
|
||||
|
||||
function runGeneralGroup(routeTests, groupName) {
|
||||
function runGeneralGroup(routeTests, groupName, shardIndex = null, shardCount = null) {
|
||||
if (groupName === generalServerGroupName) {
|
||||
if (shardCount !== null && shardCount > 1) {
|
||||
const shardFiles = generalServerTestFiles.filter(
|
||||
(_, index) => index % shardCount === shardIndex,
|
||||
);
|
||||
console.log(
|
||||
`\n[test:run] general-server shard ${shardIndex + 1}/${shardCount} running ${shardFiles.length} of ${generalServerTestFiles.length} suites`,
|
||||
);
|
||||
if (shardFiles.length === 0) {
|
||||
return;
|
||||
}
|
||||
|
||||
runVitest(
|
||||
[
|
||||
"--project",
|
||||
"@paperclipai/server",
|
||||
...serializedServerVitestArgs,
|
||||
...shardFiles,
|
||||
],
|
||||
`${groupName} shard ${shardIndex + 1}/${shardCount}`,
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const excludeRouteArgs = routeTests.flatMap((file) => ["--exclude", file.serverPath]);
|
||||
runVitest(
|
||||
[
|
||||
@@ -336,6 +365,17 @@ const routeTests = walk(serverTestsDir)
|
||||
}))
|
||||
.sort((a, b) => a.repoPath.localeCompare(b.repoPath));
|
||||
|
||||
// Every server test file that the general-server group is responsible for,
|
||||
// i.e. the whole server project minus the route/authz suites that run in the
|
||||
// dedicated serialized shards. Sharding this list across runners is what keeps
|
||||
// the general-server lane from becoming the PR critical path: the server vitest
|
||||
// config pins maxWorkers to 1, so the only way to parallelize is across jobs.
|
||||
const generalServerTestFiles = walk(serverSrcDir)
|
||||
.map((file) => toRepoPath(file))
|
||||
.filter((repoPath) => repoPath.endsWith(".test.ts"))
|
||||
.filter((repoPath) => !isRouteOrAuthzTest(repoPath))
|
||||
.sort((a, b) => a.localeCompare(b));
|
||||
|
||||
const options = parseCliOptions(process.argv.slice(2));
|
||||
if (options.dryRun) {
|
||||
const serializedSuites =
|
||||
@@ -352,6 +392,15 @@ if (options.dryRun) {
|
||||
availableGeneralGroups: generalGroupNames,
|
||||
serializedSuiteCount: routeTests.length,
|
||||
selectedSerializedSuites: serializedSuites.map((routeTest) => routeTest.repoPath),
|
||||
generalServerSuiteCount: generalServerTestFiles.length,
|
||||
selectedGeneralServerSuites:
|
||||
options.mode === generalModeName &&
|
||||
options.group === generalServerGroupName &&
|
||||
options.shardCount !== null
|
||||
? generalServerTestFiles.filter(
|
||||
(_, index) => index % options.shardCount === options.shardIndex,
|
||||
)
|
||||
: null,
|
||||
},
|
||||
null,
|
||||
2,
|
||||
@@ -362,7 +411,7 @@ if (options.dryRun) {
|
||||
|
||||
if (options.mode === generalModeName || options.mode === allModeName) {
|
||||
if (options.group) {
|
||||
runGeneralGroup(routeTests, options.group);
|
||||
runGeneralGroup(routeTests, options.group, options.shardIndex, options.shardCount);
|
||||
} else {
|
||||
runGeneralSuites(routeTests);
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user