From 3dbf964e30919064ef5db739a70fc07211ced032 Mon Sep 17 00:00:00 2001 From: Babissimo Date: Wed, 23 Sep 2026 19:00:27 +0100 Subject: [PATCH] Fail CI on a committed test.only, and keep a failed E2E's screenshots Playwright had no forbidOnly, so a test.only committed by mistake would quietly shrink the suite that decides whether production rolls back. In CI it now fails the run instead; staging's E2E, which runs first, is where it fires. Production's E2E job collects the suite in a step of its own before running it, so a suite that cannot load, from a test.only or a spec that throws at module scope, fails the job without failing the step the rollback reads: only tests that ran and failed roll production back. A failed staging or production E2E left nothing to inspect. Both E2E jobs now upload the failed tests' screenshots. Only the screenshots, although the ticket asks for traces: this repository's artifacts are public and GitHub masks secrets only in logs. A trace records every request's headers, and error-context.md holds a call log that lists them too; both would publish the node API key and the Cloudflare Access token. The error text is already in the job log, masked. With nothing to publish them, CI records no traces. The upload is bounded and allowed to fail, so collecting evidence cannot cancel e2e-prod before it publishes the verdict the rollback reads. The artifact name carries the run attempt, because a name cannot repeat within a run. e2e-prod's cap rises from 30 to 35 minutes, so the new steps' caps do not come out of the ten minutes its setup steps had. test_deploy_workflows.py holds every upload from a job that runs Playwright, and every upload of test-results in any workflow, to screenshots, with hidden files off, and to the bound. It pins production's collect step ahead of its test step, and each E2E job's cap above its steps' own caps with room for the setup steps, since a job timeout cancels the test step and a cancelled step is no verdict. ClickUp 123zgec4mxy. Co-Authored-By: Claude Opus 5.5 --- .../workflows/production-deploy-verify.yml | 34 +++++++- .github/workflows/staging-deploy-verify.yml | 18 ++++- backend/tests/test_deploy_workflows.py | 77 +++++++++++++++++++ e2e/playwright.config.ts | 7 +- 4 files changed, 128 insertions(+), 8 deletions(-) diff --git a/.github/workflows/production-deploy-verify.yml b/.github/workflows/production-deploy-verify.yml index 87a27318a..c069835c0 100644 --- a/.github/workflows/production-deploy-verify.yml +++ b/.github/workflows/production-deploy-verify.yml @@ -750,10 +750,11 @@ jobs: e2e-prod: name: Playwright E2E (production) runs-on: ubuntu-latest - # A backstop above the test step's own cap, not the primary bound: a job - # timeout cancels the test step, and a cancelled step is not a verdict the - # rollback job acts on, whereas the step cap marks it failed. - timeout-minutes: 30 + # A backstop above the steps' own caps (collect 3, tests 20, upload 2), with + # ten minutes for the setup steps, not the primary bound: a job timeout + # cancels the test step, and a cancelled step is not a verdict the rollback + # job acts on, whereas the step cap marks it failed. + timeout-minutes: 35 needs: [production-smoke-tests] # The calling job's gate, repeated as on the deploy. if: github.ref == 'refs/heads/main' && github.event_name == 'push' @@ -784,6 +785,16 @@ jobs: # fails here rather than in the tests, where it would roll production back. - name: Report the runner's Chrome run: node e2e/report-chrome.js + # A suite that cannot be collected (a committed test.only under + # forbidOnly, a spec that throws at module scope) says nothing about + # production, so it fails here, before the step the rollback reads. The + # same environment as that step, so it loads the modules the same way. + - name: Collect the E2E tests (production) + timeout-minutes: 3 + run: npm run test:e2e:prod -w e2e -- --list + env: + CI: "true" + RADAR_API_KEY: ${{ secrets.RADAR_API_KEY }} # Bounded at the step rather than relying on the job cap: a step timeout # marks the step failed, which still trips the rollback job, whereas a # job timeout cancels it and would not. A production wedged badly enough @@ -798,6 +809,21 @@ jobs: env: CI: "true" RADAR_API_KEY: ${{ secrets.RADAR_API_KEY }} + # Screenshots alone: artifacts are public, and GitHub masks secrets only in + # logs, while a trace and error-context.md's call log both list request + # headers, keys included. Bounded and allowed to fail, because an upload + # stalling into the job's cap would cancel the job before it outputs the + # verdict the rollback reads. + - name: Keep the failed tests' screenshots + if: failure() + timeout-minutes: 2 + continue-on-error: true + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: e2e-production-screenshots-${{ github.run_attempt }} + path: e2e/test-results/**/*.png + retention-days: 7 + if-no-files-found: ignore # A job of its own, so that production's root key is never handed to a job # that has just run npm's install scripts. test_deploy_workflows.py holds diff --git a/.github/workflows/staging-deploy-verify.yml b/.github/workflows/staging-deploy-verify.yml index faab1c2e3..bfe75d4c6 100644 --- a/.github/workflows/staging-deploy-verify.yml +++ b/.github/workflows/staging-deploy-verify.yml @@ -318,9 +318,9 @@ jobs: e2e: name: Playwright E2E (staging) runs-on: ubuntu-latest - # Only the setup steps sit outside the test step's own cap. See the - # production job for why the job cap is a backstop rather than the primary - # bound. + # A backstop above the steps' own caps (tests 20, upload 2), with room for + # the setup steps. See the production job for why the job cap is a + # backstop rather than the primary bound. timeout-minutes: 30 needs: [smoke-tests] # The browser is the runner image's Google Chrome, not one installed here; @@ -355,3 +355,15 @@ jobs: # login page as the app. Empty secrets render empty and send nothing. CF_ACCESS_CLIENT_ID: ${{ secrets.CF_ACCESS_CLIENT_ID }} CF_ACCESS_CLIENT_SECRET: ${{ secrets.CF_ACCESS_CLIENT_SECRET }} + # Screenshots alone, bounded and allowed to fail; see the step of the same + # name in production-deploy-verify.yml. + - name: Keep the failed tests' screenshots + if: failure() + timeout-minutes: 2 + continue-on-error: true + uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + with: + name: e2e-staging-screenshots-${{ github.run_attempt }} + path: e2e/test-results/**/*.png + retention-days: 7 + if-no-files-found: ignore diff --git a/backend/tests/test_deploy_workflows.py b/backend/tests/test_deploy_workflows.py index a51cd9104..8d38e5323 100644 --- a/backend/tests/test_deploy_workflows.py +++ b/backend/tests/test_deploy_workflows.py @@ -526,3 +526,80 @@ def test_a_following_runs_staging_does_not_wait_for_this_runs_production(): assert "github.event_name == 'pull_request' &&" in pull_request assert "github.run_id" in push and "github.run_id" not in pull_request assert _job("ci.yml", "staging")["concurrency"]["group"] != _job("ci.yml", "production")["concurrency"]["group"] + + +# ── What a failed E2E leaves behind ────────────────────────────────────────── +# This repository's artifacts are public and GitHub masks secrets only in logs. +# A Playwright trace records every request's headers and error-context.md holds +# a call log that lists them, keys included, so from test-results only the +# screenshots are ever published. + + +def _runs_playwright(job: dict) -> bool: + return any( + re.search(r"\btest:e2e\b|\bplaywright test\b", str(step.get("run", ""))) for step in job.get("steps", []) + ) + + +def _test_result_uploads() -> list: + # Every upload from a job that runs Playwright, whatever its path, since a + # broad one (`.`, the workspace) sweeps test-results in with it. Elsewhere, + # anything under e2e/ or named for Playwright: its report embeds the same + # call logs. + return [ + pytest.param(step, id=f"{path.name}:{name}") + for path in _workflow_files() + for name, job in _workflow(path.name)["jobs"].items() + for step in job.get("steps", []) + if str(step.get("uses", "")).startswith("actions/upload-artifact") + and ( + _runs_playwright(job) + or re.search(r"\be2e\b|test-results|playwright", str(step.get("with", {}).get("path", "")), re.IGNORECASE) + ) + ] + + +def test_both_e2e_jobs_keep_their_screenshots(): + assert {param.id for param in _test_result_uploads()} == { + "staging-deploy-verify.yml:e2e", + f"{PRODUCTION}:e2e-prod", + } + + +@pytest.mark.parametrize("upload", _test_result_uploads()) +def test_a_failed_e2e_publishes_its_screenshots_alone(upload): + assert upload["with"]["path"] == "e2e/test-results/**/*.png" + assert not upload["with"].get("include-hidden-files", False) + + +@pytest.mark.parametrize("upload", _test_result_uploads()) +def test_collecting_them_cannot_cost_the_verdict(upload): + # e2e-prod's output decides the rollback. An upload that stalls into the + # job's cap would cancel the job before it publishes one. + assert upload["if"] == "failure()" + assert upload["continue-on-error"] is True + assert upload["timeout-minutes"] <= 2 + + +@pytest.mark.parametrize(("workflow", "job"), [("staging-deploy-verify.yml", "e2e"), (PRODUCTION, "e2e-prod")]) +def test_an_e2e_jobs_cap_stays_a_backstop(workflow, job): + # The steps' own caps, with at least five minutes for the setup steps + # (checkout, npm ci, the Chrome report), fit under the job's. A job timeout + # cancels the test step, and a cancelled step is no verdict, so a production + # wedged enough to hang the suite would never be rolled back. + steps = _job(workflow, job)["steps"] + assert _runs_playwright(_job(workflow, job)) + capped = sum(int(step.get("timeout-minutes", 0)) for step in steps) + assert capped + 5 <= _job(workflow, job)["timeout-minutes"] + + +def test_a_suite_that_cannot_load_cannot_roll_production_back(): + # A committed test.only under forbidOnly, or a spec that throws at module + # scope, fails the collect step, and the step the rollback reads never runs. + steps = _job(PRODUCTION, "e2e-prod")["steps"] + collect = [step.get("name") for step in steps].index("Collect the E2E tests (production)") + run = [step.get("id") for step in steps].index("e2e") + assert collect < run + assert steps[collect]["run"] == steps[run]["run"] + " -- --list" + assert steps[collect]["env"] == steps[run]["env"] + assert steps[collect]["timeout-minutes"] <= 3 diff --git a/e2e/playwright.config.ts b/e2e/playwright.config.ts index b7f800061..30fae5499 100644 --- a/e2e/playwright.config.ts +++ b/e2e/playwright.config.ts @@ -117,6 +117,9 @@ export default defineConfig({ timeout: 30_000, expect: { timeout: 10_000 }, fullyParallel: true, + // A committed test.only would quietly shrink the suite that decides whether + // production rolls back, so in CI it fails the run instead. + forbidOnly: !!process.env.CI, retries: process.env.CI ? 2 : 0, reporter: process.env.CI ? "github" : "list", use: { @@ -125,7 +128,9 @@ export default defineConfig({ // names its host explicitly, so this only resolves a relative URL. baseURL: hosts.map, extraHTTPHeaders: accessHeaders, - trace: "on-first-retry", + // Never in CI, whose artifacts are public: a trace records every request's + // headers, keys included. + trace: process.env.CI ? "off" : "on-first-retry", screenshot: "only-on-failure", headless: true, },