From 9abd020967670a855e80fe5a908c8048a3aa9f14 Mon Sep 17 00:00:00 2001 From: Tudor Date: Thu, 20 Aug 2026 21:01:28 +0100 Subject: [PATCH] fix(e2e): scope the Distance-tab assertion, and separate the tile figures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The failing journey was wrong; the page was correct. Locator: getByRole('button', { name: 'Distance' }) Expected: 0 Received: 1 getByRole matches accessible names by case-insensitive SUBSTRING, so { name: 'Distance' } matched — the map toggle added by the same feature. The assertion was meant to say "no Distance tab in the admissions segmented control" and instead said "no button anywhere whose label contains the word distance". Now scoped to the control it is about, via its own aria-label, and read positively: the tab list must contain "This year" and must not contain "Distance". An absence check against an unscoped locator passes for the wrong reason the moment the selector stops matching, which is exactly how the regression this test guards would return unnoticed. Two sibling locators had the same weakness and are tightened: 'Check' is a prefix of the button's own busy label "Checking…", and the figure matcher accepted `(miles|m)` — a leftover from the mixed-unit era that would have kept passing if the headline regressed to metres, which is the thing #105 just fixed. Tightening that matcher surfaced a real defect behind it. The cut-off figure and its metric support are flex children with the gap drawn by CSS and nothing between them in the text layer, so the element read "0.88 miles1.4 km" — what a screen reader announces, and why a `miles\b` boundary could never match. Both templates now carry an explicit space. Whitespace text nodes are not rendered as flex items, so the reading changes and the layout does not. Verified against staging: 54/54. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01WDvkyqqHABm4bmth2kjAxE --- e2e/tests/journeys.spec.ts | 30 +++++++++++++++++-- .../components/lastDistanceOffered.test.tsx | 11 +++++++ .../components/school/AdmissionsSection.tsx | 8 ++++- .../school/SecondaryAdmissionsSection.tsx | 2 +- 4 files changed, 46 insertions(+), 5 deletions(-) diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 9c18dc8..b3fdbc0 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -1252,7 +1252,11 @@ test('a published cut-off distance is shown with the year it belongs to', async await expect(label).toContainText(`September ${found!.distance.year}`); // And the figure itself, in the unit councils publish in. - await expect(page.getByText(/\d+(\.\d+)? (miles|m)\b/).first()).toBeVisible(); + // No trailing \b: the tile's support figure is an adjacent text node, so + // the element reads "0.88 miles 1.4 km" and a word boundary after "miles" + // is not guaranteed. The leading shape is what matters — a decimal figure + // in miles, never a metric one. + await expect(page.getByText(/\d+\.\d+ miles/).first()).toBeVisible(); }); test('a cut-off distance is never shown without saying it is not a catchment', async ({ page }) => { @@ -1340,7 +1344,27 @@ test('a school page shows no year-by-year cut-off record', async ({ page }) => { // history back without the API. await expect(page.getByText(/Last distance offered, by year/)).toHaveCount(0); await expect(page.getByText(/too few to read as a trend/)).toHaveCount(0); - await expect(page.getByRole('button', { name: 'Distance' })).toHaveCount(0); + + /* + * No "Distance" tab in the admissions segmented control. + * + * Scoped to the control, and exact, because getByRole matches accessible + * names by case-insensitive SUBSTRING: an unscoped { name: 'Distance' } + * matched "Show this distance on a map" — the map toggle added by this same + * feature — and failed a page that was entirely correct. Naming the group + * this assertion is about also means unrelated copy elsewhere on the page + * can never break it again. + */ + const viewToggle = page.getByRole('group', { name: 'Admissions view' }); + const tabs = await viewToggle.getByRole('button').allTextContents(); + + // Read the tabs positively rather than asserting an absence against a + // locator that might resolve to nothing: if the group selector ever stops + // matching, an absence check passes for the wrong reason, which is how the + // bug this test is guarding would slip back in unnoticed. + expect(tabs, 'admissions view toggle did not resolve').toContain('This year'); + expect(tabs.map((s) => s.trim()), `admissions tabs: ${tabs.join(', ')}`) + .not.toContain('Distance'); }); test('the postcode check answers for the published year, and names it', async ({ page }) => { @@ -1354,7 +1378,7 @@ test('the postcode check answers for the published year, and names it', async ({ await expect(section).toBeVisible({ timeout: 15_000 }); await page.getByLabel('Your postcode').fill('SW1A 1AA'); - await page.getByRole('button', { name: 'Check' }).click(); + await page.getByRole('button', { name: 'Check', exact: true }).click(); const result = page.getByRole('status'); const error = page.getByRole('alert'); diff --git a/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx b/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx index 0c30e16..9f16df5 100644 --- a/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx +++ b/nextjs-app/__tests__/components/lastDistanceOffered.test.tsx @@ -30,6 +30,17 @@ describe('primary detail page', () => { expect(screen.getByText(/Last distance offered/)).toHaveTextContent('September 2024'); }); + it('keeps the figure and its metric support readable as two numbers', () => { + // They are flex children with a CSS gap and nothing between them in the + // text layer, which read as "0.48 miles770 m" to a screen reader and to any + // text matcher. Cheap to lose again, so pinned. + renderSchoolDetail({ ...primaryFixture, admissionDistance: cutoff({ distance_m: 772.49 }) }); + + const tile = document.querySelector('[class*="admissionsTileDistance"]')!; + expect(tile.textContent).toMatch(/0\.48 miles\s+770 m/); + expect(tile.textContent).not.toMatch(/miles\d/); + }); + it('never shows the figure without saying it is not a catchment', () => { renderSchoolDetail({ ...primaryFixture, admissionDistance: cutoff() }); diff --git a/nextjs-app/components/school/AdmissionsSection.tsx b/nextjs-app/components/school/AdmissionsSection.tsx index 6b423b7..fcac056 100644 --- a/nextjs-app/components/school/AdmissionsSection.tsx +++ b/nextjs-app/components/school/AdmissionsSection.tsx @@ -70,10 +70,16 @@ export function AdmissionsSection({ {/* Spans both columns rather than taking a half-width cell. This is the figure parents come to the page for, and at tile width the two-line "0.31 miles / September 2025" pairing wraps badly. */} + {/* The {' '} between the figure and its metric support is not decoration. + The two are flex children, so the gap is drawn by CSS and the text layer + had nothing between them: textContent read "0.88 miles1.4 km", which is + what a screen reader announces and what any text matcher sees. Whitespace + text nodes are not rendered as flex items, so this changes the reading + without changing the layout. */} const distanceTile = cutoff && (
- {cutoff.primary} + {cutoff.primary}{' '} {cutoff.secondary}
diff --git a/nextjs-app/components/school/SecondaryAdmissionsSection.tsx b/nextjs-app/components/school/SecondaryAdmissionsSection.tsx index 7510a2e..49c8970 100644 --- a/nextjs-app/components/school/SecondaryAdmissionsSection.tsx +++ b/nextjs-app/components/school/SecondaryAdmissionsSection.tsx @@ -79,7 +79,7 @@ export function SecondaryAdmissionsSection({ Last distance offered · {cutoff.entryYear}
- {cutoff.primary} + {cutoff.primary}{' '} {cutoff.secondary}
-- 2.54.0