diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index abfe65bd10..6971a5f789 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -58,6 +58,9 @@ jobs: - name: Test general-server shard partition run: node --test ./scripts/__tests__/run-vitest-stable-shard.test.mjs + - name: Test e2e shard partition + run: node --test ./scripts/__tests__/e2e-shard.test.mjs + - name: Test release verify workflow wiring run: node --test ./scripts/__tests__/release-verify-workflow.test.mjs @@ -356,10 +359,25 @@ jobs: fi ./scripts/release.sh canary --skip-verify --dry-run - e2e: + e2e_shards: + name: e2e shard (${{ matrix.shard_label }}) needs: [policy] runs-on: ubuntu-latest timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + include: + # The Playwright lane is pinned to workers=1 (tests/e2e/playwright.config.ts) + # because every spec shares one throwaway server and some toggle + # instance-level flags, so it can only be parallelized across runners. + # Each shard boots its own server, which keeps that isolation intact. + - shard_index: 0 + shard_count: 2 + shard_label: 1/2 + - shard_index: 1 + shard_count: 2 + shard_label: 2/2 steps: - name: Checkout repository @@ -411,14 +429,36 @@ jobs: env: PAPERCLIP_E2E_SKIP_LLM: "true" PAPERCLIP_PLAYWRIGHT_CHANNEL: "chrome" - run: pnpm run test:e2e + run: | + # Playwright's own --shard balances by test count, and one spec + # (smoke-lab) is ~40% of the lane's wall clock. Partition by recorded + # spec duration instead so both runners finish together. + specs="$(node ./scripts/e2e-shard.mjs \ + --shard-index ${{ matrix.shard_index }} --shard-count ${{ matrix.shard_count }})" + echo "shard ${{ matrix.shard_label }} specs: $specs" + pnpm run test:e2e -- $specs - name: Upload Playwright report uses: actions/upload-artifact@v7 if: always() with: - name: playwright-report + name: playwright-report-${{ matrix.shard_index }} path: | tests/e2e/playwright-report/ tests/e2e/test-results/ retention-days: 14 + + e2e: + # Preserve the legacy required-check name while the specs run sharded + # across the matrix above (same pattern as the `verify` aggregate). + name: e2e + if: ${{ always() }} + needs: [e2e_shards] + runs-on: ubuntu-latest + timeout-minutes: 5 + + steps: + - name: Fail if any e2e shard failed + env: + E2E_SHARDS_RESULT: ${{ needs.e2e_shards.result }} + run: test "$E2E_SHARDS_RESULT" = "success" diff --git a/scripts/__tests__/e2e-shard.test.mjs b/scripts/__tests__/e2e-shard.test.mjs new file mode 100644 index 0000000000..5967598316 --- /dev/null +++ b/scripts/__tests__/e2e-shard.test.mjs @@ -0,0 +1,120 @@ +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 = 2; + +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. 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. + assert.ok( + heaviest <= (total / SHARD_COUNT) * 1.15, + `heaviest shard ${heaviest}ms exceeds 115% of the even cut (${total / SHARD_COUNT}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/2)`, 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"); + assert.match(shards, /shard_count: 2/, "the shard matrix must match SHARD_COUNT"); +}); diff --git a/scripts/e2e-shard-durations.json b/scripts/e2e-shard-durations.json new file mode 100644 index 0000000000..b23c25efdb --- /dev/null +++ b/scripts/e2e-shard-durations.json @@ -0,0 +1,20 @@ +{ + "$comment": "Per-spec wall-clock durations (ms) for the Playwright e2e lane, used by scripts/e2e-shard.mjs to balance specs across the PR shard matrix. Averaged from two real PR runs of .github/workflows/pr.yml (actions runs 29775184389 and 29769465682, 2026-07-20). Specs missing here get the median weight, so the manifest only needs occasional refreshes: pull the 'Run e2e tests' logs from a recent PR run and sum the Playwright list-reporter durations per spec file.", + "unit": "ms", + "durations": { + "tests/e2e/app-not-connected.spec.ts": 26750, + "tests/e2e/application-delete-screenshot.spec.ts": 6750, + "tests/e2e/applications-crud.spec.ts": 16100, + "tests/e2e/apps-dark-mode-shots.spec.ts": 18000, + "tests/e2e/apps-prosumer-mcp-flow.spec.ts": 16800, + "tests/e2e/conference-room-typing-intro.spec.ts": 4900, + "tests/e2e/mcp-user-stories.spec.ts": 39000, + "tests/e2e/nux-phase4-screenshots.spec.ts": 19650, + "tests/e2e/onboarding.spec.ts": 3200, + "tests/e2e/pipelines-tutorial-flow.spec.ts": 30198, + "tests/e2e/planning-mode-visual-verification.spec.ts": 23250, + "tests/e2e/sidebar-takeover.spec.ts": 16650, + "tests/e2e/signoff-policy.spec.ts": 9696, + "tests/e2e/smoke-lab.spec.ts": 168000 + } +} diff --git a/scripts/e2e-shard.mjs b/scripts/e2e-shard.mjs new file mode 100644 index 0000000000..34c4ce5ee8 --- /dev/null +++ b/scripts/e2e-shard.mjs @@ -0,0 +1,59 @@ +import { readdirSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +import { loadShardDurations, partitionGeneralServerSuites } from "./general-server-shard.mjs"; + +const HERE = path.dirname(fileURLToPath(import.meta.url)); +const REPO_ROOT = path.resolve(HERE, ".."); +const E2E_DIR = path.join(REPO_ROOT, "tests", "e2e"); +const DURATIONS_MANIFEST = path.join(HERE, "e2e-shard-durations.json"); + +// Specs the default local_trusted Playwright project deliberately skips. Keep +// this in sync with `testIgnore` in tests/e2e/playwright.config.ts — the unit +// test in scripts/__tests__/e2e-shard.test.mjs fails if the two ever drift. +export const IGNORED_SPECS = ["multi-user.spec.ts", "multi-user-authenticated.spec.ts"]; + +// Enumerates the specs the default e2e lane actually runs, as repo-relative +// paths so the output can be handed straight to `playwright test`. +export function listE2eSpecs(e2eDir = E2E_DIR, repoRoot = REPO_ROOT) { + return readdirSync(e2eDir) + .filter((entry) => entry.endsWith(".spec.ts") && !IGNORED_SPECS.includes(entry)) + .map((entry) => path.relative(repoRoot, path.join(e2eDir, entry)).split(path.sep).join("/")) + .sort((a, b) => a.localeCompare(b)); +} + +// Playwright's own --shard balances by test count, which is useless here: one +// spec (smoke-lab) is ~40% of the lane's wall clock. Reuse the deterministic +// longest-processing-time partition already proven on the general-server lane +// so every runner computes the identical, non-overlapping split. +export function selectE2eShard(files, shardIndex, shardCount, durations = {}) { + return partitionGeneralServerSuites(files, shardCount, durations)[shardIndex].files; +} + +function parseArgs(argv) { + const args = { shardIndex: 0, shardCount: 1 }; + for (let index = 0; index < argv.length; index += 1) { + if (argv[index] === "--shard-index") args.shardIndex = Number(argv[index + 1]); + if (argv[index] === "--shard-count") args.shardCount = Number(argv[index + 1]); + } + return args; +} + +function main(argv) { + const { shardIndex, shardCount } = parseArgs(argv); + if (!Number.isInteger(shardCount) || shardCount < 1) { + throw new Error(`--shard-count must be a positive integer, got ${shardCount}`); + } + if (!Number.isInteger(shardIndex) || shardIndex < 0 || shardIndex >= shardCount) { + throw new Error(`--shard-index must be in [0, ${shardCount}), got ${shardIndex}`); + } + + const specs = listE2eSpecs(); + const durations = loadShardDurations(DURATIONS_MANIFEST); + process.stdout.write(`${selectE2eShard(specs, shardIndex, shardCount, durations).join(" ")}\n`); +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + main(process.argv.slice(2)); +}