diff --git a/.github/workflows/e2e-artifacts.yml b/.github/workflows/e2e-artifacts.yml index fb1be012a..937b58d1c 100644 --- a/.github/workflows/e2e-artifacts.yml +++ b/.github/workflows/e2e-artifacts.yml @@ -1,9 +1,7 @@ -# This workflow is intentionally non-blocking: it uploads Playwright -# artifacts on PRs for manual inspection but never fails the PR. Do NOT -# add `e2e-artifacts / e2e-artifacts` to the `dev` ruleset as a required -# check -- `continue-on-error: true` on the job means the check reports -# success even when tests fail, which would make the gate lie about -# every PR forever. Decision recorded in issue #54. +# This workflow is the PR E2E check for rendered app behavior. It must +# fail when Playwright smoke fails, while still uploading artifacts for +# debugging. The `dev` ruleset should require `e2e-artifacts` only after +# this workflow is merged, so the required check reflects real failures. name: e2e-artifacts on: @@ -34,7 +32,6 @@ jobs: e2e-artifacts: runs-on: ubuntu-latest timeout-minutes: 30 - continue-on-error: true steps: - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # actions/checkout@v6 with: @@ -85,10 +82,10 @@ jobs: bun --cwd packages/app test:e2e:local fi - - name: Warn on smoke failure + - name: Warn on E2E failure if: failure() run: | - echo "::warning::Non-blocking e2e-artifacts failed. Download the uploaded artifacts for the Playwright report and junit output." + echo "::warning::E2E failed. Download the uploaded artifacts for the Playwright report, junit output, and test results." - name: Upload e2e artifacts if: always() diff --git a/packages/opencode/test/config/e2e-artifacts-workflow.test.ts b/packages/opencode/test/config/e2e-artifacts-workflow.test.ts index 5d0b3cd39..ee793aa3b 100644 --- a/packages/opencode/test/config/e2e-artifacts-workflow.test.ts +++ b/packages/opencode/test/config/e2e-artifacts-workflow.test.ts @@ -6,7 +6,7 @@ const repoRoot = path.join(import.meta.dir, "../../../..") const workflowPath = path.join(repoRoot, ".github", "workflows", "e2e-artifacts.yml") describe("e2e artifacts workflow", () => { - test("defines a visible but non-blocking PR diagnostics workflow", () => { + test("defines a required PR e2e workflow with retained failure artifacts", () => { const workflow = readWorkflow(workflowPath) const parsed = parseWorkflow(workflowPath) const job = parsed.jobs?.["e2e-artifacts"] @@ -15,7 +15,7 @@ describe("e2e artifacts workflow", () => { const bunStep = steps.find((step) => step.uses?.startsWith("oven-sh/setup-bun@")) const installBrowsersStep = steps.find((step) => step.name === "Install Playwright browsers") const runStep = steps.find((step) => step.name === "Run e2e") - const warnStep = steps.find((step) => step.name === "Warn on smoke failure") + const warnStep = steps.find((step) => step.name === "Warn on E2E failure") const uploadStep = steps.find((step) => step.name === "Upload e2e artifacts") expect(parsed.name).toBe("e2e-artifacts") @@ -36,15 +36,18 @@ describe("e2e artifacts workflow", () => { ) expect(parsed.permissions).toEqual({ contents: "read" }) expect(job?.["runs-on"]).toBe("ubuntu-latest") - expect(job?.["continue-on-error"]).toBe(true) + expect(job?.["continue-on-error"]).not.toBe(true) expect(checkoutStep?.uses).toBe("actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd") expect(checkoutStep?.with).toEqual({ "persist-credentials": false }) expect(bunStep?.uses).toBe("oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6") expect(installBrowsersStep?.run).toBe("bunx playwright install --with-deps chromium") expect(runStep?.run).toContain("bun --cwd packages/app test:e2e:local:smoke") + expect(runStep?.["continue-on-error"]).not.toBe(true) expect(warnStep?.if).toBe("failure()") expect(warnStep?.run).toContain("::warning::") + expect(warnStep?.run).not.toContain("Non-blocking") expect(uploadStep?.uses).toBe("actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a") + expect(uploadStep?.if).toBe("always()") expect(uploadStep?.with?.name).toBe("e2e-artifacts-linux-${{ github.run_attempt }}") expect(uploadStep?.with?.["if-no-files-found"]).toBe("ignore") expect(uploadStep?.with?.["retention-days"]).toBe(7)