mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
## Thinking Path > - Paperclip uses CI to keep control-plane changes safe and mergeable. > - The PR workflow splits serialized server tests across isolated runners. > - A recent successful run spent 305 seconds in serialized shard 2/4. > - That job was the slowest check in the run. > - The four shards reported about 739 seconds of Vitest suite time. > - This pull request adds a fifth serialized shard and keeps release verification aligned. > - The benefit is a shorter PR critical path with no loss of test coverage. ## Linked Issues or Issue Description **What existing behavior does this improve?** The PR and release verification workflows run serialized server tests in four shards. **Current behavior** Successful PR run 30876682788 spent 305 seconds in `Verify serialized server suites (2/4)`. The test step used 256 seconds and made this job the slowest check. **Proposed behavior** Run the same serialized suite set in five complete and non-overlapping shards. **Reason and benefit** The measured suites reported about 739 seconds of total Vitest time. Five runners reduce the expected average suite time from about 185 seconds to about 148 seconds before setup overhead. **Breaking changes** None. The change only alters CI partition size. ## What Changed - Split serialized server tests into five shards in the PR workflow. - Apply the same five-shard layout to release verification. - Add a partition test that proves complete and non-overlapping serialized coverage. - Update release workflow coverage tests for five shards. ## Verification - `node --test scripts/__tests__/run-vitest-stable-shard.test.mjs scripts/__tests__/release-verify-workflow.test.mjs` - `git diff --check` ## Risks - Low risk. CI uses one additional runner for the serialized lane. - Round-robin partition weights can still vary as suite timings change. > This change does not overlap with planned core work in `ROADMAP.md`. Related PR #10663 optimized the separate general-server lane. ## Model Used - OpenAI Codex, GPT-5, agentic coding with reasoning, tool use, and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [ ] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Devin Foley <139239+devinfoley@users.noreply.github.com>
160 lines
7.0 KiB
JavaScript
160 lines
7.0 KiB
JavaScript
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";
|
|
|
|
import {
|
|
defaultSuiteWeight,
|
|
loadShardDurations,
|
|
partitionGeneralServerSuites,
|
|
} from "../general-server-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");
|
|
|
|
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 = 4;
|
|
const SERIALIZED_SHARD_COUNT = 5;
|
|
|
|
test("the serialized shards form a complete, non-overlapping partition", () => {
|
|
const shards = Array.from({ length: SERIALIZED_SHARD_COUNT }, (_, index) =>
|
|
dryRunJson(["--mode", "serialized", "--shard-index", String(index), "--shard-count", String(SERIALIZED_SHARD_COUNT)]),
|
|
);
|
|
|
|
const total = shards[0].serializedSuiteCount;
|
|
const selected = shards.flatMap((shard) => shard.selectedSerializedSuites);
|
|
assert.equal(selected.length, total, "every serialized suite must be selected exactly once");
|
|
assert.equal(new Set(selected).size, total, "serialized shards must not overlap");
|
|
});
|
|
|
|
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");
|
|
});
|
|
|
|
test("duration-aware partition balances skewed weights better than round-robin", () => {
|
|
// Round-robin puts all three heavy suites on shard 0 (indexes 0, 3, 6).
|
|
const files = ["a", "b", "c", "d", "e", "f", "g", "h", "i"];
|
|
const durations = { a: 30000, d: 30000, g: 30000, b: 100, c: 100, e: 100, f: 100, h: 100, i: 100 };
|
|
|
|
const shards = partitionGeneralServerSuites(files, 3, durations);
|
|
const totals = shards.map((shard) => shard.totalWeight);
|
|
const maxTotal = Math.max(...totals);
|
|
const minTotal = Math.min(...totals);
|
|
assert.ok(
|
|
maxTotal - minTotal <= 200,
|
|
`expected near-even shard weights, got ${totals.join(", ")}`,
|
|
);
|
|
assert.equal(
|
|
shards.flatMap((shard) => shard.files).sort().join(","),
|
|
files.join(","),
|
|
"partition must cover every file exactly once",
|
|
);
|
|
});
|
|
|
|
test("the partition is deterministic for identical inputs", () => {
|
|
const files = Array.from({ length: 50 }, (_, index) => `suite-${index}.test.ts`);
|
|
const durations = Object.fromEntries(files.map((file, index) => [file, (index * 37) % 5000]));
|
|
|
|
const first = partitionGeneralServerSuites(files, 3, durations);
|
|
const second = partitionGeneralServerSuites(files, 3, durations);
|
|
assert.deepEqual(first, second, "same inputs must always produce the same partition");
|
|
});
|
|
|
|
test("suites missing from the manifest get the median weight", () => {
|
|
assert.equal(defaultSuiteWeight({ a: 100, b: 300, c: 900 }), 300);
|
|
assert.equal(defaultSuiteWeight({ a: 100, b: 300, c: 500, d: 900 }), 400);
|
|
assert.equal(defaultSuiteWeight({}), 1000, "empty manifest falls back to a fixed weight");
|
|
});
|
|
|
|
test("a missing or malformed manifest degrades to uniform weights", () => {
|
|
assert.deepEqual(loadShardDurations(path.join(repoRoot, "scripts", "no-such-manifest.json")), {});
|
|
|
|
const files = ["a", "b", "c", "d"];
|
|
const shards = partitionGeneralServerSuites(files, 2, {});
|
|
assert.equal(shards[0].files.length + shards[1].files.length, files.length);
|
|
assert.equal(Math.abs(shards[0].files.length - shards[1].files.length), 0);
|
|
});
|
|
|
|
test("the checked-in manifest loads and covers most of the current suite set", () => {
|
|
const durations = loadShardDurations(durationsManifest);
|
|
assert.ok(Object.keys(durations).length > 0, "manifest must parse to a non-empty duration map");
|
|
|
|
const shard = dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", "0", "--shard-count", "1"]);
|
|
const currentFiles = shard.selectedGeneralServerSuites;
|
|
const known = currentFiles.filter((file) => durations[file] !== undefined).length;
|
|
assert.ok(
|
|
known / currentFiles.length >= 0.5,
|
|
`manifest is stale: only ${known} of ${currentFiles.length} suites have recorded durations — regenerate it from a recent PR run (see the manifest's $comment)`,
|
|
);
|
|
});
|
|
|
|
test("the real shard partition is duration-balanced", () => {
|
|
const durations = loadShardDurations(durationsManifest);
|
|
const fallback = defaultSuiteWeight(durations);
|
|
const shards = Array.from({ length: SHARD_COUNT }, (_, index) =>
|
|
dryRunJson(["--mode", "general", "--group", "general-server", "--shard-index", String(index), "--shard-count", String(SHARD_COUNT)]),
|
|
);
|
|
|
|
const totals = shards.map((shard) =>
|
|
shard.selectedGeneralServerSuites.reduce((sum, file) => sum + (durations[file] ?? fallback), 0),
|
|
);
|
|
const maxTotal = Math.max(...totals);
|
|
const minTotal = Math.min(...totals);
|
|
// LPT keeps the spread within the heaviest single suite; use that as the bound.
|
|
const heaviest = Math.max(...Object.values(durations));
|
|
assert.ok(
|
|
maxTotal - minTotal <= heaviest,
|
|
`shard weight spread ${maxTotal - minTotal}ms exceeds heaviest suite ${heaviest}ms: ${totals.join(", ")}`,
|
|
);
|
|
});
|