From c9b0a65ce9a221521f9dfd5cd28666da9b8c570f Mon Sep 17 00:00:00 2001 From: d-oit Date: Thu, 24 Sep 2026 15:06:51 +0200 Subject: [PATCH] test(e2e): wait for app readiness inside the navigation helpers Amp-Thread-ID: https://ampcode.com/threads/T-01a0cf2b-5969-7627-bccd-702e8d30adda Co-authored-by: Amp --- e2e/helpers/navigation.ts | 49 ++++++++++--------- e2e/responsive.spec.ts | 15 ++++++ e2e/right-panel.spec.ts | 4 ++ ...49-nightly-full-viewport-e2e-2026-09-24.md | 22 +++++++-- 4 files changed, 63 insertions(+), 27 deletions(-) diff --git a/e2e/helpers/navigation.ts b/e2e/helpers/navigation.ts index ac7b55c5..ca1b3efc 100644 --- a/e2e/helpers/navigation.ts +++ b/e2e/helpers/navigation.ts @@ -1,20 +1,42 @@ import { expect, type Page } from '@playwright/test'; +/** + * Wait until the app shell has mounted and its global listeners are bound. + * + * Waits on `data-app-ready`, which AppShell sets from its own mount effect. React + * flushes child effects before parent effects, so when the attribute appears + * every descendant listener — including CommandPalette's window-level Ctrl+K + * handler and React's delegated click handlers — has already been bound. An + * interaction issued before that is simply lost, with nothing to retry it. + * + * Neither `networkidle` nor the `
` landmark can stand in for this: the + * shell renders `
` unconditionally, so a server-rendered DOM satisfies both + * before hydration (plans/149 §4). + * + * Declared first: the helpers below call it, and module-scope const arrows must + * be defined before use (temporal dead zone). + */ +export const waitForAppReady = async (page: Page): Promise => { + await expect(page.locator('[data-app-ready="true"]')).toBeAttached(); +}; + /** * Open the mobile drawer when the persistent sidebar is hidden (mobile and * tablet viewports). No-op on desktop where the sidebar is always visible. * After this resolves, `getByRole('navigation', { name: /main navigation/i })` * matches the visible navigation on every viewport. * - * Defined before `navClick` because `navClick` calls it — module-scope const - * arrows must be declared before use (temporal dead zone). + * Waits for the shell first: the sidebar and the "Open menu" trigger are both + * server-rendered, so clicking either before hydration does nothing and the + * caller then fails on a later assertion (plans/149 §5.2). */ export const openNavIfHidden = async (page: Page): Promise => { + await waitForAppReady(page); const nav = page.getByRole('navigation', { name: /main navigation/i }); if (await nav.isVisible()) return; await page.getByRole('button', { name: /open menu/i }).first().click(); await expect(page.getByRole('navigation', { name: /main navigation/i })).toBeVisible(); -} +}; /** * Click a sidebar navigation button by label, scoped to the main navigation. @@ -35,7 +57,7 @@ export const navClick = async (page: Page, name: RegExp | string): Promise .getByRole('button', { name }) .first() .click(); -} +}; /** * Assert that primary navigation is reachable: the persistent sidebar on @@ -49,21 +71,4 @@ export const expectNavigationReachable = async (page: Page): Promise => { } else { await expect(page.getByRole('button', { name: /open menu/i })).toBeVisible(); } -} - -/** - * Wait until the app shell has mounted and its global listeners are bound. - * - * Waits on `data-app-ready`, which AppShell sets from its own mount effect. React - * flushes child effects before parent effects, so when the attribute appears - * every descendant listener — including CommandPalette's window-level Ctrl+K - * handler — has already been bound. A keypress issued before that is simply lost, - * with nothing to retry it. - * - * Neither `networkidle` nor the `
` landmark can stand in for this: the - * shell renders `
` unconditionally, so a server-rendered DOM satisfies both - * before hydration (plans/149 §4). - */ -export const waitForAppReady = async (page: Page): Promise => { - await expect(page.locator('[data-app-ready="true"]')).toBeAttached(); -} +}; diff --git a/e2e/responsive.spec.ts b/e2e/responsive.spec.ts index a72e1178..114d7d3e 100644 --- a/e2e/responsive.spec.ts +++ b/e2e/responsive.spec.ts @@ -1,9 +1,12 @@ import { test, expect } from '@playwright/test'; +import { waitForAppReady } from './helpers/navigation'; test.describe('Responsive behavior', () => { test('desktop: sidebar is visible', async ({ page }) => { await page.setViewportSize({ width: 1280, height: 800 }); await page.goto('/'); + // Server-rendered controls: a click before hydration is lost (plans/149 §5.2). + await waitForAppReady(page); const sidebar = page.getByRole('navigation', { name: /main navigation/i }); await expect(sidebar).toBeVisible(); @@ -12,6 +15,8 @@ test.describe('Responsive behavior', () => { test('tablet: sidebar is hidden, mobile drawer available', async ({ page }) => { await page.setViewportSize({ width: 768, height: 1024 }); await page.goto('/'); + // Server-rendered controls: a click before hydration is lost (plans/149 §5.2). + await waitForAppReady(page); // Desktop sidebar should be hidden at tablet size // The mobile drawer hamburger should be visible @@ -27,6 +32,8 @@ test.describe('Responsive behavior', () => { test('mobile: layout adapts to small viewport', async ({ page }) => { await page.setViewportSize({ width: 375, height: 667 }); await page.goto('/'); + // Server-rendered controls: a click before hydration is lost (plans/149 §5.2). + await waitForAppReady(page); // Page should still be functional await expect(page).toHaveTitle(/DO Knowledge Studio/); @@ -41,6 +48,8 @@ test.describe('Responsive behavior', () => { test('desktop: three-pane layout at wide viewport', async ({ page }) => { await page.setViewportSize({ width: 1440, height: 900 }); await page.goto('/'); + // Server-rendered controls: a click before hydration is lost (plans/149 §5.2). + await waitForAppReady(page); // At wide viewport, the right panel may be visible await expect(page).toHaveTitle(/DO Knowledge Studio/); @@ -49,6 +58,8 @@ test.describe('Responsive behavior', () => { test('larger screen (1920px): layout scales without horizontal overflow', async ({ page }) => { await page.setViewportSize({ width: 1920, height: 1080 }); await page.goto('/'); + // Server-rendered controls: a click before hydration is lost (plans/149 §5.2). + await waitForAppReady(page); const sidebar = page.getByRole('navigation', { name: /main navigation/i }); await expect(sidebar).toBeVisible(); @@ -66,6 +77,8 @@ test.describe('Responsive behavior', () => { test('larger screen (1920px): library grid shows multiple columns', async ({ page }) => { await page.setViewportSize({ width: 1920, height: 1080 }); await page.goto('/'); + // Server-rendered controls: a click before hydration is lost (plans/149 §5.2). + await waitForAppReady(page); const nav = page.getByRole('navigation', { name: /main navigation/i }); await nav.getByRole('button', { name: /library/i }).first().click(); @@ -77,6 +90,8 @@ test.describe('Responsive behavior', () => { test('viewport resize does not break layout', async ({ page }) => { await page.goto('/'); + // Server-rendered controls: a click before hydration is lost (plans/149 §5.2). + await waitForAppReady(page); await expect(page).toHaveTitle(/DO Knowledge Studio/); // Resize through breakpoints diff --git a/e2e/right-panel.spec.ts b/e2e/right-panel.spec.ts index d06aae4e..15256d89 100644 --- a/e2e/right-panel.spec.ts +++ b/e2e/right-panel.spec.ts @@ -1,4 +1,5 @@ import { expect, test } from '@playwright/test' +import { waitForAppReady } from './helpers/navigation' // Matches `--breakpoint-wide` in src/app/globals.css. Below it the right panel is // `hidden wide:flex`, so its close control is intentionally absent and the sidebar @@ -8,6 +9,9 @@ const WIDE_BREAKPOINT_PX = 1100 test.describe('Right panel', () => { test.beforeEach(async ({ page }) => { await page.goto('/') + // The panel and its close control are server-rendered; a click before + // hydration is lost (plans/149 §5.2). + await waitForAppReady(page) }) test('close control dismisses Search and exposes the panel toggle', async ({ page }) => { diff --git a/plans/149-nightly-full-viewport-e2e-2026-09-24.md b/plans/149-nightly-full-viewport-e2e-2026-09-24.md index b85c4578..daa34f43 100644 --- a/plans/149-nightly-full-viewport-e2e-2026-09-24.md +++ b/plans/149-nightly-full-viewport-e2e-2026-09-24.md @@ -185,11 +185,23 @@ have used. 1. **The next real nightly should be confirmed.** The dispatch run proves the mechanism; the 03:00 UTC schedule run is the last piece. If it reports `E2E Tests: skipped` again, the cause is a dependency this plan did not see. -2. **Pre-hydration interactions are now observable, but not everywhere.** The - shell exposes `data-app-ready`, so any spec that interacts before hydration can - call `waitForAppReady`. The specs that click server-rendered controls straight - after `goto` have not been audited — worth doing when a click-order flake - actually shows up, rather than pre-emptively across 24 specs. +2. **Pre-hydration interaction audit — closed at the helpers (2026-09-24).** The + exposure was measured across all 24 specs rather than patched per spec: + - `openNavIfHidden` (and therefore `navClick`) now waits for `data-app-ready` + first. That covers the 22 specs that navigate through the helpers — + `crud-workflow`, `home`, `timeline`, `progressive-disclosure` and + `accessibility` all have `navClick(...)` as their first action, so no per-spec + edit was needed. + - `responsive.spec.ts` waits after each of its seven `goto` calls. It sets the + viewport *before* navigating, so a `beforeEach` wait would run against + `about:blank` and time out — worth remembering for viewport-specific specs. + - `right-panel.spec.ts` waits in its `beforeEach`: its first action is a click + on a server-rendered close control. + - `contrast.spec.ts` performs no interactions. + - `claim-extraction` and `editor-mentions` build on `createNewEntity`, which + navigates via `navClick`. + What remains unguarded is a spec that clicks a statically imported view's + element before hydration *without* going through the helpers; none does today. 3. **PR runs still cover one viewport.** The nightly closes the gap daily, not per PR. If a viewport-specific regression lands, the next nightly catches it — acceptable for now; a matrix job per viewport would cost ~3× the runner time