Merge pull request 'test(e2e): wait for the carousel's smooth scroll to settle before measuring it' (#153) from fix/nearby-journey-waits-for-smooth-scroll into main

Reviewed-on: #153

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<number> {
+  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,20 +2801,29 @@ test('nearby schools link on to other schools and into compare', async ({ page }
     await expect(back).toBeDisabled();
 
     await forward.click();
-    await expect
+    expect(await settledScrollLeft(scroller)).toBeGreaterThan(8);
-      .poll(() => scroller.evaluate((node: HTMLElement) => node.scrollLeft))
-      .toBeGreaterThan(8);
     await expect(back).toBeEnabled();
   }
 
   // The compare hand-off, and the row must not jump back to the start when the
   // footer re-renders underneath it.
-  const offsetBefore = await scroller.evaluate((node: HTMLElement) => node.scrollLeft);
+  //
-  await section.getByRole('button', { name: /Add to compare/ }).first().click();
+  // Click the LAST card's button, not the first. Playwright scrolls a target
+  // into view before clicking it, so clicking card one while the row is paged
+  // to the end scrolls the container back to the start — and the assertion
+  // below then measures Playwright's own scrolling rather than the app's.
+  // That is what this test did on its first staging run: 537 → 2, reproduced
+  // afterwards on a static page with no React on it at all.
+  //
+  // 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 settledScrollLeft(scroller);
+  await section.getByRole('button', { name: /Add to compare/ }).last().click();
   await expect(
     section.getByRole('button', { name: /Added to compare/ }).first(),
   ).toBeVisible();
-  expect(await scroller.evaluate((node: HTMLElement) => node.scrollLeft)).toBe(offsetBefore);
+  const offsetAfter = await settledScrollLeft(scroller);
+  expect(Math.abs(offsetAfter - offsetBefore)).toBeLessThanOrEqual(8);
 });
 
 /**