diff --git a/e2e/v137-sharing-managed-profiles-a11y.spec.ts b/e2e/v137-sharing-managed-profiles-a11y.spec.ts index 87c3e3128..53ca7303f 100644 --- a/e2e/v137-sharing-managed-profiles-a11y.spec.ts +++ b/e2e/v137-sharing-managed-profiles-a11y.spec.ts @@ -263,26 +263,62 @@ test.describe("shared record accessibility states", () => { test("[loading] the hold before the active record is known is announced", async ({ page, }) => { - await page.route("**/api/auth/me", async (route) => { - await new Promise((resolve) => setTimeout(resolve, 1500)); - await route.continue(); + const gate = '[data-slot="record-scope-hydration-gate"]'; + + // Hold `/api/auth/me` open for exactly as long as the assertions need, + // then release it once. + // + // A fixed sleep in the handler cannot do that, and it was the cause of a + // CI-only failure here. Three things go wrong with it. The hold ends 1500 + // ms after the FIRST request whatever the axe scan is doing, so on a slow + // runner the state being scanned is not the state the test names. Every + // retry and every refetch queues another sleeping handler, so the number + // of live handlers is a function of machine speed. And a handler still + // sleeping when its request is cancelled — a navigation, a React Query + // abort, teardown — answers `route.continue()` with "Route is already + // handled", which is what the runner reported. + // + // An explicit gate fixes all three: one release, at a point the test + // chooses, after which the hold is provably over. + let release!: () => void; + const held = new Promise((resolve) => { + release = resolve; }); + let holds = 0; + + await page.route("**/api/auth/me", async (route) => { + holds += 1; + await held; + // Continuing a route the browser has already given up on is not a + // failure of anything this test asserts, and it is the one call here + // that can reject for a reason that has nothing to do with the subject. + await route.continue().catch(() => {}); + }); + await page.goto("/measurements"); - await expect( - page.locator('[data-slot="record-scope-hydration-gate"]'), - ).toBeVisible(); + await expect(page.locator(gate)).toBeVisible(); await scanForViolations(page); - await expectAnnounced(page, '[data-slot="record-scope-hydration-gate"]'); + await expectAnnounced(page, gate); // The label lives inside the region, so a reader that reaches the region // reaches the sentence. A label rendered as a sibling would be announced // separately or not at all. - await expectReaderOrder(page, [ - '[data-slot="record-scope-hydration-gate"]', - '[data-slot="record-scope-hydration-gate"] .sr-only', - ]); + await expectReaderOrder(page, [gate, `${gate} .sr-only`]); + // The hold was real. Without this the whole test passes on a page that was + // merely slow, and the state it claims to have scanned would be a + // coincidence rather than something it caused. + expect( + holds, + "nothing was held, so the gate was not this test's doing", + ).toBeGreaterThan(0); + + release(); await page.unroute("**/api/auth/me"); + // And the gate was waiting on that hold rather than on something else: + // releasing it ends the state. An entry assertion with no exit assertion + // cannot tell a hold apart from a stuck page. + await expect(page.locator(gate)).toHaveCount(0); }); test("[error] a failed read says so and hands the retry to a keyboard", async ({ diff --git a/e2e/v137-sharing-managed-profiles.spec.ts b/e2e/v137-sharing-managed-profiles.spec.ts index ac9373488..6fc2c9aaa 100644 --- a/e2e/v137-sharing-managed-profiles.spec.ts +++ b/e2e/v137-sharing-managed-profiles.spec.ts @@ -156,15 +156,30 @@ test.describe.serial("scoped sharing browser journeys", () => { ); await expect(entry).toHaveCount(1); - const unexpectedOcrCapability = - record.domain === "labs" - ? page.waitForRequest( - (request) => - request.method() === "GET" && - new URL(request.url()).pathname === "/api/labs/ocr/capability", - { timeout: 500 }, - ) - : null; + // The labs doorway must not pull the OCR capability probe: OCR is an + // owner surface and a scoped READ grant does not reach it. + // + // Proved with a listener rather than `waitForRequest(…, { timeout })`, + // and the difference is not stylistic. A timed wait is an instrument + // that FAILS by its own mechanics: the rejection it produces is the + // pass condition, so it has to be created early and awaited late, and in + // between it is an unhandled rejection that lands on whichever test + // happens to be finishing. Its window is wrong too — the 500 ms starts + // before the switch is even clicked, so on a slow runner it can expire + // before the labs page has loaded, which is the one moment it exists to + // watch. + // + // A listener has no timer to lose. It covers everything from before the + // click to after the record read has landed, which is strictly more than + // 500 ms, and it is asserted below with a paired positive control: the + // domain's own read must appear in the same set, so a matcher that + // stopped seeing requests fails instead of reporting an absence. + const seenPaths = new Set(); + page.on("request", (request) => { + if (request.method() !== "GET") return; + seenPaths.add(new URL(request.url()).pathname); + }); + const read = page.waitForResponse( (response) => response.request().method() === "GET" && @@ -183,8 +198,19 @@ test.describe.serial("scoped sharing browser journeys", () => { page.locator('[data-slot="shared-record-unavailable"]'), ).toHaveCount(0); await read; - if (unexpectedOcrCapability) { - await expect(unexpectedOcrCapability).rejects.toThrow(/Timeout/); + + // The positive control first: if this path is missing the listener saw + // nothing useful and the absence below would be a statement about the + // matcher rather than about the product. + expect( + [...seenPaths], + `the ${record.domain} read never reached the request listener`, + ).toContain(DOMAIN_READS[record.domain]); + if (record.domain === "labs") { + expect( + [...seenPaths], + "a scoped labs grant pulled the owner-only OCR capability probe", + ).not.toContain("/api/labs/ocr/capability"); } await leaveRecord(page);