From a9947fba32f1c57a11eda7eaf8d9a49392c60392 Mon Sep 17 00:00:00 2001 From: Omar Ramadan Date: Thu, 6 Aug 2026 22:06:03 -0700 Subject: [PATCH] ci: fold serialized suites into server shards (BLO-22428) Co-Authored-By: Paperclip --- .github/workflows/pr.yml | 78 +++---------------- .../__tests__/pr-ci-shard-folding.test.mjs | 54 +++++++++++++ .../__tests__/pr-verify-lane-outcome.test.mjs | 26 ------- 3 files changed, 66 insertions(+), 92 deletions(-) create mode 100644 scripts/__tests__/pr-ci-shard-folding.test.mjs diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 1e1fd5b5b937..d4abf1611049 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -130,6 +130,9 @@ jobs: - name: Test verify job cancelled/skipped-vs-failed lane outcome run: node --test ./scripts/__tests__/pr-verify-lane-outcome.test.mjs + - name: Test PR server-shard folding + run: node --test ./scripts/__tests__/pr-ci-shard-folding.test.mjs + - name: Validate release package manifest run: node ./scripts/release-package-map.mjs check @@ -456,13 +459,17 @@ jobs: fi pnpm test:run:general -- "${args[@]}" + # Reuse the checkout, dependency install, cache restore, and embedded + # Postgres setup already paid for by each server shard. This removes four + # ARC pod cold starts without reducing serialized-suite coverage. + - name: Run serialized server test shard + if: matrix.group == 'general-server' + run: pnpm test:run:serialized -- --shard-index ${{ matrix.shard_index }} --shard-count ${{ matrix.shard_count }} + verify: # Preserve the legacy required-check name while the underlying work runs in parallel. - # BLO-20869: verify_serialized_server is folded in here (rather than made - # required on its own) so a cancelled/skipped/never-scheduled serialized - # shard fails this required check instead of being invisible to the merge - # gate. See the issue for the measured latency tradeoff of putting a - # ~90-minute job on the required path this way. + # Serialized server suites run inside the four general-server matrix jobs, + # so general_tests remains the required aggregate for both test families. name: verify if: ${{ always() }} needs: @@ -473,7 +480,6 @@ jobs: worktree_install, opencode_responses_replay, build, - verify_serialized_server, ] runs-on: arc-light timeout-minutes: 5 @@ -507,7 +513,6 @@ jobs: WORKTREE_INSTALL_RESULT: ${{ needs.worktree_install.result }} OPENCODE_RESPONSES_REPLAY_RESULT: ${{ needs.opencode_responses_replay.result }} BUILD_RESULT: ${{ needs.build.result }} - VERIFY_SERIALIZED_SERVER_RESULT: ${{ needs.verify_serialized_server.result }} run: | lane_names=( helm_chart @@ -516,7 +521,6 @@ jobs: worktree_install opencode_responses_replay build - verify_serialized_server ) lane_results=( "$HELM_CHART_RESULT" @@ -525,7 +529,6 @@ jobs: "$WORKTREE_INSTALL_RESULT" "$OPENCODE_RESPONSES_REPLAY_RESULT" "$BUILD_RESULT" - "$VERIFY_SERIALIZED_SERVER_RESULT" ) cancelled_lanes=() @@ -602,63 +605,6 @@ jobs: - name: Build run: pnpm build - verify_serialized_server: - name: Verify serialized server suites (${{ matrix.shard_label }}) - needs: [policy, general_tests] - # BLO-22428: route merge_group traffic to the dedicated arc-merge-queue - # pool so pull_request and merge_group traffic cannot starve each other. - # See typecheck_release_registry above. - runs-on: ${{ github.event_name == 'merge_group' && 'arc-merge-queue' || 'arc-paperclip-general' }} - timeout-minutes: 90 - strategy: - fail-fast: false - max-parallel: 4 - matrix: - include: - - shard_index: 0 - shard_count: 4 - shard_label: 1/4 - - shard_index: 1 - shard_count: 4 - shard_label: 2/4 - - shard_index: 2 - shard_count: 4 - shard_label: 3/4 - - shard_index: 3 - shard_count: 4 - shard_label: 4/4 - - steps: - - name: Checkout repository - uses: actions/checkout@v6 - - - name: Setup pnpm - uses: pnpm/action-setup@v6 - with: - version: 9.15.4 - - - name: Restore regenerated PR lockfile (if policy uploaded one) - uses: actions/download-artifact@v4 - continue-on-error: true - with: - name: pr-lockfile - path: . - - - name: Setup Node.js - uses: actions/setup-node@v6 - with: - node-version: 24 - cache: pnpm - - - name: Set up GHA-backed sccache - uses: mozilla-actions/sccache-action@v0.0.9 - - - name: Install dependencies - run: pnpm install --frozen-lockfile - - - name: Run serialized server test shard - run: pnpm test:run:serialized -- --shard-index ${{ matrix.shard_index }} --shard-count ${{ matrix.shard_count }} - canary_dry_run: name: Canary Dry Run needs: [policy] diff --git a/scripts/__tests__/pr-ci-shard-folding.test.mjs b/scripts/__tests__/pr-ci-shard-folding.test.mjs new file mode 100644 index 000000000000..021f267ca051 --- /dev/null +++ b/scripts/__tests__/pr-ci-shard-folding.test.mjs @@ -0,0 +1,54 @@ +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { test } from "node:test"; + +const workflow = readFileSync(new URL("../../.github/workflows/pr.yml", import.meta.url), "utf8"); + +function jobBlock(name, nextName) { + const start = workflow.indexOf(`\n ${name}:\n`); + assert.notEqual(start, -1, `pr.yml must define ${name}`); + const end = workflow.indexOf(`\n ${nextName}:\n`, start + 1); + assert.notEqual(end, -1, `pr.yml must define ${nextName} after ${name}`); + return workflow.slice(start, end); +} + +test("server shards run general and serialized suites in the same four jobs", () => { + const general = jobBlock("general_tests", "verify"); + const serverEntries = general.match( + / - group: general-server\n(?: [^\n]*\n)*/g, + ) ?? []; + assert.equal(serverEntries.length, 4, "general_tests must retain four isolated server shards"); + for (const [index, entry] of serverEntries.entries()) { + assert.match( + entry, + new RegExp(`group_label: server ${index + 1}/4`), + `server shard ${index} must retain its matching label`, + ); + assert.match(entry, new RegExp(`shard_index: ${index}`)); + assert.match(entry, /shard_count: 4/); + } + assert.match(general, /pnpm test:run:general -- "\$\{args\[@\]\}"/); + assert.match( + general, + /- name: Run serialized server test shard\n if: matrix\.group == 'general-server'\n run: pnpm test:run:serialized -- --shard-index \$\{\{ matrix\.shard_index \}\} --shard-count \$\{\{ matrix\.shard_count \}\}/, + "serialized suites must run only in the corresponding server shard", + ); + assert.ok( + general.indexOf("pnpm test:run:general") < general.indexOf("pnpm test:run:serialized"), + "each server shard must run general suites before serialized suites", + ); +}); + +test("serialized coverage is aggregated through general_tests without a second job matrix", () => { + assert.doesNotMatch(workflow, /\n verify_serialized_server:\n/); + assert.equal( + workflow.match(/pnpm test:run:serialized -- --shard-index/g)?.length, + 1, + "the PR workflow should declare one matrix-driven serialized command", + ); + + const verify = jobBlock("verify", "build"); + assert.match(verify, /\n general_tests,/); + assert.doesNotMatch(verify, /verify_serialized_server/); + assert.match(verify, /GENERAL_TESTS_RESULT: \$\{\{ needs\.general_tests\.result \}\}/); +}); diff --git a/scripts/__tests__/pr-verify-lane-outcome.test.mjs b/scripts/__tests__/pr-verify-lane-outcome.test.mjs index a10d642ab975..55b49cc8538c 100644 --- a/scripts/__tests__/pr-verify-lane-outcome.test.mjs +++ b/scripts/__tests__/pr-verify-lane-outcome.test.mjs @@ -55,7 +55,6 @@ function runVerifyStep(results) { WORKTREE_INSTALL_RESULT: results.worktree_install ?? "success", OPENCODE_RESPONSES_REPLAY_RESULT: results.opencode_responses_replay ?? "success", BUILD_RESULT: results.build ?? "success", - VERIFY_SERIALIZED_SERVER_RESULT: results.verify_serialized_server ?? "success", }; return spawnSync("bash", ["-c", script], { env, encoding: "utf8" }); } @@ -122,31 +121,6 @@ test("verify step annotates both a real failure and a cancellation when a run ha assert.match(result.stdout, /general_tests/); }); -// BLO-20869: verify_serialized_server must be treated exactly like the other -// required lanes -- cancelled, skipped, or never-scheduled (which also reads -// as "skipped" via `needs`) must all fail this required check instead of -// being invisible to the merge gate. -test("verify step fails when the serialized server suite is cancelled", () => { - const result = runVerifyStep({ verify_serialized_server: "cancelled" }); - assert.notEqual(result.status, 0); - assert.match(result.stdout, /::error title=verify: lane cancelled::/); - assert.match(result.stdout, /verify_serialized_server/); -}); - -test("verify step fails when the serialized server suite is skipped", () => { - const result = runVerifyStep({ verify_serialized_server: "skipped" }); - assert.notEqual(result.status, 0); - assert.match(result.stdout, /::error title=verify: lane skipped::/); - assert.match(result.stdout, /verify_serialized_server/); -}); - -test("verify step fails when the serialized server suite genuinely fails", () => { - const result = runVerifyStep({ verify_serialized_server: "failure" }); - assert.notEqual(result.status, 0); - assert.match(result.stdout, /::error title=verify: lane failure::/); - assert.match(result.stdout, /verify_serialized_server/); -}); - test("verify step annotates a skipped lane as an unmet dependency, not a failure", () => { const result = runVerifyStep({ worktree_install: "skipped" }); assert.notEqual(result.status, 0);