From 63593b3add77b8b1795fcfac8bf92c762f72b13e Mon Sep 17 00:00:00 2001 From: Anton Niklasson Date: Fri, 13 Nov 2026 15:11:11 +0100 Subject: [PATCH] Attach named status board screenshots to every browser report --- .github/workflows/feedback-loop.yml | 2 +- packages/harness/README.md | 4 ++++ packages/harness/playwright.config.ts | 4 ++++ packages/harness/src/artifacts.ts | 19 +++++++++++++++++++ packages/harness/src/board.e2e.ts | 6 ++++++ packages/harness/src/states.e2e.ts | 12 +++++++++--- 6 files changed, 43 insertions(+), 4 deletions(-) create mode 100644 packages/harness/src/artifacts.ts diff --git a/.github/workflows/feedback-loop.yml b/.github/workflows/feedback-loop.yml index 84dfb38..a98837b 100644 --- a/.github/workflows/feedback-loop.yml +++ b/.github/workflows/feedback-loop.yml @@ -19,7 +19,7 @@ jobs: - run: pnpm --filter harness exec playwright install --with-deps chromium - run: pnpm test:e2e - uses: actions/upload-artifact@v4 - if: failure() + if: always() with: name: browser-feedback-loop path: | diff --git a/packages/harness/README.md b/packages/harness/README.md index fec1069..bc42523 100644 --- a/packages/harness/README.md +++ b/packages/harness/README.md @@ -21,3 +21,7 @@ Additional phases: `conflicts`, `needs-approval`, `changes-requested`, `empty`, ## GitHub access boundary `pnpm check:boundaries` parses production TypeScript and checks client dependencies. GitHub/network clients and direct server/CLI requests must stay in `packages/sync`; renderer requests are centralized in `api.ts` and use relative `/api/` URLs. Consumers may import only the public `sync` entry point. Electron's loopback readiness probe and port allocation are explicit exceptions; its updater remains separate from dashboard data access. Tests and the simulator may implement an HTTP server. This is a regression guard, not a security sandbox. It runs as part of lint and CI. + +## Visual review + +Every browser run captures named states (in-flight, queued, queue-failed, outage, recovered, conflicts, approval/review changes, empty lists, partial outage, and rate-limit pause). View PNG attachments in the HTML report, or download the `browser-feedback-loop` Actions artifact, uploaded on successful and failed runs. Screenshots use a pinned viewport, light theme, locale and timezone with animations disabled. These are review artifacts, not pixel-diff assertions; text/structure/state assertions determine correctness. diff --git a/packages/harness/playwright.config.ts b/packages/harness/playwright.config.ts index 2aed8b3..6e62d8d 100644 --- a/packages/harness/playwright.config.ts +++ b/packages/harness/playwright.config.ts @@ -10,6 +10,10 @@ export default defineConfig({ reporter: [["list"], ["html", { open: "never" }]], use: { browserName: "chromium", + colorScheme: "light", + locale: "en-GB", + timezoneId: "UTC", + reducedMotion: "reduce", viewport: { width: 1440, height: 900 }, trace: "retain-on-failure", screenshot: "only-on-failure", diff --git a/packages/harness/src/artifacts.ts b/packages/harness/src/artifacts.ts new file mode 100644 index 0000000..0265f51 --- /dev/null +++ b/packages/harness/src/artifacts.ts @@ -0,0 +1,19 @@ +import type { Page, TestInfo } from "@playwright/test"; + +/** Review evidence, not pixel-diff baselines: viewport/theme are pinned in config. */ +export async function captureState( + page: Page, + testInfo: TestInfo, + name: string, +) { + await page.evaluate(() => document.fonts.ready); + const path = testInfo.outputPath(`${name}.png`); + await page.screenshot({ + path, + fullPage: true, + animations: "disabled", + caret: "hide", + scale: "css", + }); + await testInfo.attach(name, { path, contentType: "image/png" }); +} diff --git a/packages/harness/src/board.e2e.ts b/packages/harness/src/board.e2e.ts index 2879490..f2e3cf9 100644 --- a/packages/harness/src/board.e2e.ts +++ b/packages/harness/src/board.e2e.ts @@ -1,5 +1,6 @@ import { test, expect } from "@playwright/test"; import { startScenario } from "./runtime.js"; +import { captureState } from "./artifacts.js"; import { SCENARIO_TITLE } from "./github.js"; test("polls queue entry, queue failure, and recovery through the real sync engine", async ({ @@ -34,6 +35,7 @@ test("polls queue entry, queue failure, and recovery through the real sync engin card.getByRole("img", { name: "Ready for review", exact: true }), ).toBeVisible(); + await captureState(page, testInfo, "in-flight"); scenario.setPhase("queued"); await scenario.syncAll(); expect(await scenario.persistedPrs()).toMatchObject([ @@ -47,6 +49,7 @@ test("polls queue entry, queue failure, and recovery through the real sync engin await expect(card.getByText("pending", { exact: true })).toBeVisible(); await expect(card).toHaveCount(1); + await captureState(page, testInfo, "queued"); scenario.setPhase("queue-failed"); await scenario.syncAll(); expect(await scenario.persistedPrs()).toMatchObject([ @@ -58,6 +61,7 @@ test("polls queue entry, queue failure, and recovery through the real sync engin await expect(card.getByText("failing", { exact: true })).toBeVisible(); await expect(card).toHaveCount(1); + await captureState(page, testInfo, "queue-failed"); scenario.setPhase("offline"); await expect(scenario.syncAll()).rejects.toThrow("Scenario sync failed"); expect(await scenario.persistedPrs()).toMatchObject([ @@ -70,6 +74,7 @@ test("polls queue entry, queue failure, and recovery through the real sync engin }), ).toBeVisible(); + await captureState(page, testInfo, "upstream-outage"); scenario.setPhase("in-flight"); await scenario.syncAll(); await expect(card.getByText("passing", { exact: true })).toBeVisible(); @@ -78,6 +83,7 @@ test("polls queue entry, queue failure, and recovery through the real sync engin await expect( page.getByLabel("Sync status: Up to date", { exact: true }), ).toBeVisible(); + await captureState(page, testInfo, "recovered"); expect(errors).toEqual([]); expect(apiResponses.length).toBeGreaterThan(3); expect(apiResponses.every((response) => response.status === 200)).toBe( diff --git a/packages/harness/src/states.e2e.ts b/packages/harness/src/states.e2e.ts index 0c732fe..7152099 100644 --- a/packages/harness/src/states.e2e.ts +++ b/packages/harness/src/states.e2e.ts @@ -1,10 +1,11 @@ import { test, expect } from "@playwright/test"; import { startScenario } from "./runtime.js"; +import { captureState } from "./artifacts.js"; import { SCENARIO_TITLE } from "./github.js"; test("renders conflict, review-request membership, approval and empty states", async ({ page, -}) => { +}, testInfo) => { const scenario = await startScenario(); try { for (const phase of [ @@ -42,13 +43,16 @@ test("renders conflict, review-request membership, approval and empty states", a .getByRole("listitem") .filter({ hasText: "Review the storage change" }), ).toHaveCount(phase === "review-requested" ? 1 : 0); + await captureState(page, testInfo, phase); } } finally { await scenario.close(); } }); -test("keeps two instances separate when one fails", async ({ page }) => { +test("keeps two instances separate when one fails", async ({ + page, +}, testInfo) => { const scenario = await startScenario({ instances: 2 }); const errors: string[] = []; page.on("console", (message) => { @@ -73,6 +77,7 @@ test("keeps two instances separate when one fails", async ({ page }) => { exact: true, }), ).toBeVisible(); + await captureState(page, testInfo, "partial-outage"); expect(errors).toEqual([]); } finally { await scenario.close(); @@ -81,7 +86,7 @@ test("keeps two instances separate when one fails", async ({ page }) => { test("shows a rate-limit pause and clears it after the budget resets", async ({ page, -}) => { +}, testInfo) => { const scenario = await startScenario(); try { await scenario.syncPrs(); @@ -96,6 +101,7 @@ test("shows a rate-limit pause and clears it after the budget resets", async ({ await expect(paused).toBeVisible(); await paused.click(); await expect(page.getByText(/Retry after/).first()).toBeVisible(); + await captureState(page, testInfo, "rate-limit-pause"); scenario.setPhase("in-flight"); await expect .poll(