From 64c2f7410a49f93020ea93f64657fa722f1ffbeb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Mon, 3 Aug 2026 21:47:58 +0200 Subject: [PATCH] Wait for the menu to open, not for the network to go quiet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The water-capture journey clicked the dashboard quick-add trigger after `waitForLoadState("networkidle")` and then waited thirty seconds for a menu item that never arrived. The failure snapshot shows the page rendered, the trigger present, and no menu anywhere: the click landed on server HTML that React had not attached a handler to yet, and the one click the test had was spent before hydration. `networkidle` cannot close that window, because the trigger is in the first HTML and exists long before the client bundle finishes. The gate has to be the element's own state. `openMenu` clicks and confirms the trigger reports itself open — Radix stamps `data-state`, the hand-rolled mobile capture button carries `aria-expanded` off React state — and spends another click if it does not, because the problem is a lost event and waiting longer for a swallowed click achieves nothing. Out of attempts it fails naming the state the trigger was actually in, rather than letting the caller time out on a menu item that was never going to appear. This is a race the spec always had; it surfaces when the machine is loaded enough to push hydration past the click, which is why it reads as a branch-specific regression when it is not one. The tour passthrough spec keeps its single un-retried click on the same trigger: proving that exactly one click reaches the button through the overlay is that test's entire point. --- e2e/open-menu.ts | 62 +++++++++++++++++++++++++++++++++++++++ e2e/water-capture.spec.ts | 8 +++-- 2 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 e2e/open-menu.ts diff --git a/e2e/open-menu.ts b/e2e/open-menu.ts new file mode 100644 index 000000000..5fe70144b --- /dev/null +++ b/e2e/open-menu.ts @@ -0,0 +1,62 @@ +import { expect, type Locator, type Page } from "@playwright/test"; + +/** + * Click a menu trigger and confirm the menu actually opened. + * + * A bare `trigger.click()` is a coin flip on a server-rendered page. Playwright + * clicks as soon as the element is visible, stable and enabled — all three of + * which are true of markup React has not attached a handler to yet. The click + * lands, nothing happens, and the next line waits thirty seconds for a menu + * item that will never appear, because the one click the test had was spent + * before hydration. + * + * `networkidle` does not close that window either: the trigger is in the first + * HTML, so it is present long before the client bundle finishes. The gate has + * to be the element's own state, not the network's. + * + * This is the failure that took two branches to read: the quick-add trigger on + * the dashboard, clicked, no menu in the failure snapshot, and no defect in the + * component. It surfaces when the machine is loaded — a heavier suite is enough + * to push hydration past the click — so it reads as a branch-specific + * regression when it is a race the spec always had. + * + * The signal is the trigger's own open state. Radix stamps `data-state="open"`; + * a hand-rolled trigger carries `aria-expanded="true"` off React state. Either + * one only appears once a handler ran, which is exactly the thing being waited + * for. Retry the click rather than raising a timeout: the problem is a lost + * event, and waiting longer for a click that was already swallowed does + * nothing. + */ +export async function openMenu(page: Page, trigger: Locator): Promise { + await expect(trigger).toBeVisible(); + + for (let attempt = 0; attempt < 5; attempt++) { + await trigger.click(); + if (await isOpen(trigger)) return; + // Swallowed by a not-yet-hydrated trigger. Give the bundle a moment and + // spend another click. + await page.waitForTimeout(250); + } + + // Out of attempts. Fail naming the state the trigger was actually in, rather + // than letting the caller time out on a menu item that was never going to + // appear. + const state = await trigger.getAttribute("data-state"); + const expanded = await trigger.getAttribute("aria-expanded"); + throw new Error( + `openMenu: the trigger never opened after 5 clicks (data-state=${state}, aria-expanded=${expanded}). ` + + "The page is most likely still hydrating; the trigger is in the server HTML and takes clicks before React attaches.", + ); +} + +async function isOpen(trigger: Locator): Promise { + for (let i = 0; i < 10; i++) { + const [state, expanded] = await Promise.all([ + trigger.getAttribute("data-state"), + trigger.getAttribute("aria-expanded"), + ]); + if (state === "open" || expanded === "true") return true; + await trigger.page().waitForTimeout(100); + } + return false; +} diff --git a/e2e/water-capture.spec.ts b/e2e/water-capture.spec.ts index 7c9e99bf4..d16fa8eb9 100644 --- a/e2e/water-capture.spec.ts +++ b/e2e/water-capture.spec.ts @@ -1,6 +1,7 @@ import { expect, test } from "./setup/test"; import { STORAGE_STATE_PATH } from "./setup/global-setup"; +import { openMenu } from "./open-menu"; import { mockDashboardSnapshot, WEIGHT_ONLY_SUMMARIES, @@ -66,7 +67,7 @@ test.describe("water capture", () => { if (toastMayOverlap) { await capture.dispatchEvent("click"); } else { - await capture.click(); + await openMenu(page, capture); } const option = page.getByTestId("capture-picker-water"); await expect(option).toBeVisible(); @@ -78,7 +79,10 @@ test.describe("water capture", () => { await option.click(); } } else { - await page.locator('[data-tour-id="dashboard-quick-add"]').click(); + await openMenu( + page, + page.locator('[data-tour-id="dashboard-quick-add"]'), + ); await page.getByRole("menuitem", { name: "Log water" }).click(); } await expect(