Visual regression test cleanup (#2933)

* clean up visual regression stuff

* use a worktree for visual baseline, no stashing

* same treatment on compare script

* actually just delete the compare script

* fix spurious time differences by setting fixed time

* update safety test about e2e test file locations

Author: david-crespo

.gitignore

@@ -33,5 +33,5 @@ playwright-report/
 .vercel
 
 # Visual regression snapshots
-test/e2e/**/*-snapshots/
-test/e2e/**/*.png
+test/visual/**/*-snapshots/
+test/visual/**/*.png

AGENTS.md

@@ -27,7 +27,7 @@
 - Cover role-gated flows by logging in with `getPageAsUser`; exercise negative paths (e.g., forbidden actions) alongside happy paths as shown in `test/e2e/system-update.e2e.ts`.
 - When UI needs new mock behavior, extend the MSW handlers/db minimally so E2E tests stay deterministic; prefer storing full API responses so subsequent calls see the updated state (`mock-api/msw/db.ts`, `mock-api/msw/handlers.ts`).
 - Co-locate Vitest specs next to the code they cover; use Testing Library utilities (`render`, `renderHook`, `fireEvent`, fake timers) to assert observable output rather than implementation details (`app/ui/lib/FileInput.spec.tsx`, `app/hooks/use-pagination.spec.ts`).
-- For sweeping styling changes, coordinate with the visual regression harness and follow `test/e2e/VISUAL-REGRESSION.md` for the workflow.
+- For sweeping styling changes, coordinate with the visual regression harness and follow `test/visual/README.md` for the workflow.
 
 # Data fetching pattern
 

app/api/__tests__/safety.spec.ts

@@ -74,8 +74,8 @@ const listFiles = (s: string) =>
   execSync(`git ls-files | grep "${s}"`).toString().trim().split('\n')
 
 // avoid accidentally making an e2e file in the wrong place
-it('e2e tests are only in test/e2e', () => {
+it('e2e tests are only in test/e2e or test/visual', () => {
   for (const file of listFiles('\\.e2e\\.')) {
-    expect(file).toMatch(/^test\/e2e/)
+    expect(file).toMatch(/^test\/(e2e|visual)/)
   }
 })

package.json

@@ -23,7 +23,7 @@
     "e2e": "playwright test",
     "e2ec": "playwright test --project=chrome",
     "visual:baseline": "./tools/generate-visual-baseline.sh",
-    "visual:compare": "./tools/compare-visual-changes.sh",
+    "visual:compare": "playwright test -c playwright.visual.config.ts",
     "lint": "oxlint --type-aware && eslint --ext .js,.ts,.tsx app test mock-api",
     "oxlint": "oxlint --type-aware",
     "fmt": "prettier --cache --write . && npm run lint -- --fix",

playwright.config.ts

@@ -24,8 +24,6 @@ export default {
   // default is 5 seconds. somehow playwright really hates async route modules,
   // takes a long time to load them. https://playwright.dev/docs/test-timeouts
   expect: { timeout: 10_000 },
-  // Skip visual regression tests in CI
-  grep: process.env.CI ? /^(?!.*@visual)/ : undefined,
   use: {
     trace: process.env.CI ? 'on-first-retry' : 'retain-on-failure',
     baseURL: 'http://localhost:4009',

playwright.visual.config.ts

@@ -0,0 +1,32 @@
+/*
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, you can obtain one at https://mozilla.org/MPL/2.0/.
+ *
+ * Copyright Oxide Computer Company
+ */
+
+import { devices, type PlaywrightTestConfig } from '@playwright/test'
+
+// the regression test stuff is not run in CI and only needs one browser
+
+export default {
+  testDir: './test/visual',
+  testMatch: /\.e2e\.ts/,
+  fullyParallel: true,
+  workers: '66%',
+  use: {
+    baseURL: 'http://localhost:4009',
+  },
+  projects: [
+    {
+      name: 'chrome',
+      use: { ...devices['Desktop Chrome'] },
+    },
+  ],
+  // use different port so it doesn't conflict with local dev server
+  webServer: {
+    command: 'npm run start:msw -- --port 4009',
+    port: 4009,
+  },
+} satisfies PlaywrightTestConfig

test/e2e/VISUAL-REGRESSION.md renamed to test/visual/README.md

@@ -17,13 +17,7 @@ npm run visual:baseline -- abc123
 npm run visual:baseline -- HEAD~1
 ```
 
-This will:
-
-1. Fetch the latest main branch (if using `--main`)
-2. Temporarily checkout the specified commit
-3. Install dependencies
-4. Run all E2E tests with `--update-snapshots` to generate baseline images
-5. Restore your working directory to its previous state
+This will check out that revision in a git work tree, run the visual snapshot tests, and copy the snapshots back into your current working directory.
 
 ### 2. Compare Current Changes
 
@@ -60,7 +54,7 @@ If differences are detected:
 If the visual changes are intentional and correct:
 
 ```bash
-npm run visual:compare -- --update
+npm run visual:compare -- --update-snapshots
 ```
 
 This will update the baseline snapshots to match the current state.

test/e2e/visual-regression.e2e.ts renamed to test/visual/regression.e2e.ts

@@ -14,92 +14,99 @@
  * CSS frameworks, or making broad styling changes.
  */
 
-import { expect, test } from './utils'
+import { expect, test } from '../e2e/utils'
+
+// set a fixed time to avoid diffs due to irrelevant time differences
+test.beforeEach(async ({ page }) => {
+  await page.clock.setFixedTime(new Date('2025-10-23T12:34:56.000Z'))
+})
 
 test.describe('Visual Regression', { tag: '@visual' }, () => {
   test('projects list', async ({ page }) => {
     await page.goto('/projects')
-    await page.getByRole('heading', { name: 'Projects' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'Projects' })).toBeVisible()
     await expect(page).toHaveScreenshot('projects-list.png', { fullPage: true })
   })
 
   test('instances list', async ({ page }) => {
     await page.goto('/projects/mock-project/instances')
-    await page.getByRole('heading', { name: 'Instances' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'Instances' })).toBeVisible()
     await expect(page).toHaveScreenshot('instances-list.png', { fullPage: true })
   })
 
   test('instance detail', async ({ page }) => {
     await page.goto('/projects/mock-project/instances/db1')
-    await page.getByRole('heading', { name: 'db1' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'db1' })).toBeVisible()
     await expect(page).toHaveScreenshot('instance-detail.png', { fullPage: true })
   })
 
   test('create disk', async ({ page }) => {
     await page.goto('/projects/mock-project/disks-new')
-    await page.getByRole('heading', { name: 'Create disk' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'Create disk' })).toBeVisible()
     await expect(page).toHaveScreenshot('disks-new.png', { fullPage: true })
   })
 
   test('disks list', async ({ page }) => {
     await page.goto('/projects/mock-project/disks')
-    await page.getByRole('heading', { name: 'Disks' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'Disks' })).toBeVisible()
     await expect(page).toHaveScreenshot('disks-list.png', { fullPage: true })
   })
 
   test('vpcs list', async ({ page }) => {
     await page.goto('/projects/mock-project/vpcs')
-    await page.getByRole('heading', { name: 'VPCs' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'VPCs' })).toBeVisible()
     await expect(page).toHaveScreenshot('vpcs-list.png', { fullPage: true })
   })
 
   test('snapshots list', async ({ page }) => {
     await page.goto('/projects/mock-project/snapshots')
-    await page.getByRole('heading', { name: 'Snapshots' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'Snapshots' })).toBeVisible()
     await expect(page).toHaveScreenshot('snapshots-list.png', { fullPage: true })
   })
 
   test('images list', async ({ page }) => {
     await page.goto('/projects/mock-project/images')
-    await page.getByRole('heading', { name: 'Images' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'Images' })).toBeVisible()
     await expect(page).toHaveScreenshot('images-list.png', { fullPage: true })
   })
 
   test('silo images list', async ({ page }) => {
     await page.goto('/images')
-    await page.getByRole('heading', { name: 'Silo Images' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'Silo Images' })).toBeVisible()
     await expect(page).toHaveScreenshot('silo-images.png', { fullPage: true })
     await page.click('role=button[name="Promote image"]')
     await expect(page).toHaveScreenshot('silo-images-promote.png', { fullPage: true })
   })
 
   test('silo image', async ({ page }) => {
     await page.goto('/images/arch-2022-06-01/edit')
-    await page.getByRole('heading', { name: 'Silo image', exact: true }).waitFor()
+    await expect(
+      page.getByRole('heading', { name: 'Silo image', exact: true })
+    ).toBeVisible()
     await expect(page).toHaveScreenshot('silo-image.png', { fullPage: true })
   })
 
   test('system utilization', async ({ page }) => {
     await page.goto('/utilization')
-    await page.getByRole('heading', { name: 'Utilization' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'Utilization' })).toBeVisible()
     await expect(page).toHaveScreenshot('system-utilization.png', { fullPage: true })
   })
 
   test('system silos list', async ({ page }) => {
     await page.goto('/system/silos')
-    await page.getByRole('heading', { name: 'Silos' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'Silos' })).toBeVisible()
     await expect(page).toHaveScreenshot('system-silos.png', { fullPage: true })
   })
 
   test('system networking ip pools', async ({ page }) => {
     await page.goto('/system/networking/ip-pools')
-    await page.getByRole('heading', { name: 'IP Pools' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'IP Pools' })).toBeVisible()
     await expect(page).toHaveScreenshot('system-ip-pools.png', { fullPage: true })
   })
 
   test('settings profile', async ({ page }) => {
     await page.goto('/settings/profile')
-    await page.getByRole('heading', { name: 'Profile' }).waitFor()
+    await expect(page.getByRole('heading', { name: 'Profile' })).toBeVisible()
     await expect(page).toHaveScreenshot('settings-profile.png', { fullPage: true })
   })