From e221777eb41971c6c5c4f9b0e95fc5488c4f8c5a Mon Sep 17 00:00:00 2001 From: paul lahana Date: Sat, 3 Oct 2026 10:59:18 -0400 Subject: [PATCH] test: improve e2e test stability with polling Replace fixed waits with polling to handle CSS transitions reliably. Add drawerHeight helper to measure drawer size changes. Add settledGap function that polls until gap measurements stabilize, preventing false passes from mid-transition reads. Add pointOnGreyLine function that finds a clickable point on the route line and polls until coordinates hold steady. These changes make tests more robust to timing variations in loaded test runners. --- tests/e2e/route-identity.spec.js | 36 ++++++++++++++++++----- tests/e2e/scrubber-drawer-overlap.spec.js | 30 ++++++++++++++++--- 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/tests/e2e/route-identity.spec.js b/tests/e2e/route-identity.spec.js index 62985d2..4467a28 100644 --- a/tests/e2e/route-identity.spec.js +++ b/tests/e2e/route-identity.spec.js @@ -76,17 +76,39 @@ test('tapping the other label selects that route, without the pick menu', async await expect(page.locator('.pick-menu')).toHaveCount(0); }); +// A point on the grey line's stroke that a tap really reaches. The middle of +// the path's bounding box is off the line as soon as the route bends, the +// middle of the line can sit under a label (which selects the same route, so +// the test would pass without touching the line), and the fit that frames the +// routes is animated: a point read mid-zoom has moved by the time the click +// lands. So: the first sample the line itself answers to, read until it holds. +async function pointOnGreyLine(page) { + const read = () => page.locator('path.route-alt').evaluate((path) => { + const m = path.getScreenCTM(); + const len = path.getTotalLength(); + for (let i = 1; i < 20; i++) { + const p = path.getPointAtLength((len * i) / 20); + const x = m.a * p.x + m.c * p.y + m.e; + const y = m.b * p.x + m.d * p.y + m.f; + if (document.elementFromPoint(x, y) === path) return { x, y }; + } + return null; + }); + let last = null; + await expect.poll(async () => { + const at = await read(); + const held = at !== null && last !== null && at.x === last.x && at.y === last.y; + last = at; + return held; + }, { message: 'an uncovered point on the grey line, held still', intervals: [150] }).toBe(true); + return last; +} + test('tapping the grey line selects that route too', async ({ page }) => { await search(page); const target = other(await activeTab(page)); - // A point on the stroke itself: the middle of the path's bounding box is off - // the line as soon as the route bends. - const at = await page.locator('path.route-alt').evaluate((path) => { - const p = path.getPointAtLength(path.getTotalLength() / 2); - const m = path.getScreenCTM(); - return { x: m.a * p.x + m.c * p.y + m.e, y: m.b * p.x + m.d * p.y + m.f }; - }); + const at = await pointOnGreyLine(page); await page.mouse.click(at.x, at.y); await expectSelected(page, target); diff --git a/tests/e2e/scrubber-drawer-overlap.spec.js b/tests/e2e/scrubber-drawer-overlap.spec.js index 38fc55a..9ef9e2d 100644 --- a/tests/e2e/scrubber-drawer-overlap.spec.js +++ b/tests/e2e/scrubber-drawer-overlap.spec.js @@ -60,6 +60,25 @@ async function gap(page) { }); } +const drawerHeight = (page) => + page.evaluate(() => document.getElementById('results').getBoundingClientRect().height); + +// The gap once nothing moves. The drawer and the scrubber both get there +// through CSS transitions (0.32s in main.css), which a loaded runner can +// stretch past any fixed wait: a read mid-way says nothing, and polling for +// "gap >= 0" would pass on the first read, before the drawer has grown. So +// read until two reads in a row agree, and judge that one. +async function settledGap(page) { + let last = null; + await expect.poll(async () => { + const now = await gap(page); + const held = now === last; + last = now; + return held; + }, { message: 'drawer and scrubber at rest', intervals: [150] }).toBe(true); + return last; +} + // Both bottom-sheet viewports on purpose: from 900px up the drawer docks as a // side panel with the scrubber inside it (main.css), where there is no top // edge to be pushed under and no handle to expand. That layout has its own @@ -78,9 +97,9 @@ for (const viewport of [ await page.click('#drawer-handle'); await expect(page.locator('#results')).toHaveClass(/expanded/); - await page.waitForTimeout(500); // let the 0.32s expand transition settle - expect(await gap(page)).toBeGreaterThanOrEqual(0); + expect(await settledGap(page)).toBeGreaterThanOrEqual(0); + const withoutNote = await drawerHeight(page); // Scrub into the grazing window: this is what grows the drawer. await page.locator('#scrubber-range').evaluate(el => { @@ -89,9 +108,12 @@ for (const viewport of [ }); await expect(page.locator('#grazing-sun-note')).toHaveClass(/on/); - await page.waitForTimeout(300); + + // Otherwise there is nothing for the scrubber to follow, and the check + // below passes whatever the code does. + await expect.poll(() => drawerHeight(page)).toBeGreaterThan(withoutNote); // The regression: the drawer grew and the scrubber did not follow. - expect(await gap(page)).toBeGreaterThanOrEqual(0); + expect(await settledGap(page)).toBeGreaterThanOrEqual(0); }); }