test: give the two sharing probes instruments that cannot fail by themselves

Both failed on CI and neither failure was about the product.

The loading-state scan held /api/auth/me open with a fixed sleep in the
route handler. Three things follow from that and all three are wrong: the
hold ends 1500 ms after the first request whatever the axe scan is doing,
so the state being scanned is not the state the test names; every refetch
queues another sleeping handler, so how many are live is a property of
the machine; and a handler still sleeping when its request is cancelled
answers route.continue with "Route is already handled", which is what the
runner reported. It now holds on an explicit gate released at a point the
test chooses, counts what it held so a page that was merely slow cannot
pass for a page it held, and asserts the gate goes away on release, which
is the exit control the entry assertion never had.

The labs doorway proved that no OCR probe fires by waiting for that
request with a 500 ms timeout and expecting the rejection. An instrument
whose pass condition is its own failure has to be armed early and awaited
late, and in between it is an unhandled rejection that lands on whichever
test is finishing. Its window was wrong too: the timer started before the
switch was clicked, so on a slow runner it could expire before the labs
page existed. A request listener replaces it, covering everything from
before the click to after the record read landed, with the domain's own
read asserted in the same set so an absence cannot be a statement about a
matcher that stopped matching.
This commit is contained in:
Marc-André Bombeck committed 2026-08-07 10:26:41 +02:00
1 parent a5b6508961
commit 53ffbfdec4
2 files changed
+84 -22

No files matched your search

+47 -11
View File
@@ -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<void>((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 ({
+37 -11
View File
@@ -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<string>();
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);