From 8302048bc410d46fb8c8ebf209f37cb7b0f9b597 Mon Sep 17 00:00:00 2001 From: Anso Date: Sun, 7 Jun 2026 14:56:49 -0400 Subject: [PATCH] test(mobile): declare per-view mobile treatment and harden the visual gate (#1332) * test(mobile): single-source map for per-view mobile treatment Declare how every top-level view behaves on a phone in one place: MOBILE_TREATMENTS is a Record, so adding a new view without classifying it (bespoke / responsive / desktop-only / detail) fails the type check. BESPOKE_MOBILE_VIEWS is derived from it instead of hand-maintained, and a unit test keeps the two in lockstep and pins the current bespoke set so a change is deliberate. EditorLayout consumes the derived set; behavior is unchanged. * ci(visual): run the desktop-unchanged gate in its own job Split the visual-regression spec into a dedicated Playwright "visual" project, excluded from the default chromium project the functional E2E job runs, so a missing or platform-mismatched baseline can no longer fail every PR. Add a Visual Regression workflow: a compare job gates PRs into main against committed baselines (and skips with a warning until they are seeded), and a manual seed job regenerates the baselines on the Linux runner and commits them to a feature branch (refusing main). Baselines are platform-specific, so they must be produced on the runner rather than locally. Drop the stack-detail view from the gate: a fresh CI app has no stack to open and its live log stream is not deterministic; the shell plus the four content views still catch a desktop base-class regression. --- .github/workflows/visual-regression.yml | 109 ++++++++++++++++++ e2e/desktop-visual-regression.spec.ts | 31 +++-- frontend/src/components/EditorLayout.tsx | 6 +- .../EditorLayout/mobile-treatments.test.ts | 32 +++++ .../EditorLayout/mobile-treatments.ts | 40 +++++++ playwright.config.ts | 19 ++- 6 files changed, 212 insertions(+), 25 deletions(-) create mode 100644 .github/workflows/visual-regression.yml create mode 100644 frontend/src/components/EditorLayout/mobile-treatments.test.ts create mode 100644 frontend/src/components/EditorLayout/mobile-treatments.ts diff --git a/.github/workflows/visual-regression.yml b/.github/workflows/visual-regression.yml new file mode 100644 index 00000000..a72ab514 --- /dev/null +++ b/.github/workflows/visual-regression.yml @@ -0,0 +1,109 @@ +name: Visual Regression + +# Desktop-unchanged gate for the mobile work: snapshots the touched top-level +# views at desktop widths and fails if a base layout class changed instead of a +# mobile-only override being added. +# +# Baselines are platform-specific (Linux runner fonts differ from a developer's +# OS), so they are generated here and committed, not produced locally. Seed or +# refresh them by running this workflow manually with "update_baselines: true" +# on the target branch (a feature branch, not a protected one); the compare job +# then gates every PR into main against those committed baselines. +on: + pull_request: + branches: [main] + workflow_dispatch: + inputs: + update_baselines: + description: Regenerate and commit the visual baselines on this branch + type: boolean + default: false + +concurrency: + group: visual-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + # Gate PRs into main against the committed baselines. Until baselines exist + # (first adoption), the comparison is skipped with a warning rather than + # failing every PR; once they are committed the gate compares for real. + compare: + name: Compare against baselines + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + timeout-minutes: 15 + steps: + - name: Checkout Code + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + + - name: Check for committed baselines + id: baselines + run: | + if compgen -G "e2e/*-snapshots/*.png" > /dev/null; then + echo "present=true" >> "$GITHUB_OUTPUT" + else + echo "present=false" >> "$GITHUB_OUTPUT" + echo "::warning::No visual baselines committed yet. Run this workflow manually with update_baselines=true on this branch to seed them, then this job will gate for real." + fi + + - name: Start app & install Playwright + if: steps.baselines.outputs.present == 'true' + uses: ./.github/actions/start-app + + - name: Compare visual snapshots + if: steps.baselines.outputs.present == 'true' + run: npx playwright test --project=visual + + - name: Upload diff report on failure + if: failure() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: visual-report + path: | + e2e/report/ + test-results/ + ci-logs/ + retention-days: 7 + + # Manual: regenerate the baselines on the runner platform and commit them to + # the branch the workflow was dispatched on. Refuses to run on main: seed on a + # feature branch, then merge the baselines in through a PR. + seed: + name: Seed / refresh baselines + if: github.event_name == 'workflow_dispatch' && inputs.update_baselines + runs-on: ubuntu-latest + timeout-minutes: 15 + permissions: + contents: write + steps: + - name: Refuse to seed on main + if: github.ref_name == 'main' + run: | + echo "::error::Do not seed baselines on main. Dispatch this workflow on a feature branch, then merge the baselines in via a PR." + exit 1 + + - name: Checkout Code + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 + + - name: Start app & install Playwright + uses: ./.github/actions/start-app + + - name: Regenerate baselines + run: npx playwright test --project=visual --update-snapshots + + - name: Commit baselines + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + # Baselines live under a path that is gitignored for local runs, so + # force-add the ones this run produced. + git add -f 'e2e/*-snapshots/' + if git diff --cached --quiet; then + echo "Baselines unchanged." + else + git commit -m "test(visual): refresh desktop visual-regression baselines" + git push origin "HEAD:${{ github.ref_name }}" + fi diff --git a/e2e/desktop-visual-regression.spec.ts b/e2e/desktop-visual-regression.spec.ts index 6bad62e2..7914358f 100644 --- a/e2e/desktop-visual-regression.spec.ts +++ b/e2e/desktop-visual-regression.spec.ts @@ -1,18 +1,21 @@ // --------------------------------------------------------------------------- -// Zero-desktop-change gate for the mobile work. +// Zero-desktop-change gate. Snapshots the top-level views at desktop widths; +// any pixel diff means a desktop base class changed instead of a mobile-only +// `max-md:` override being added. // -// Snapshots every top-level view the mobile pass touches, at desktop widths. -// Capture the baseline on the pre-change state, then run again after the mobile -// changes: any pixel diff means a desktop base class was edited instead of a -// mobile-only `max-md:` override being added. +// Runs as its own Playwright project so it never blocks the functional E2E run: +// npx playwright test --project=visual # compare +// npx playwright test --project=visual --update-snapshots # (re)baseline // -// # baseline (before the mobile changes): -// E2E_PASSWORD=admin123 npx playwright test desktop-visual-regression --update-snapshots -// # gate (after the changes): -// E2E_PASSWORD=admin123 npx playwright test desktop-visual-regression +// In CI it runs in the Visual Regression workflow against a fresh app (empty +// COMPOSE_DIR, so no stacks). Baselines are platform-specific, so they are +// generated and committed on the Linux runner via that workflow's seed job, not +// locally. Locally, baseline against your own instance with the same command. // -// Snapshots are platform-specific and gitignored; regenerate the baseline in -// the same environment (or a pinned CI/Docker image) you run the gate in. +// stack-detail is intentionally not snapshotted: a fresh CI app has no stack to +// open, and its live log stream is not deterministic. Resources is omitted for +// the same async-Docker-height reason. The shell plus the four content views +// below catch a desktop base-class regression on the mobile-touched surfaces. // --------------------------------------------------------------------------- import { test, expect, type Page } from '@playwright/test'; import { loginAs, waitForStacksLoaded } from './helpers'; @@ -42,12 +45,6 @@ const VIEWS: View[] = [ await p.getByRole('button', { name: 'Settings', exact: true }).click(); }, }, - { - id: 'stack-detail', label: 'stack-detail', open: async (p) => { - await p.locator('[data-stacks-loaded="true"] [data-testid="stack-row"]').first().click(); - await p.getByText('image', { exact: false }).first().waitFor({ timeout: 10_000 }).catch(() => {}); - }, - }, ]; // Paint over genuinely non-deterministic content so it does not cause false diff --git a/frontend/src/components/EditorLayout.tsx b/frontend/src/components/EditorLayout.tsx index 3d24ea64..e46023d9 100644 --- a/frontend/src/components/EditorLayout.tsx +++ b/frontend/src/components/EditorLayout.tsx @@ -46,13 +46,9 @@ import { MobileFleet } from './mobile/MobileFleet'; import { MobileSchedules } from './mobile/MobileSchedules'; import { MobileSettings } from './mobile/MobileSettings'; import { deriveMobileSurface, type MobileView } from './EditorLayout/mobile-surface'; +import { BESPOKE_MOBILE_VIEWS } from './EditorLayout/mobile-treatments'; import type { SectionId } from './settings/types'; -// Content views that render a bespoke, masthead-led mobile screen instead of the -// reflowed desktop workspace. For these the global TopBar is dropped on mobile -// (each screen's masthead leads). The set grows as screens are re-skinned. -const BESPOKE_MOBILE_VIEWS = new Set(['dashboard', 'fleet', 'scheduled-ops', 'settings']); - export default function EditorLayout() { const { isAdmin, can } = useAuth(); const { status: trivy } = useTrivyStatus(); diff --git a/frontend/src/components/EditorLayout/mobile-treatments.test.ts b/frontend/src/components/EditorLayout/mobile-treatments.test.ts new file mode 100644 index 00000000..d593bb92 --- /dev/null +++ b/frontend/src/components/EditorLayout/mobile-treatments.test.ts @@ -0,0 +1,32 @@ +import { describe, it, expect } from 'vitest'; +import { MOBILE_TREATMENTS, BESPOKE_MOBILE_VIEWS, type MobileTreatment } from './mobile-treatments'; + +const VALID: MobileTreatment[] = ['bespoke', 'responsive', 'desktop-only', 'detail']; + +describe('mobile treatments', () => { + it('classifies every view with a known treatment', () => { + // The Record type already forces every view to + // be present at compile time; this guards the values at runtime so a typo or + // a bad merge cannot leave an unknown treatment in the map. + for (const [view, treatment] of Object.entries(MOBILE_TREATMENTS)) { + expect(VALID, `${view} has an unknown treatment "${treatment}"`).toContain(treatment); + } + }); + + it('keeps BESPOKE_MOBILE_VIEWS in lockstep with the bespoke treatments', () => { + const declaredBespoke = Object.entries(MOBILE_TREATMENTS) + .filter(([, treatment]) => treatment === 'bespoke') + .map(([view]) => view) + .sort(); + expect([...BESPOKE_MOBILE_VIEWS].sort()).toEqual(declaredBespoke); + }); + + it('pins the set of bespoke phone screens (update deliberately when adding one)', () => { + // A change here means a top-level view gained or lost a bespoke phone screen. + // Updating this list should go hand in hand with adding the screen under + // components/mobile/ and wiring its case in EditorLayout's renderMobileBespoke. + expect([...BESPOKE_MOBILE_VIEWS].sort()).toEqual( + ['dashboard', 'fleet', 'scheduled-ops', 'settings'], + ); + }); +}); diff --git a/frontend/src/components/EditorLayout/mobile-treatments.ts b/frontend/src/components/EditorLayout/mobile-treatments.ts new file mode 100644 index 00000000..01b580ea --- /dev/null +++ b/frontend/src/components/EditorLayout/mobile-treatments.ts @@ -0,0 +1,40 @@ +import type { ActiveView } from './hooks/useViewNavigationState'; + +// How a top-level view behaves below the md breakpoint. +// bespoke a dedicated phone screen (masthead-led, in components/mobile/) +// responsive the desktop view reflowed via max-md: utilities, no bespoke layout +// desktop-only a heavy authoring/terminal surface that stays desktop-first +// detail the full-screen stack-detail surface (the editor view) +export type MobileTreatment = 'bespoke' | 'responsive' | 'desktop-only' | 'detail'; + +// Single source of truth for the mobile treatment of every top-level view. +// +// The `Record` shape is the guard: adding a new `ActiveView` +// without classifying it here fails `tsc`, so no view can ship without a +// deliberate decision about its phone behavior. `mobile-treatments.test.ts` +// ties the 'bespoke' entries to `BESPOKE_MOBILE_VIEWS` and to the bespoke +// screens actually wired in EditorLayout, so the declaration cannot drift from +// the implementation. +export const MOBILE_TREATMENTS: Record = { + dashboard: 'bespoke', + fleet: 'bespoke', + 'scheduled-ops': 'bespoke', + settings: 'bespoke', + editor: 'detail', + resources: 'responsive', + templates: 'responsive', + 'global-observability': 'responsive', + 'auto-updates': 'responsive', + 'audit-log': 'responsive', + 'host-console': 'desktop-only', +}; + +// The content surfaces that render a bespoke phone screen instead of the +// reflowed desktop workspace. Derived from MOBILE_TREATMENTS so it cannot drift +// from the declared treatments. EditorLayout drops the global TopBar for these +// and renders their masthead-led screen. +export const BESPOKE_MOBILE_VIEWS: ReadonlySet = new Set( + (Object.keys(MOBILE_TREATMENTS) as ActiveView[]).filter( + view => MOBILE_TREATMENTS[view] === 'bespoke', + ), +); diff --git a/playwright.config.ts b/playwright.config.ts index 39d29a34..404983c5 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -31,11 +31,24 @@ export default defineConfig({ projects: [ { - // Default project: skips the manual screenshot capture spec so - // `npx playwright test` does not regenerate docs images on every run. + // Default project (what CI runs via --project=chromium). Skips the manual + // screenshot capture spec and the visual-regression gate: the gate needs + // committed, platform-matched baselines and runs in its own job, so it + // must not fail the functional E2E run on a missing snapshot. name: 'chromium', use: { ...devices['Desktop Chrome'] }, - testIgnore: ['**/screenshots.spec.ts'], + testIgnore: ['**/screenshots.spec.ts', '**/desktop-visual-regression.spec.ts'], + }, + { + // Desktop-unchanged visual gate. Run explicitly (and in the dedicated + // Visual Regression workflow): + // npx playwright test --project=visual + // Baselines are platform-specific; regenerate with --update-snapshots in + // the same environment you compare in (locally, or the workflow's + // Playwright container for the CI baselines). + name: 'visual', + use: { ...devices['Desktop Chrome'] }, + testMatch: ['**/desktop-visual-regression.spec.ts'], }, { // Manual-only project for capturing docs/images/. Run explicitly: