Files
PaperClipAI/scripts/__tests__/e2e-shard.test.mjs
T
Devin FoleyandCody 8676795188 ci: split e2e PR lane into three shards (#10629)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - The pull request workflow protects changes with a Playwright e2e
lane.
> - That lane already uses a weighted file partition so slow specs do
not cluster by test count.
> - Recent green PR runs showed the two e2e shard jobs were slower than
the next slow required lane.
> - The largest spec is indivisible, so a third shard lets that spec run
alone and lets the rest split by duration.
> - This pull request changes only the PR e2e shard matrix and the guard
test.
> - The benefit is a shorter expected PR critical path while the
required `e2e` aggregate check name stays stable.

## Linked Issues or Issue Description

Refs #9923

**What existing behavior does this improve?**

The `pull_request` workflow Playwright e2e lane.

**Subsystem affected**

Cross-cutting: GitHub Actions CI and test scripts.

**Current behavior**

The PR workflow runs the weighted Playwright e2e partition across two
jobs. Recent green runs showed those jobs as the slowest required
checks.

**Proposed behavior**

The PR workflow runs the same e2e spec set across three weighted jobs.
The aggregate required check stays named `e2e`.

**Reason and benefit**

The third shard lets the slow smoke-lab spec run alone while the rest of
the catalog stays balanced. This should shorten the PR critical path.
The win is bounded by fixed per-job setup time.

**Breaking changes**

None. The required aggregate check contract is preserved.

## What Changed

- Change the PR e2e shard matrix from two entries to three entries.
- Update the shard guard test to expect three shards.
- Floor the balance bound at the largest single spec weight.
- Assert that the workflow does not define more shard indexes than
`SHARD_COUNT`.

## Verification

- `node --test ./scripts/__tests__/e2e-shard.test.mjs` passes with 6
tests.
- The recorded-weight partition is complete and non-overlapping: 168.0s,
116.5s, and 114.4s.
- I checked `ROADMAP.md` and found no overlapping roadmap-level core
feature.
- I searched public GitHub PRs and issues for related e2e shard work. I
found related PR #9923 and no open duplicate for this branch or change.

## Risks

- This adds one extra GitHub Actions runner to the PR e2e lane.
- The wall-clock win is bounded by fixed per-job setup.
- Behavior risk is low because the aggregate required check remains
named `e2e`.

## Model Used

OpenAI Codex, GPT-5, tool-enabled coding agent in this repository. The
runtime did not expose the context-window size.

## 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)
- [x] 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: Cody <noreply@paperclip.ing>
2026-08-01 10:10:22 -07:00

145 lines
6.5 KiB
JavaScript

import assert from "node:assert/strict";
import { spawnSync } from "node:child_process";
import { readFileSync } from "node:fs";
import path from "node:path";
import { fileURLToPath } from "node:url";
import test from "node:test";
import { loadShardDurations } from "../general-server-shard.mjs";
import { IGNORED_SPECS, listE2eSpecs, selectE2eShard } from "../e2e-shard.mjs";
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", "..");
const script = path.join(repoRoot, "scripts", "e2e-shard.mjs");
const durationsManifest = path.join(repoRoot, "scripts", "e2e-shard-durations.json");
const playwrightConfig = path.join(repoRoot, "tests", "e2e", "playwright.config.ts");
const prWorkflow = path.join(repoRoot, ".github", "workflows", "pr.yml");
const SHARD_COUNT = 3;
function runShard(args) {
const result = spawnSync(process.execPath, [script, ...args], { cwd: repoRoot, encoding: "utf8" });
assert.equal(result.status, 0, `expected success for ${args.join(" ")}: ${result.stderr}`);
return result.stdout.trim().split(/\s+/).filter(Boolean);
}
test("the e2e shards form a complete, non-overlapping partition", () => {
const specs = listE2eSpecs();
assert.ok(specs.length > 0, "expected a non-empty e2e spec set");
const shards = Array.from({ length: SHARD_COUNT }, (_, index) =>
runShard(["--shard-index", String(index), "--shard-count", String(SHARD_COUNT)]),
);
const combined = shards.flat();
assert.equal(combined.length, specs.length, "every spec must land on exactly one shard");
assert.deepEqual([...combined].sort(), [...specs].sort());
for (const shard of shards) {
assert.ok(shard.length > 0, "no shard may be empty — Playwright fails a run with no matching specs");
}
});
test("the ignored spec list matches playwright.config.ts testIgnore", () => {
const config = readFileSync(playwrightConfig, "utf8");
const match = config.match(/testIgnore:\s*\[([^\]]*)\]/);
assert.ok(match, "expected a testIgnore array in playwright.config.ts");
const configured = [...match[1].matchAll(/"([^"]+)"/g)].map((entry) => entry[1]);
assert.deepEqual([...configured].sort(), [...IGNORED_SPECS].sort());
});
test("the duration manifest only names specs that still exist", () => {
const durations = loadShardDurations(durationsManifest);
assert.ok(Object.keys(durations).length > 0, "expected a populated duration manifest");
const specs = new Set(listE2eSpecs());
for (const file of Object.keys(durations)) {
assert.ok(specs.has(file), `duration manifest names a spec that no longer runs: ${file}`);
}
});
test("the weighted partition keeps the shards close to balanced", () => {
const durations = loadShardDurations(durationsManifest);
const specs = listE2eSpecs();
const weights = Array.from({ length: SHARD_COUNT }, (_, index) =>
selectE2eShard(specs, index, SHARD_COUNT, durations).reduce((sum, file) => sum + (durations[file] ?? 0), 0),
);
const heaviest = Math.max(...weights);
const total = weights.reduce((sum, weight) => sum + weight, 0);
// Round-robin/count-based sharding would strand the ~168s smoke-lab spec on
// one runner alongside other specs. Assert the weighted split stays within
// 15% of an even cut so a future spec-time regression surfaces here instead
// of on the PR critical path. A single indivisible spec (smoke-lab) can
// legitimately exceed the even cut on its own, so the bound is floored at
// the largest per-spec weight — the best any file-level partition can do.
const largestSpec = Math.max(...specs.map((file) => durations[file] ?? 0));
const bound = Math.max((total / SHARD_COUNT) * 1.15, largestSpec);
assert.ok(
heaviest <= bound,
`heaviest shard ${heaviest}ms exceeds the balance bound (${bound}ms)`,
);
});
test("shard arguments are validated", () => {
for (const args of [
["--shard-index", "2", "--shard-count", "2"],
["--shard-index", "-1", "--shard-count", "2"],
["--shard-index", "0", "--shard-count", "0"],
]) {
const result = spawnSync(process.execPath, [script, ...args], { cwd: repoRoot, encoding: "utf8" });
assert.notEqual(result.status, 0, `expected failure for ${args.join(" ")}`);
}
});
test("pr.yml keeps a stable aggregate check named e2e over the shard matrix", () => {
// Branch protection requires a check literally named `e2e`. The shards run
// as `e2e shard (n/3)`, so the aggregate job below is what keeps the
// required-check contract intact — same pattern as the `verify` aggregate.
const workflow = readFileSync(prWorkflow, "utf8");
const jobs = new Map();
let current = null;
for (const line of workflow.split("\n")) {
const header = /^ {2}([A-Za-z0-9_-]+):\s*$/.exec(line);
if (header) {
current = header[1];
jobs.set(current, []);
continue;
}
if (current && /^\S/.test(line)) current = null;
if (current) jobs.get(current).push(line);
}
for (const [id, lines] of jobs) jobs.set(id, lines.join("\n"));
const aggregate = jobs.get("e2e");
assert.ok(aggregate, "pr.yml must define an `e2e` job to satisfy branch protection");
assert.match(aggregate, /^ {4}name: e2e$/m, "the aggregate job must be named exactly `e2e`");
assert.match(aggregate, /^ {4}if: \$\{\{ always\(\) \}\}$/m, "the aggregate must run even when a shard fails");
assert.match(aggregate, /^ {4}needs: \[e2e_shards\]$/m, "the aggregate must depend on the shard matrix");
assert.match(
aggregate,
/test "\$E2E_SHARDS_RESULT" = "success"/,
"the aggregate must fail unless every shard succeeded",
);
const shards = jobs.get("e2e_shards");
assert.ok(shards, "pr.yml must define the `e2e_shards` matrix job");
const matrixEntries = [
...shards.matchAll(
/^ {10}- shard_index: (?<shardIndex>\d+)\n {12}shard_count: (?<shardCount>\d+)\n {12}shard_label: (?<shardLabel>\d+\/\d+)$/gm,
),
].map((match) => ({
shardIndex: Number(match.groups.shardIndex),
shardCount: Number(match.groups.shardCount),
shardLabel: match.groups.shardLabel,
}));
assert.equal(matrixEntries.length, SHARD_COUNT, "the shard matrix must define exactly SHARD_COUNT entries");
assert.deepEqual(
matrixEntries.map((entry) => entry.shardIndex).sort((a, b) => a - b),
Array.from({ length: SHARD_COUNT }, (_, index) => index),
"the shard matrix must define each shard index exactly once",
);
for (const entry of matrixEntries) {
assert.equal(entry.shardCount, SHARD_COUNT, "each shard matrix entry must use the same SHARD_COUNT");
assert.equal(entry.shardLabel, `${entry.shardIndex + 1}/${SHARD_COUNT}`, "each shard label must match its index");
}
});