From 077aca6008033722a5ceef3a44c2185c9997401e Mon Sep 17 00:00:00 2001 From: Tudor Date: Tue, 22 Sep 2026 15:13:32 +0100 Subject: [PATCH] test(e2e): wait for the carousel's smooth scroll to settle before measuring it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second failure of the same assertion, and the first fix addressed a real but different problem. This is the one that was actually producing the number. The arrows scroll with `behavior: 'smooth'`. The test polled for "has it moved at all" — satisfied 50ms in, at 13px of a 1300px journey — then recorded the offset, clicked, and recorded again. The row was still travelling throughout, so the delta it measured was the tail of the arrow's animation, not the effect of the selection. Hence a deterministic 659, roughly half of the 1317 this school's row scrolls. Measured against staging rather than reasoned about: the animation runs about 700ms, and the samples are in the helper's comment. settledScrollLeft waits for two identical readings before trusting one. The app was never at fault — driving staging by hand, the offset holds at 1317 across the selection, exactly as intended. Verified against staging both ways: main's version of this test fails there, this version passes, along with all three mobile widths. Co-Authored-By: Claude Opus 5 --- e2e/tests/journeys.spec.ts | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 1ee65e5..65d3812 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -1,4 +1,4 @@ -import { test, expect, Page } from '@playwright/test'; +import { test, expect, Locator, Page } from '@playwright/test'; /** * Journey tests for SchoolCompare, run against the staging environment as the @@ -19,6 +19,31 @@ function schoolLinks(page: Page) { return page.locator('a[href^="/school/"]'); } +/** + * A scroll offset that has stopped moving. + * + * The carousel arrows scroll with `behavior: 'smooth'`, so a reading taken + * straight after a click lands mid-animation. Measured against staging: the + * animation runs ~700ms, and a poll for "has it moved at all" is satisfied + * 50ms in, at 13px of a 1300px journey. A test that then records an offset, + * does something, and records again is measuring the tail of the arrow's + * animation rather than the effect of whatever it did in between. + * + * Two identical readings in a row is the cheapest sound definition of settled. + */ +async function settledScrollLeft(scroller: Locator): Promise { + let previous = -1; + await expect + .poll(async () => { + const current = await scroller.evaluate((node: HTMLElement) => Math.round(node.scrollLeft)); + const settled = current === previous; + previous = current; + return settled; + }, { timeout: 10_000 }) + .toBe(true); + return previous; +} + /** * Two URNs guaranteed to be pure-primary (same phase). The compare page's * phase tabs split all-through schools (which carry KS4 data) onto the @@ -2776,9 +2801,7 @@ test('nearby schools link on to other schools and into compare', async ({ page } await expect(back).toBeDisabled(); await forward.click(); - await expect - .poll(() => scroller.evaluate((node: HTMLElement) => node.scrollLeft)) - .toBeGreaterThan(8); + expect(await settledScrollLeft(scroller)).toBeGreaterThan(8); await expect(back).toBeEnabled(); } @@ -2794,12 +2817,12 @@ test('nearby schools link on to other schools and into compare', async ({ page } // // A few pixels of snap or sub-pixel adjustment are fine; a reset to the // start is not, which is the whole point of the check. - const offsetBefore = await scroller.evaluate((node: HTMLElement) => node.scrollLeft); + const offsetBefore = await settledScrollLeft(scroller); await section.getByRole('button', { name: /Add to compare/ }).last().click(); await expect( section.getByRole('button', { name: /Added to compare/ }).first(), ).toBeVisible(); - const offsetAfter = await scroller.evaluate((node: HTMLElement) => node.scrollLeft); + const offsetAfter = await settledScrollLeft(scroller); expect(Math.abs(offsetAfter - offsetBefore)).toBeLessThanOrEqual(8); }); -- 2.54.0