From e01d737f6af56e3fce5533471145f7a4aa291f93 Mon Sep 17 00:00:00 2001 From: TurtleWolfe Date: Sun, 23 Aug 2026 12:29:39 +0000 Subject: [PATCH] perf(ci): right-size local E2E browser coverage Run Chromium on the ordinary PR path while preserving full cross-browser coverage on push, dispatch, and full-e2e label events. Make aggregation mode-aware and report queue versus execution timing. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/e2e-local.yml | 283 +++++++--- CLAUDE.md | 3 +- .../__tests__/e2e-local-browser-gate.test.js | 491 ++++++++++++++++++ scripts/__tests__/e2e-local-triggers.test.js | 33 +- 4 files changed, 732 insertions(+), 78 deletions(-) create mode 100644 scripts/__tests__/e2e-local-browser-gate.test.js diff --git a/.github/workflows/e2e-local.yml b/.github/workflows/e2e-local.yml index adc72433..fe67d2c2 100644 --- a/.github/workflows/e2e-local.yml +++ b/.github/workflows/e2e-local.yml @@ -2,6 +2,10 @@ # which silently truncated this name to "E2E (local Supabase) —". name: 'E2E (local Supabase)' +permissions: + contents: read + actions: read + # THIS LANE IS REQUIRED, AND IT IS WHERE A PR GETS ITS E2E COVERAGE. # # `E2E (local) result` became a required context on `main` when #739 closed @@ -22,9 +26,11 @@ name: 'E2E (local Supabase)' # Still true, and not to be undone casually: e2e.yml is not deleted, and its # repo-wide mutex stays. Six `@hosted` tests can only pass there. # -# NOW THE FULL 24-JOB MATRIX (#575 step 1). It mirrors e2e.yml's three matrices -# exactly — same projects, same shard counts, same `workers: 2` on the msg-iso -# entries — because parity is only meaningful if the two run the SAME work. +# THE FULL 24-SHARD MATRIX STILL EXISTS (#575 step 1), but the common PR path is +# eight Chromium shards (#950). Firefox and WebKit run after merge on push to +# main, on workflow_dispatch, or when the PR event already carries `full-e2e`. +# Full mode mirrors e2e.yml exactly — same projects, same shard counts, same +# `workers: 2` on the msg-iso entries — so parity remains meaningful there. # # What it does NOT mirror is the contention machinery. e2e.yml splits into three # per-browser jobs each capped at `max-parallel: 2` for one reason: to hold peak @@ -76,6 +82,10 @@ on: # nothing to run. pull_request: branches: [main] + # `labeled` is what makes adding `full-e2e` launch the advertised 24-shard run. + # Other labels still take the normal Chromium path; never skip those events, because a + # vacuous green required context on the same SHA could hide an earlier real failure. + types: [opened, synchronize, reopened, labeled] push: branches: [main] @@ -96,7 +106,35 @@ jobs: timeout-minutes: 5 outputs: run: ${{ steps.decide.outputs.run }} + browsers: ${{ steps.browsers.outputs.browsers }} + expected_shards: ${{ steps.browsers.outputs.expected_shards }} + browser_mode: ${{ steps.browsers.outputs.browser_mode }} steps: + # Filter the matrix BEFORE runner allocation. A step-level browser gate would still + # enqueue 24 jobs and preserve the runner bottleneck #950 exists to remove. + - name: Select browser coverage + id: browsers + env: + E2E_FULL_BROWSERS: >- + ${{ github.event_name != 'pull_request' || + contains(github.event.pull_request.labels.*.name, 'full-e2e') }} + run: | + if [ "$E2E_FULL_BROWSERS" = "true" ]; then + browsers='["chromium","firefox","webkit"]' + expected_shards=24 + browser_mode=full + else + browsers='["chromium"]' + expected_shards=8 + browser_mode=chromium + fi + { + echo "browsers=$browsers" + echo "expected_shards=$expected_shards" + echo "browser_mode=$browser_mode" + } >> "$GITHUB_OUTPUT" + echo "browser mode: $browser_mode ($expected_shards shards)" + - uses: actions/checkout@v5 with: # Full history: the PR case diffs against the merge base, which a shallow @@ -130,7 +168,7 @@ jobs: node scripts/ci/e2e-local-changes.mjs "$BASE" "$HEAD" e2e-local: - name: ${{ matrix.project }} ${{ matrix.shard }} + name: ${{ matrix.browser }}-${{ matrix.slice.project }} ${{ matrix.slice.shard }} needs: changes if: needs.changes.outputs.run == 'true' runs-on: ubuntu-latest @@ -157,44 +195,16 @@ jobs: # cloud project that does not exist here. fail-fast: false matrix: - include: - # ===== CHROMIUM ===== - - { project: chromium-msg, browser: chromium, shard: 1/1 } - - { - project: chromium-msg-iso, - browser: chromium, - shard: 1/1, - workers: 2, - } - - { project: chromium-gen, browser: chromium, shard: 1/6 } - - { project: chromium-gen, browser: chromium, shard: 2/6 } - - { project: chromium-gen, browser: chromium, shard: 3/6 } - - { project: chromium-gen, browser: chromium, shard: 4/6 } - - { project: chromium-gen, browser: chromium, shard: 5/6 } - - { project: chromium-gen, browser: chromium, shard: 6/6 } - # ===== FIREFOX ===== - - { project: firefox-msg, browser: firefox, shard: 1/1 } - - { - project: firefox-msg-iso, - browser: firefox, - shard: 1/1, - workers: 2, - } - - { project: firefox-gen, browser: firefox, shard: 1/6 } - - { project: firefox-gen, browser: firefox, shard: 2/6 } - - { project: firefox-gen, browser: firefox, shard: 3/6 } - - { project: firefox-gen, browser: firefox, shard: 4/6 } - - { project: firefox-gen, browser: firefox, shard: 5/6 } - - { project: firefox-gen, browser: firefox, shard: 6/6 } - # ===== WEBKIT ===== - - { project: webkit-msg, browser: webkit, shard: 1/1 } - - { project: webkit-msg-iso, browser: webkit, shard: 1/1, workers: 2 } - - { project: webkit-gen, browser: webkit, shard: 1/6 } - - { project: webkit-gen, browser: webkit, shard: 2/6 } - - { project: webkit-gen, browser: webkit, shard: 3/6 } - - { project: webkit-gen, browser: webkit, shard: 4/6 } - - { project: webkit-gen, browser: webkit, shard: 5/6 } - - { project: webkit-gen, browser: webkit, shard: 6/6 } + browser: ${{ fromJSON(needs.changes.outputs.browsers) }} + slice: + - { project: msg, shard: 1/1 } + - { project: msg-iso, shard: 1/1, workers: 2 } + - { project: gen, shard: 1/6 } + - { project: gen, shard: 2/6 } + - { project: gen, shard: 3/6 } + - { project: gen, shard: 4/6 } + - { project: gen, shard: 5/6 } + - { project: gen, shard: 6/6 } steps: - name: Checkout the code under test @@ -374,7 +384,7 @@ jobs: # # This is also what CLAUDE.md asks for -- 'Docker-first development is # MANDATORY' -- and CI was the one place that ignored it. - - name: Run ${{ matrix.project }} ${{ matrix.shard }} + - name: Run ${{ matrix.browser }}-${{ matrix.slice.project }} ${{ matrix.slice.shard }} run: | npx serve out -l 3000 & npx wait-on http://localhost:3000 --timeout 60000 @@ -429,7 +439,7 @@ jobs: # the script, because it was needed identically at ten call sites and # the one place it was written by hand is the one place it was wrong. scripts/ci/playwright-in-container.sh test \ - --project=${{ matrix.project }} --shard=${{ matrix.shard }} \ + --project=${{ matrix.browser }}-${{ matrix.slice.project }} --shard=${{ matrix.slice.shard }} \ $WORKERS_FLAG \ --grep-invert='@hosted' \ --reporter=line,json,./tests/e2e/reporters/assertion-count-reporter.ts || true @@ -450,7 +460,7 @@ jobs: # the check into the shared script without this line would have # silently disarmed the very protection #830 added. PLAYWRIGHT_REQUIRED_ENV: NEXT_PUBLIC_SUPABASE_URL SUPABASE_SERVICE_ROLE_KEY NEXT_PUBLIC_SUPABASE_ANON_KEY - WORKERS: ${{ matrix.workers }} + WORKERS: ${{ matrix.slice.workers }} # EVERY JOB MUST DO ITS OWN FULL SETUP (#575). # # auth.setup.ts:146 gates the second half of setup on @@ -540,7 +550,6 @@ jobs: sys.exit(1) EOF - - name: No passing test may have run zero assertions (#861) if: always() # Separate from the step above because it answers a different question. @@ -552,13 +561,13 @@ jobs: - name: Name this shard's artifact if: always() id: slug - run: echo "id=${{ matrix.project }}-$(echo '${{ matrix.shard }}' | tr '/' '-')" >> "$GITHUB_OUTPUT" + run: echo "id=${{ matrix.browser }}-${{ matrix.slice.project }}-$(echo '${{ matrix.slice.shard }}' | tr '/' '-')" >> "$GITHUB_OUTPUT" - name: Upload results if: always() uses: actions/upload-artifact@v6 with: - # 24 jobs uploading `e2e-local-report` would collide; upload-artifact v4+ + # Every selected shard uploading `e2e-local-report` would collide; upload-artifact v4+ # errors on a duplicate name, so every shard needs its own. name: e2e-local-${{ steps.slug.outputs.id }} path: | @@ -577,7 +586,7 @@ jobs: # diagnosed from log text because the trace it advertised did not exist. # # ONLY ON FAILURE, deliberately. On a green run this directory is noise, and - # uploading it from 24 shards every time would trade one waste for another. The + # uploading it from every selected shard would trade one waste for another. The # artifact is worth having exactly when someone is about to go looking for it. - name: Upload failure diagnostics if: failure() @@ -593,7 +602,7 @@ jobs: # PARITY — the only thing that makes this run worth doing (#575 step 2) # ========================================================================== # - # "All 24 jobs went green" is NOT parity. A local stack that silently skipped + # "All selected jobs went green" is NOT parity. A local stack that silently skipped # every auth test would also report all-green — that is the #396 shape, and it # is exactly what a migration that failed to apply would look like. # @@ -608,10 +617,11 @@ jobs: # # TWO TIERS, BECAUSE THEY ANSWER DIFFERENT QUESTIONS (#732). # - # Tier 1 — "did the suite run and pass?" True on any commit. 24/24 shards, - # zero failures, above a floor. This is the gate. - # Tier 2 — "does it match the cloud exactly?" Only interpretable on de0f7f0, - # because on any other SHA the test set has moved. + # Tier 1 — "did the selected suite run and pass?" True on any commit. Every + # expected shard, zero failures, above a mode-scaled floor. This is the gate. + # Tier 2 — "does the full matrix match the cloud exactly?" Only interpretable + # in 24-shard mode on de0f7f0, because the baseline covers all browsers + # and on any other SHA the test set has moved. # # They used to be one check, with tier 2's SHA guard placed FIRST — so on every # commit that was not the baseline the job exited 0 before asking whether @@ -665,6 +675,8 @@ jobs: # the check on a real parity run or fail every PR against a baseline that # does not describe it. TESTED_SHA: ${{ steps.tested.outputs.sha }} + EXPECTED_SHARDS: ${{ needs.changes.outputs.expected_shards }} + BROWSER_MODE: ${{ needs.changes.outputs.browser_mode }} run: | set -uo pipefail python3 - <<'EOF' @@ -678,15 +690,29 @@ jobs: # baseline itself cannot be regenerated (artifacts expired; quota until 09-02). BASE = {"passed": 1807, "skipped": 194, "flaky": 0} BASE_SHA = "de0f7f080c8d75949e4e6c89fdf66ab7d3da8029" - # Deliberately well under both the cloud baseline (1807) and the last local - # run (1894): this is a "did the suite actually run" floor, not a parity - # number. Parity is tier 2 below. + browser_mode = os.environ.get("BROWSER_MODE", "") + try: + expected_shards = int(os.environ["EXPECTED_SHARDS"]) + except (KeyError, ValueError): + print("::error::EXPECTED_SHARDS is missing or invalid — browser selection did not report a usable plan") + sys.exit(1) + expected_by_mode = {"chromium": 8, "full": 24} + if expected_by_mode.get(browser_mode) != expected_shards: + print(f"::error::inconsistent browser plan: mode={browser_mode!r}, expected_shards={expected_shards}") + sys.exit(1) + + # Preserve the existing 1700/24 anti-vacuity threshold and scale it for the + # selected matrix. ceil(1700 * 8 / 24) = 567; the measured Chromium slice on + # run 32584683318 reported 683 passing/flaky tests, so this keeps real margin. + # This is a "did the suite actually run" floor, not a parity number. Full-mode + # parity is tier 2 below. # # UNCHANGED by the @hosted split (#725), and the reason is worth stating because # it is counter-intuitive: the six excluded tests were FAILING locally, so they # were counted in `failed`, never in `passed`. Removing them subtracts nothing # from this total. - FLOOR = 1700 + FULL_FLOOR = 1700 + FLOOR = (FULL_FLOOR * expected_shards + 23) // 24 files = sorted(pathlib.Path("shards").rglob("results.json")) if not files: @@ -715,21 +741,28 @@ jobs: for s in data.get("suites", []): walk(s) - print(f"shards reporting: {len(files)}/24") - print(f" passed {tot['passed']:>5} baseline {BASE['passed']}") - print(f" skipped {tot['skipped']:>5} baseline {BASE['skipped']}") - print(f" flaky {tot['flaky']:>5} baseline {BASE['flaky']}") + print(f"browser mode: {browser_mode}") + print(f"shards reporting: {len(files)}/{expected_shards}") + print(f" passed {tot['passed']:>5}") + print(f" skipped {tot['skipped']:>5}") + print(f" flaky {tot['flaky']:>5}") print(f" failed {tot['failed']:>5}") summary = os.environ.get("GITHUB_STEP_SUMMARY") if summary: with open(summary, "a") as fh: - fh.write("### E2E local parity\n\n") - fh.write("| | local | cloud baseline |\n|---|---|---|\n") - for k in ("passed", "skipped", "flaky"): - mark = "OK" if tot[k] == BASE[k] else "DIFF" - fh.write(f"| {k} | {tot[k]} | {BASE[k]} | {mark} |\n") - fh.write(f"| failed | {tot['failed']} | 0 | |\n") + fh.write(f"### E2E local — {browser_mode} mode\n\n") + if browser_mode == "full": + fh.write("| | local | cloud baseline |\n|---|---|---|\n") + for k in ("passed", "skipped", "flaky"): + mark = "OK" if tot[k] == BASE[k] else "DIFF" + fh.write(f"| {k} | {tot[k]} | {BASE[k]} | {mark} |\n") + fh.write(f"| failed | {tot['failed']} | 0 | |\n") + else: + fh.write("| metric | value |\n|---|---:|\n") + fh.write(f"| shards | {len(files)} / {expected_shards} |\n") + for k in ("passed", "skipped", "flaky", "failed"): + fh.write(f"| {k} | {tot[k]} |\n") # ── TIER 1: the always-on gate (#732) ──────────────────────────────── # @@ -741,8 +774,8 @@ jobs: # "did anything fail" down with it. Those are separate questions, so # they are now separate tiers. problems = [] - if len(files) != 24: - problems.append(f"only {len(files)} of 24 shards reported") + if len(files) != expected_shards: + problems.append(f"only {len(files)} of {expected_shards} shards reported") if tot["failed"]: problems.append(f"{tot['failed']} test(s) failed") # A floor, not an equality: the test set legitimately grows. It catches @@ -759,11 +792,20 @@ jobs: print(f"::error::{p}") sys.exit(1) - # ── TIER 2: count parity, only interpretable on the baseline SHA ────── + # ── TIER 2: count parity, only interpretable for a full baseline run ─ + if browser_mode != "full": + print("") + print(f"Artifact totals OK: {expected_shards}/{expected_shards} shards, 0 failures, above the floor.") + print("The shard matrix's OWN verdict is a separate step after this one") + print("— these totals cannot see a shard that failed after reporting (#934).") + print("COUNT PARITY not evaluated — Chromium PR mode is intentionally an") + print("8-shard subset, while the cloud baseline describes all 24 shards.") + sys.exit(0) + on_baseline = os.environ.get("TESTED_SHA", "") == BASE_SHA if not on_baseline: print("") - print("Artifact totals OK: 24/24 shards, 0 failures, above the floor.") + print(f"Artifact totals OK: {expected_shards}/{expected_shards} shards, 0 failures, above the floor.") print("The shard matrix's OWN verdict is a separate step after this one") print("— these totals cannot see a shard that failed after reporting (#934).") print("COUNT PARITY not evaluated — the cloud numbers describe") @@ -825,3 +867,100 @@ jobs: echo "::error::log names the cause. Do not relax this to make a run green." exit 1 fi + + # Queue time and execution time were previously collapsed into one "E2E is slow" + # number (#950). This is observational only: telemetry must never strand the required + # check if GitHub's own API is unavailable. + - name: Report queue and execution timing + # Failed-job reruns carry successful jobs forward with their old timestamps, so + # combining them would publish plausible-looking but false queue statistics. + if: needs.changes.outputs.run == 'true' && github.run_attempt == '1' + continue-on-error: true + uses: actions/github-script@v8 + env: + E2E_BROWSER_MODE: ${{ needs.changes.outputs.browser_mode }} + E2E_EXPECTED_SHARDS: ${{ needs.changes.outputs.expected_shards }} + with: + script: | + const owner = context.repo.owner; + const repo = context.repo.repo; + const runId = context.runId; + const attemptNumber = Number(process.env.GITHUB_RUN_ATTEMPT || '1'); + const { data: run } = await github.rest.actions.getWorkflowRunAttempt({ + owner, + repo, + run_id: runId, + attempt_number: attemptNumber, + }); + const jobs = await github.paginate( + github.rest.actions.listJobsForWorkflowRunAttempt, + { + owner, + repo, + run_id: runId, + attempt_number: attemptNumber, + per_page: 100, + } + ); + + const changes = jobs.find((job) => job.name === 'Does this change need E2E?'); + const shards = jobs.filter( + (job) => /^(chromium|firefox|webkit)-(msg|gen)/.test(job.name) && + job.started_at && job.completed_at + ); + const expected = Number(process.env.E2E_EXPECTED_SHARDS); + if (!changes?.started_at || !changes.completed_at || shards.length !== expected) { + throw new Error( + `timing inputs incomplete: changes=${changes?.started_at || 'missing'}/` + + `${changes?.completed_at || 'missing'}, ` + + `shards=${shards.length}/${expected}` + ); + } + + const workflowStartedAt = new Date(run.run_started_at).getTime(); + const readyAt = new Date(changes.completed_at).getTime(); + const minutes = (milliseconds) => milliseconds / 60000; + const queue = shards.map((job) => + minutes(Math.max(0, new Date(job.started_at).getTime() - readyAt)) + ); + const execution = shards.map((job) => + minutes(new Date(job.completed_at).getTime() - new Date(job.started_at).getTime()) + ); + const stats = (values) => { + const sorted = [...values].sort((a, b) => a - b); + const middle = Math.floor(sorted.length / 2); + const median = sorted.length % 2 + ? sorted[middle] + : (sorted[middle - 1] + sorted[middle]) / 2; + return [sorted[0], median, sorted.at(-1)].map((value) => value.toFixed(1)); + }; + const [queueMin, queueMedian, queueMax] = stats(queue); + const [runMin, runMedian, runMax] = stats(execution); + const initialQueue = minutes( + Math.max(0, new Date(changes.started_at).getTime() - workflowStartedAt) + ).toFixed(1); + const firstShardAt = Math.min( + ...shards.map((job) => new Date(job.started_at).getTime()) + ); + const firstShardDelay = minutes( + Math.max(0, firstShardAt - workflowStartedAt) + ).toFixed(1); + + await core.summary + .addHeading('E2E local timing', 3) + .addRaw( + `${process.env.E2E_BROWSER_MODE} mode · ${expected} shards · ` + + `initial queue ${initialQueue} min · ` + + `first shard ${firstShardDelay} min after workflow start\n\n` + ) + .addTable([ + [ + { data: 'minutes', header: true }, + { data: 'min', header: true }, + { data: 'median', header: true }, + { data: 'max', header: true }, + ], + ['shard queue after changes', queueMin, queueMedian, queueMax], + ['shard execution', runMin, runMedian, runMax], + ]) + .write(); diff --git a/CLAUDE.md b/CLAUDE.md index 5c5bb485..a9cad4ec 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -416,6 +416,7 @@ Most of the pain below traces to one fact — every job shared one cloud Supabas | --------------- | -------------------------------------------- | ---------------------------------------------- | | backend | a Supabase per runner, brought up in the job | the shared hosted project | | runs on | **every PR and every push to main** | push/PR, currently blocked by the budget guard | +| browsers | PR: Chromium; push/dispatch/`full-e2e`: all | Chromium normally; cross-browser is opt-in | | secrets | **none** — uses the tracked public demo keys | 32 `secrets.*` references | | mutex | none needed; nothing is shared | repo-wide, `max-parallel: 2` | | `@hosted` tests | excluded via `--grep-invert` | runs everything | @@ -697,7 +698,7 @@ To lift it: `gh api -X DELETE repos/TortoiseWolfe/ScriptHammer/branches/main/pro - Never create components manually - use the generator - All PRs must pass component structure validation -- **E2E runs in CI on TWO lanes** — `e2e-local.yml` (a Supabase per runner, every PR, no secrets) and `e2e.yml` (the shared hosted project). 24 jobs each. See "THERE ARE NOW TWO E2E LANES" above before debugging either +- **E2E runs in CI on TWO lanes** — `e2e-local.yml` (a Supabase per runner, every PR, no secrets) and `e2e.yml` (the shared hosted project). The local lane runs 8 Chromium shards on an ordinary PR and 24 shards on push, dispatch, or a `full-e2e` PR. See "THERE ARE NOW TWO E2E LANES" above before debugging either - Docker-first development is mandatory - Use `min-h-11 min-w-11` for 44px touch targets (mobile-first) diff --git a/scripts/__tests__/e2e-local-browser-gate.test.js b/scripts/__tests__/e2e-local-browser-gate.test.js new file mode 100644 index 00000000..e41069a3 --- /dev/null +++ b/scripts/__tests__/e2e-local-browser-gate.test.js @@ -0,0 +1,491 @@ +/** + * The required local E2E lane must select browsers before runner allocation (#950). + * + * A step-level gate still creates all 24 matrix jobs, so it does not remove the queue. + * The `changes` job instead emits a browser axis consumed by the matrix: ordinary PRs + * get Chromium's eight shards; push, dispatch, and `full-e2e` PR events get all 24. + * The aggregate must consume the same plan or its old literal 24-shard gate makes every + * ordinary PR red. + * + * These checks strip comments before matching. The workflow explains every invariant in + * prose, and matching that prose is how several earlier guards became unable to fail. + */ + +'use strict'; + +const { describe, it } = require('node:test'); +const assert = require('node:assert'); +const fs = require('node:fs'); +const path = require('node:path'); + +const ROOT = path.resolve(__dirname, '..', '..'); +const WORKFLOW = path.join(ROOT, '.github', 'workflows', 'e2e-local.yml'); +const CLAUDE = path.join(ROOT, 'CLAUDE.md'); +const RAW = fs.readFileSync(WORKFLOW, 'utf8'); + +function stripComments(text) { + return text + .split('\n') + .filter((line) => !/^\s*#/.test(line)) + .join('\n'); +} + +function between(text, startNeedle, endNeedle) { + const start = text.indexOf(startNeedle); + if (start === -1) return ''; + const rest = text.slice(start); + const end = rest.indexOf(endNeedle, startNeedle.length); + return end === -1 ? rest : rest.slice(0, end); +} + +function normalizedLines(text) { + return text + .split('\n') + .map((line) => line.trim()) + .filter(Boolean); +} + +function triggerErrors(text) { + const errors = []; + const trigger = between(text, 'on:\n', 'jobs:\n'); + if ( + !/^ pull_request:\n branches: \[main\]\n types: \[opened, synchronize, reopened, labeled\]$/m.test( + trigger + ) + ) { + errors.push('pull requests no longer include the full-e2e label event'); + } + return errors; +} + +function shellAssignments(branch) { + return Object.fromEntries( + [ + ...branch.matchAll(/^\s*(browsers|expected_shards|browser_mode)=(.+)$/gm), + ].map(([, name, value]) => [name, value.trim()]) + ); +} + +function selectionErrors(changes) { + const errors = []; + const selector = between( + changes, + ' - name: Select browser coverage', + ' - uses: actions/checkout@v5' + ); + if (!/^ id: browsers$/m.test(selector)) { + errors.push('selector step has no stable browsers id'); + } + + const env = selector.match( + /E2E_FULL_BROWSERS:\s*>-\s*([\s\S]*?)\n run: \|/ + ); + const expression = env?.[1].replace(/\s+/g, ' ').trim(); + const expectedExpression = + "${{ github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'full-e2e') }}"; + if (expression !== expectedExpression) { + errors.push(`browser event expression changed: ${expression || 'missing'}`); + } + + const scriptStart = selector.indexOf(' run: |'); + const script = scriptStart === -1 ? '' : selector.slice(scriptStart + 14); + const expectedScript = [ + 'if [ "$E2E_FULL_BROWSERS" = "true" ]; then', + 'browsers=\'["chromium","firefox","webkit"]\'', + 'expected_shards=24', + 'browser_mode=full', + 'else', + 'browsers=\'["chromium"]\'', + 'expected_shards=8', + 'browser_mode=chromium', + 'fi', + '{', + 'echo "browsers=$browsers"', + 'echo "expected_shards=$expected_shards"', + 'echo "browser_mode=$browser_mode"', + '} >> "$GITHUB_OUTPUT"', + 'echo "browser mode: $browser_mode ($expected_shards shards)"', + ]; + if ( + JSON.stringify(normalizedLines(script)) !== JSON.stringify(expectedScript) + ) { + errors.push( + 'selector run script is not the exact event-to-matrix contract' + ); + } + + const branch = selector.match( + /^\s*if \[ "\$E2E_FULL_BROWSERS" = "true" \]; then\n([\s\S]*?)^\s*else\n([\s\S]*?)^\s*fi\s*$/m + ); + if (!branch) { + errors.push('selector no longer uses a positive true branch'); + } else { + const full = shellAssignments(branch[1]); + const ordinary = shellAssignments(branch[2]); + const expectedFull = { + browsers: `'["chromium","firefox","webkit"]'`, + expected_shards: '24', + browser_mode: 'full', + }; + const expectedOrdinary = { + browsers: `'["chromium"]'`, + expected_shards: '8', + browser_mode: 'chromium', + }; + if (JSON.stringify(full) !== JSON.stringify(expectedFull)) { + errors.push(`full branch changed: ${JSON.stringify(full)}`); + } + if (JSON.stringify(ordinary) !== JSON.stringify(expectedOrdinary)) { + errors.push(`ordinary PR branch changed: ${JSON.stringify(ordinary)}`); + } + } + + for (const output of ['browsers', 'expected_shards', 'browser_mode']) { + const mapping = new RegExp( + `^ ${output}: \\$\\{\\{ steps\\.browsers\\.outputs\\.${output} \\}\\}$`, + 'm' + ); + if (!mapping.test(changes)) { + errors.push(`changes output ${output} is not wired to the selector`); + } + const emission = new RegExp( + `^ echo "${output}=\\$${output}"$`, + 'm' + ); + if (!emission.test(selector)) { + errors.push(`selector does not emit ${output}`); + } + } + if (!/^ \} >> "\$GITHUB_OUTPUT"$/m.test(selector)) { + errors.push('selector output group is not appended to GITHUB_OUTPUT'); + } + return errors; +} + +function matrixErrors(text) { + const errors = []; + if ( + !/browser:\s*\$\{\{\s*fromJSON\(needs\.changes\.outputs\.browsers\)\s*\}\}/.test( + text + ) + ) { + errors.push('matrix browser axis is not derived from the changes output'); + } + + const matrixBlock = between(text, ' matrix:', '\n\n steps:'); + const expectedMatrix = [ + 'matrix:', + 'browser: ${{ fromJSON(needs.changes.outputs.browsers) }}', + 'slice:', + '- { project: msg, shard: 1/1 }', + '- { project: msg-iso, shard: 1/1, workers: 2 }', + '- { project: gen, shard: 1/6 }', + '- { project: gen, shard: 2/6 }', + '- { project: gen, shard: 3/6 }', + '- { project: gen, shard: 4/6 }', + '- { project: gen, shard: 5/6 }', + '- { project: gen, shard: 6/6 }', + ]; + if ( + JSON.stringify(normalizedLines(matrixBlock)) !== + JSON.stringify(expectedMatrix) + ) { + errors.push('matrix block is not the exact browser × eight-slice contract'); + } + const axes = [...matrixBlock.matchAll(/^ ([a-z][\w-]*):/gm)].map( + (match) => match[1] + ); + if (JSON.stringify(axes) !== JSON.stringify(['browser', 'slice'])) { + errors.push(`matrix axes are not exactly browser × slice: ${axes}`); + } + + const rows = [...matrixBlock.matchAll(/^\s*-\s*\{([^}]+)\}/gm)].map((match) => + match[1].replace(/\s+/g, ' ').trim() + ); + const expectedRows = [ + 'project: msg, shard: 1/1', + 'project: msg-iso, shard: 1/1, workers: 2', + 'project: gen, shard: 1/6', + 'project: gen, shard: 2/6', + 'project: gen, shard: 3/6', + 'project: gen, shard: 4/6', + 'project: gen, shard: 5/6', + 'project: gen, shard: 6/6', + ]; + if (JSON.stringify(rows) !== JSON.stringify(expectedRows)) { + errors.push(`matrix slices changed: ${JSON.stringify(rows)}`); + } + if ( + !/--project=\$\{\{ matrix\.browser \}\}-\$\{\{ matrix\.slice\.project \}\}/.test( + text + ) + ) { + errors.push('Playwright project is not composed from browser + slice'); + } + if (!/--shard=\$\{\{ matrix\.slice\.shard \}\}/.test(text)) { + errors.push('Playwright shard is not read from the selected slice'); + } + if (!/^ WORKERS: \$\{\{ matrix\.slice\.workers \}\}$/m.test(text)) { + errors.push('workers override is not read from the selected slice'); + } + return errors; +} + +function aggregateErrors(text) { + const errors = []; + if ( + !/^\s*EXPECTED_SHARDS:\s*\$\{\{\s*needs\.changes\.outputs\.expected_shards\s*\}\}/m.test( + text + ) + ) { + errors.push('aggregate does not consume the selector shard count'); + } + if ( + !/^\s*BROWSER_MODE:\s*\$\{\{\s*needs\.changes\.outputs\.browser_mode\s*\}\}/m.test( + text + ) + ) { + errors.push('aggregate does not consume the selector browser mode'); + } + if ( + !/expected_shards\s*=\s*int\(os\.environ\["EXPECTED_SHARDS"\]\)/.test(text) + ) { + errors.push('aggregate does not parse EXPECTED_SHARDS'); + } + if (!/len\(files\)\s*!=\s*expected_shards/.test(text)) { + errors.push('artifact completeness is not checked against expected_shards'); + } + if (/len\(files\)\s*!=\s*24/.test(text)) { + errors.push('literal 24-shard artifact gate survived'); + } + if ( + !/FLOOR\s*=\s*\(FULL_FLOOR \* expected_shards \+ 23\) \/\/ 24/.test(text) + ) { + errors.push('anti-vacuity floor is not scaled with the selected matrix'); + } + const subset = text.indexOf('if browser_mode != "full":'); + const baseline = text.indexOf('on_baseline ='); + const subsetBlock = text.slice(subset, baseline); + const expectedSubset = [ + 'if browser_mode != "full":', + 'print("")', + 'print(f"Artifact totals OK: {expected_shards}/{expected_shards} shards, 0 failures, above the floor.")', + 'print("The shard matrix\'s OWN verdict is a separate step after this one")', + 'print("— these totals cannot see a shard that failed after reporting (#934).")', + 'print("COUNT PARITY not evaluated — Chromium PR mode is intentionally an")', + 'print("8-shard subset, while the cloud baseline describes all 24 shards.")', + 'sys.exit(0)', + ]; + if ( + subset === -1 || + baseline === -1 || + subset > baseline || + JSON.stringify(normalizedLines(subsetBlock)) !== + JSON.stringify(expectedSubset) + ) { + errors.push('Chromium subset is not excluded before full cloud parity'); + } + return errors; +} + +describe('e2e-local browser selection (#950)', () => { + const code = stripComments(RAW); + const changes = between(code, ' changes:', ' e2e-local:'); + const selection = between( + changes, + ' - name: Select browser coverage', + ' - uses: actions/checkout@v5' + ); + const matrix = between(code, ' e2e-local:', ' parity:'); + const aggregate = between(code, ' parity:', '__END_OF_WORKFLOW__'); + + it('found real workflow code rather than comments or an empty stale path', () => { + assert.ok(RAW.length > 20_000, `${WORKFLOW} is suspiciously small`); + assert.ok( + code.length < RAW.length - 5_000, + 'comment stripper removed too little' + ); + assert.ok(selection.length > 500, 'browser selector step not found'); + assert.ok(matrix.length > 5_000, 'matrix job not found'); + assert.ok(aggregate.length > 5_000, 'aggregate job not found'); + assert.doesNotMatch(code, /Filter the matrix BEFORE runner allocation/); + }); + + it('launches a fresh run when full-e2e is added', () => { + assert.deepStrictEqual(triggerErrors(code), []); + }); + + it('selects one browser for ordinary PRs and three for full events', () => { + assert.deepStrictEqual(selectionErrors(changes), []); + }); + + it('builds only the selected browser × eight slices', () => { + assert.deepStrictEqual(matrixErrors(matrix), []); + }); + + it('makes the required aggregate consume the selected shard count and floor', () => { + assert.deepStrictEqual(aggregateErrors(aggregate), []); + }); + + it('records timing without letting telemetry strand the required check', () => { + const timing = between( + aggregate, + ' - name: Report queue and execution timing', + '__END_OF_WORKFLOW__' + ); + assert.ok(timing.length > 1_000, 'timing step not found'); + assert.match( + timing, + /if: needs\.changes\.outputs\.run == 'true' && github\.run_attempt == '1'/ + ); + assert.match(timing, /continue-on-error:\s*true/); + assert.match(timing, /uses:\s*actions\/github-script@v8/); + assert.match(timing, /getWorkflowRunAttempt/); + assert.match(timing, /listJobsForWorkflowRunAttempt/); + assert.match(timing, /changes\.started_at/); + assert.match(timing, /run\.run_started_at/); + assert.match(timing, /firstShardAt\s*=\s*Math\.min/); + assert.doesNotMatch(timing, /run\.created_at/); + assert.match(code, /^permissions:\n\s+contents: read\n\s+actions: read/m); + }); + + it('documents the 8-shard PR path for future maintainers', () => { + const claude = fs.readFileSync(CLAUDE, 'utf8'); + assert.match(claude, /8 Chromium shards on an ordinary PR/); + assert.match(claude, /24 shards on push, dispatch, or a `full-e2e` PR/); + }); + + it('CONTROL: selector checks reject both load-saving regressions', () => { + const collapsed = changes.replace( + '["chromium","firefox","webkit"]', + '["chromium"]' + ); + assert.ok( + selectionErrors(collapsed).length > 0, + 'collapsed full mode was accepted' + ); + + const noPush = changes.replace( + "github.event_name != 'pull_request' ||", + '' + ); + assert.ok( + selectionErrors(noPush).length > 0, + 'Chromium-only push was accepted' + ); + + const inverted = changes.replace( + 'if [ "$E2E_FULL_BROWSERS" = "true" ]; then', + 'if [ "$E2E_FULL_BROWSERS" != "true" ]; then' + ); + assert.ok( + selectionErrors(inverted).length > 0, + 'inverted full/ordinary branches were accepted' + ); + + const missingOutput = changes.replace( + ' echo "browsers=$browsers"\n', + '' + ); + assert.ok( + selectionErrors(missingOutput).length > 0, + 'selector with an un-emitted browser plan was accepted' + ); + + const overwrittenPlan = changes.replace( + ' {\n', + ' browsers=\'["chromium"]\'\n expected_shards=8\n browser_mode=chromium\n {\n' + ); + assert.ok( + selectionErrors(overwrittenPlan).length > 0, + 'selector outputs overwritten after the branch were accepted' + ); + }); + + it('CONTROL: trigger check rejects losing the labeled activity', () => { + const noLabelEvent = code.replace( + 'types: [opened, synchronize, reopened, labeled]', + 'types: [opened, synchronize, reopened]' + ); + assert.ok( + triggerErrors(noLabelEvent).length > 0, + 'full-e2e label without a triggering activity was accepted' + ); + }); + + it('CONTROL: matrix check rejects fixed, malformed, and extra axes', () => { + const fixed = matrix.replace( + '${{ fromJSON(needs.changes.outputs.browsers) }}', + '[chromium, firefox, webkit]' + ); + assert.ok( + matrixErrors(fixed).length > 0, + 'fixed 24-job matrix was accepted' + ); + + const malformedShard = matrix.replace( + '{ project: gen, shard: 1/6 }', + '{ project: gen, shard: 1/7 }' + ); + assert.ok( + matrixErrors(malformedShard).length > 0, + 'incorrect shard denominator was accepted' + ); + + const extraAxis = matrix.replace( + ' slice:', + ' duplicate: [a, b]\n slice:' + ); + assert.ok( + matrixErrors(extraAxis).length > 0, + 'extra runner-multiplying matrix axis was accepted' + ); + + const quotedAxis = matrix.replace( + ' slice:', + ' "duplicate": [a, b]\n slice:' + ); + assert.ok( + matrixErrors(quotedAxis).length > 0, + 'quoted extra matrix axis was accepted' + ); + + const multilineSlice = matrix.replace( + ' - { project: gen, shard: 6/6 }', + ' - { project: gen, shard: 6/6 }\n - project: gen\n shard: 7/7' + ); + assert.ok( + matrixErrors(multilineSlice).length > 0, + 'multiline ninth slice was accepted' + ); + }); + + it('CONTROL: aggregate check rejects restoring the literal 24', () => { + const fixed = aggregate.replace( + '${{ needs.changes.outputs.expected_shards }}', + '24' + ); + assert.ok( + aggregateErrors(fixed).length > 0, + 'literal 24-shard aggregate was accepted' + ); + + const paritySubset = aggregate.replace( + /if browser_mode != "full":([\s\S]*?)sys\.exit\(0\)/, + 'if browser_mode != "full":$1print("subset")' + ); + assert.ok( + aggregateErrors(paritySubset).length > 0, + 'Chromium subset falling through to cloud parity was accepted' + ); + + const hiddenExit = aggregate.replace( + ' sys.exit(0)\n\n on_baseline =', + ' if False:\n sys.exit(0)\n\n on_baseline =' + ); + assert.ok( + aggregateErrors(hiddenExit).length > 0, + 'Chromium exit hidden behind a false branch was accepted' + ); + }); +}); diff --git a/scripts/__tests__/e2e-local-triggers.test.js b/scripts/__tests__/e2e-local-triggers.test.js index e3844528..3a4ddb1b 100644 --- a/scripts/__tests__/e2e-local-triggers.test.js +++ b/scripts/__tests__/e2e-local-triggers.test.js @@ -43,6 +43,26 @@ function triggerBlock(text) { return end === -1 ? rest : rest.slice(0, end); } +function matrixGateErrors(text) { + const errors = []; + const start = text.indexOf(' e2e-local:'); + const end = text.indexOf(' parity:', start); + const matrix = + start === -1 ? '' : text.slice(start, end === -1 ? undefined : end); + if (!/^ needs:\s*changes$/m.test(matrix)) { + errors.push('matrix no longer depends on changes'); + } + if ( + !new RegExp( + `^ if: ${GATE.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}$`, + 'm' + ).test(matrix) + ) { + errors.push('matrix is not gated by the changes result'); + } + return errors; +} + describe('e2e-local.yml runs on every PR without trapping docs-only ones (#575)', () => { it('the parser found the workflow it is asserting about', () => { // Without this every assertion below could pass vacuously on an empty read. @@ -72,13 +92,16 @@ describe('e2e-local.yml runs on every PR without trapping docs-only ones (#575)' }); it('the matrix is gated on the changes job, so docs-only PRs cost nothing', () => { - assert.ok( - /e2e-local:[\s\S]*?needs:\s*changes/.test(yaml), - 'the matrix no longer depends on `changes` — every docs typo would run 24 jobs' + assert.deepStrictEqual( + matrixGateErrors(yaml), + [], + 'the e2e-local matrix itself must carry the changes dependency and gate' ); + + const ungated = yaml.replace(` if: ${GATE}\n`, ''); assert.ok( - yaml.includes(`if: ${GATE}`), - `the matrix is not gated on \`${GATE}\`` + matrixGateErrors(ungated).length > 0, + 'CONTROL: removing the matrix job gate was accepted' ); });