From 90ac6e082586b74e4dd9d6b8a816c5d957e4b944 Mon Sep 17 00:00:00 2001 From: Logan Date: Wed, 19 Aug 2026 11:50:46 -0400 Subject: [PATCH] test(e2e): wait for the scroll range the assertion needs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The guard polled for `scrollHeight > clientHeight + 40` and the next line asserted the container could be scrolled to 80, so any range in 41-79 satisfied the precondition and could not satisfy the assertion. The grid passes through exactly that while it settles, because it recomputes its columns after a viewport change rather than during it, so the test read a clamped scrollTop and reported 10 against 80. It failed CI on a pull request that changes one paragraph of CLAUDE.md and nothing else, while WebKit passed in the same run. Reproduced locally: 0 failures in 6 runs before #132, 2 in 9 after, 0 in 10 with this change. #132 is what made it reachable rather than what broke it. The queue panel's mode is measured rather than media-queried, so a viewport change at this width costs one more layout pass, and cover-grid settles after it instead of before. The settled range is 330 and stable, the main panel is 700px, and the panel is correctly display:none while closed — there is no user-visible defect, only a wider window for a race the spec already had. A threshold below the value its caller depends on is not a guard, so the target is one constant that both the guard and the assertion read. Closes #133 --- e2e/specs/album-dropdown.spec.ts | 38 +++++++++++++++++++++++--------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/e2e/specs/album-dropdown.spec.ts b/e2e/specs/album-dropdown.spec.ts index 324a194..c629b62 100644 --- a/e2e/specs/album-dropdown.spec.ts +++ b/e2e/specs/album-dropdown.spec.ts @@ -1,6 +1,12 @@ import { test, expect } from '../support/fixtures.js'; import type { Page } from '@playwright/test'; +/** + * How far the scroll test scrolls. One constant, because the guard and + * the assertion have to agree about it — they did not, which is #133. + */ +const SCROLL_TARGET = 80; + /** * Plan 007 phase 5: expanding an album shows its tracks. * @@ -104,20 +110,27 @@ test.describe('the album dropdown', () => { await app.setViewportSize({ width: 900, height: 600 }); try { - await expect.poll(() => scrollRange(app)).toMatchObject({ - scrollable: true, - overflowY: 'auto', - }); + // Wait for the range the assertion below actually needs, not for + // "scrollable at all" (#133). The guard used to be + // `scrollHeight > clientHeight + 40` while the next line asks to + // reach 80, so any range in 41-79 satisfied it and could not + // satisfy the assertion — and the grid passes through exactly + // that while it settles, because it recomputes its columns after + // the resize rather than during it. The settled range here is + // 330, so this waits rather than weakening anything. + await expect + .poll(() => scrollRange(app)) + .toMatchObject({ room: true, overflowY: 'auto' }); - await app.evaluate(() => { + await app.evaluate((target) => { const sc = document .querySelector('cover-grid') ?.shadowRoot?.querySelector('.grid-scroll-container'); - if (sc) sc.scrollTop = 80; - }); + if (sc) sc.scrollTop = target; + }, SCROLL_TARGET); - expect(await scrollTop(app)).toBe(80); + expect(await scrollTop(app)).toBe(SCROLL_TARGET); // And the dropdown it opens is on screen, wherever the manager // decides that leaves the scroll. It is *not* "the position is @@ -250,16 +263,19 @@ async function closeDropdown(app: Page): Promise { /** Whether the grid can scroll at all, which decides if a probe can move. */ async function scrollRange(app: Page) { - return app.evaluate(() => { + return app.evaluate((target) => { const sc = document .querySelector('cover-grid') ?.shadowRoot?.querySelector('.grid-scroll-container'); return { - scrollable: !!sc && sc.scrollHeight > sc.clientHeight + 40, + // `room` is the precondition of the assertion that follows it: + // enough range to actually reach the target. A threshold below + // what the caller depends on is not a guard. + room: !!sc && sc.scrollHeight - sc.clientHeight >= target, overflowY: sc ? getComputedStyle(sc).overflowY : '', }; - }); + }, SCROLL_TARGET); } async function scrollTop(app: Page): Promise {