tests: faster local e2e runs, fix three CI flakes

.claude/skills/debug-e2e/SKILL.md

@@ -69,7 +69,7 @@ Prefer narrowing with `-g` and `--project` first, then increase `--repeat-each`
 
 ### 3. Analyze failure artifacts
 
-When tests fail, Playwright saves traces and error context:
+When tests fail, Playwright saves error context. Traces are off by default in local runs because recording them slows the suite; add `--trace retain-on-failure` to the test command to keep a trace for each failed test.
 
 ```bash
 # View error context (page snapshot at failure time)

AGENTS.md

@@ -47,7 +47,7 @@
 - Treat Vitest browser specs as small e2e tests: query by accessible role, label, or visible text and use retrying browser matchers. Avoid selectors coupled to CSS classes or internal DOM structure; inspect layout or computed styles only when the behavior has no semantic representation.
 - For sweeping styling changes, coordinate with the visual regression harness and follow `test/visual/README.md` for the workflow.
 - Fix root causes of flaky timing rather than adding `sleep()` workarounds in tests.
-- When an intermittent browser or E2E failure does not reproduce under normal repetition, retry under controlled CPU contention before concluding it is CI-only. Keep the load bounded so the browser and test runner can still make progress, enable failure-only traces when available, and use a shell trap to clean up every churn process.
+- When an intermittent browser or E2E failure does not reproduce under normal repetition, retry under controlled CPU contention before concluding it is CI-only. Keep the load bounded so the browser and test runner can still make progress, enable failure-only traces (`--trace retain-on-failure`, off by default locally), and use a shell trap to clean up every churn process.
 - Local Playwright runs write a compact plain-text report to `.e2e-logs/` (gitignored, one timestamped `.log` per run, last 10 kept) via the custom reporter at `test/e2e/compact-reporter.ts`. Top line is `status: ... total=N passed=N ...`; each failure is a `── UNEXPECTED|FLAKY file:line title` block followed by the error (ANSI stripped). Latest run: `ls .e2e-logs | tail -1` — Read it directly, no parsing needed.
 
 # Data fetching pattern

playwright.config.ts

@@ -33,7 +33,10 @@ export default {
   // an LLM agent can read the failures.
   reporter: process.env.CI ? 'list' : [['list'], ['./test/e2e/compact-reporter.ts']],
   use: {
-    trace: process.env.CI ? 'on-first-retry' : 'retain-on-failure',
+    // Recording a trace for every test makes local runs much slower, so it's
+    // off by default. Pass `--trace retain-on-failure` to keep traces of
+    // failed tests, e.g. when chasing a flake.
+    trace: process.env.CI ? 'on-first-retry' : 'off',
     baseURL: 'http://localhost:4009',
   },
   projects: [

test/e2e/route-announcer.e2e.ts

@@ -7,22 +7,46 @@
  */
 import { expect, test, type Page } from '@playwright/test'
 
+declare global {
+  interface Window {
+    announcements: string[]
+  }
+}
+
 // react-aria appends each announcement as a child of a hidden polite live
-// region shared with toasts. CSS locator because there's a second region for
+// region shared with toasts, and removes it 7 seconds later. A slow CI runner
+// can take longer than that to get to an assertion, so record announcements as
+// they're added. Match on the polite region because there's a second one for
 // assertive announcements and we only want the messages from this one.
-const announcements = (page: Page) =>
-  page.locator('[data-live-announcer] [aria-live="polite"] > div')
+test.beforeEach(async ({ page }) => {
+  await page.addInitScript(() => {
+    window.announcements = []
+    new MutationObserver((records) => {
+      for (const { target, addedNodes } of records) {
+        if (
+          target instanceof Element &&
+          target.matches('[data-live-announcer] [aria-live="polite"]')
+        ) {
+          for (const node of addedNodes) window.announcements.push(node.textContent ?? '')
+        }
+      }
+    }).observe(document, { childList: true, subtree: true })
+  })
+})
+
+const expectAnnouncements = (page: Page) =>
+  expect.poll(() => page.evaluate(() => window.announcements))
 
 test('route announcer', async ({ page }) => {
   await page.goto('/projects')
   await expect(page.getByRole('heading', { name: 'Projects' })).toBeVisible()
 
   // nothing on first load — the browser already announced the page
-  await expect(announcements(page)).toHaveCount(0)
+  await expectAnnouncements(page).toEqual([])
 
   // nav by a link inside the content, which unmounts along with the page
   await page.getByRole('link', { name: 'mock-project' }).click()
-  await expect(announcements(page)).toHaveText(['Instances, mock-project, Projects'])
+  await expectAnnouncements(page).toEqual(['Instances, mock-project, Projects'])
   // focus goes to the new page's heading, like (better than) a real page load
   await expect(page.getByRole('heading', { name: 'Instances' })).toBeFocused()
 
@@ -32,7 +56,7 @@ test('route announcer', async ({ page }) => {
     .getByRole('navigation', { name: 'Sidebar navigation' })
     .getByRole('link', { name: 'Disks' })
   await disksLink.click()
-  await expect(announcements(page)).toHaveText([
+  await expectAnnouncements(page).toEqual([
     'Instances, mock-project, Projects',
     'Disks, mock-project, Projects',
   ])
@@ -42,12 +66,12 @@ test('route announcer', async ({ page }) => {
   // than replacing it, so it doesn't announce or take focus from the dialog
   await page.getByRole('link', { name: 'New Disk' }).click()
   await expect(page.getByRole('dialog', { name: 'Create disk' })).toBeVisible()
-  await expect(announcements(page)).toHaveCount(2)
+  await expectAnnouncements(page).toHaveLength(2)
 
   // ...and neither does dismissing it
   await page.getByRole('button', { name: 'Cancel' }).click()
   await expect(page.getByRole('dialog', { name: 'Create disk' })).toBeHidden()
-  await expect(announcements(page)).toHaveCount(2)
+  await expectAnnouncements(page).toHaveLength(2)
 })
 
 // tabs that live in the query param rather than the path aren't navigations as
@@ -56,7 +80,7 @@ test('route announcer ignores query param tabs', async ({ page }) => {
   await page.goto('/system/utilization')
   await page.getByRole('tab', { name: 'Metrics' }).click()
   await expect(page).toHaveURL(/tab=metrics/)
-  await expect(announcements(page)).toHaveCount(0)
+  await expectAnnouncements(page).toEqual([])
 })
 
 // VPC tabs, on the other hand, are real routes. Routes like these share a crumb
@@ -65,7 +89,7 @@ test('route announcer on tab routes', async ({ page }) => {
   await page.goto('/projects/mock-project/vpcs/default/firewall-rules')
   const tab = page.getByRole('tab', { name: 'Routers' })
   await tab.click()
-  await expect(announcements(page)).toHaveText([
+  await expectAnnouncements(page).toEqual([
     'Routers, default, VPCs, mock-project, Projects',
   ])
   // the tab is still there, so it keeps focus
@@ -78,5 +102,5 @@ test('route announcer ignores detail side modals', async ({ page }) => {
   await page.goto('/images')
   await page.getByRole('link', { name: 'ubuntu-22-04' }).click()
   await expect(page.getByRole('dialog', { name: 'Image details' })).toBeVisible()
-  await expect(announcements(page)).toHaveCount(0)
+  await expectAnnouncements(page).toEqual([])
 })

test/e2e/utils.ts

@@ -179,11 +179,20 @@ export async function stopInstance(page: Page) {
 }
 
 /**
- * Assert that a toast with text matching `expectedText` is visible.
+ * Assert that a toast with text matching `expectedText` is visible, then
+ * dismiss that toast and wait for it to go away.
  */
 export async function expectToast(page: Page, expectedText: string | RegExp) {
   await expect(page.getByTestId('Toasts')).toContainText(expectedText)
-  await closeToast(page)
+  // Dismiss the toast we matched, not whichever is first. An error test often
+  // has a "creating..." toast followed closely by a result toast, and which one
+  // is first depends on how fast the mock answers.
+  const toast = page
+    .getByTestId('Toasts')
+    .locator(':scope > div')
+    .filter({ hasText: expectedText })
+    .first()
+  await dismissToast(toast.getByRole('button', { name: 'Dismiss notification' }))
 }
 
 /**
@@ -193,18 +202,25 @@ export async function expectNoToast(page: Page, expectedText: string | RegExp) {
   await expect(page.getByTestId('Toasts')).not.toContainText(expectedText)
 }
 
+/** Close the first toast and wait for it to go away. */
+export async function closeToast(page: Page) {
+  await dismissToast(page.getByRole('button', { name: 'Dismiss notification' }).first())
+}
+
 /**
- * Close first toast and wait for it to fade out. For some reason it prevents
- * things from working, but only in tests as far as we can tell.
+ * Click a toast's dismiss button and wait until the toast has left the DOM
+ * (after its exit animation), so it can't cover what the test clicks next.
+ * Hold the element, not the locator: a toast arriving in between would change
+ * what the locator points to.
  */
-export async function closeToast(page: Page) {
-  // first() is a hack aimed at situations where we're testing an error
-  // response, which usually means we have an initial "creating..." toast
-  // followed by an error toast. Sometimes the error toast shows up so fast that
-  // we don't have time to close the first one. Without first(), this errors out
-  // because there are two toasts.
-  await page.getByRole('button', { name: 'Dismiss notification' }).first().click()
-  await sleep(1000)
+async function dismissToast(button: Locator) {
+  // elementHandle waits for a match and throws on timeout, so it's never null
+  const handle = (await button.elementHandle())!
+  await handle.click().catch(async (e) => {
+    // the toast timed out and left on its own before the click landed
+    if (await handle.evaluate((el) => el.isConnected)) throw e
+  })
+  await handle.waitForElementState('hidden')
 }
 
 /**

test/e2e/vpcs.e2e.ts

@@ -13,7 +13,9 @@ test('can nav to VpcPage from /', async ({ page }) => {
   await page.goto('/')
   await page.getByRole('table').getByRole('link', { name: 'mock-project' }).click()
   await page.waitForURL('**/projects/mock-project/**')
-  await page.getByRole('link', { name: 'VPCs' }).click()
+  // exact because the URL changes before the projects table goes away, and on
+  // a slow machine a substring match clicks the project-no-vpcs link in it
+  await page.getByRole('link', { name: 'VPCs', exact: true }).click()
   await page.waitForURL('**/vpcs**')
 
   await expectRowVisible(page.getByRole('table'), {