From 10f169e54d8e02f88ab341088083f4bf073408ba Mon Sep 17 00:00:00 2001 From: Damian Pieczynski Date: Mon, 14 Sep 2026 08:18:23 +0200 Subject: [PATCH 1/2] test(react-virtual): wait for smooth scrolls to settle instead of a fixed 2s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "smooth scrolls to index 1000" flaked on CI (#1280 at 83bdf230, which had passed two days earlier on the same commit, and #1248 before it). A smooth scroll's duration scales with distance — index 1000 is ~50,000px — and reconcileScroll may re-drive it as rows measure; under 6x CPU throttling the scroll arrives ~2.2s after the click, past the fixed 2s wait, and the target row is not rendered yet. Replace every fixed wait with a helper that waits for the target row to be visible and for scrollTop to stop moving, with a generous timeout. The 200ms mid-animation interrupt in the last test is kept: it is part of the scenario, not a settle wait. Co-Authored-By: Claude Fable 5.1 --- .../e2e/app/test/smooth-scroll.spec.ts | 65 ++++++++++--------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/packages/react-virtual/e2e/app/test/smooth-scroll.spec.ts b/packages/react-virtual/e2e/app/test/smooth-scroll.spec.ts index d8650db91..d6cc2bab6 100644 --- a/packages/react-virtual/e2e/app/test/smooth-scroll.spec.ts +++ b/packages/react-virtual/e2e/app/test/smooth-scroll.spec.ts @@ -1,13 +1,36 @@ import { expect, test } from '@playwright/test' +import type { Page } from '@playwright/test' + +// A smooth scroll's duration scales with distance (index 1000 is ~50,000px), +// and reconcileScroll may re-drive it as rows measure. On a busy CI runner that +// can outlast a fixed 2s wait — the "index 1000" case has flaked exactly that +// way. Wait for the target row to render and the scroll position to stop +// moving instead. +async function waitForSmoothScroll(page: Page, testId: string) { + await expect(page.locator(`[data-testid="${testId}"]`)).toBeVisible({ + timeout: 15_000, + }) + let last = -1 + await expect + .poll( + async () => { + const cur = await page.evaluate( + () => document.querySelector('#scroll-container')!.scrollTop, + ) + const settled = cur === last + last = cur + return settled + }, + { timeout: 15_000, intervals: [100] }, + ) + .toBe(true) +} test('smooth scrolls to index 1000', async ({ page }) => { await page.goto('/smooth-scroll/') await page.click('#scroll-to-1000') - // Smooth scroll animation is 500ms + reconciliation time - await page.waitForTimeout(2000) - - await expect(page.locator('[data-testid="item-1000"]')).toBeVisible() + await waitForSmoothScroll(page, 'item-1000') const delta = await page.evaluate(() => { const item = document.querySelector('[data-testid="item-1000"]') @@ -29,9 +52,7 @@ test('smooth scrolls to index 100', async ({ page }) => { await page.goto('/smooth-scroll/') await page.click('#scroll-to-100') - await page.waitForTimeout(2000) - - await expect(page.locator('[data-testid="item-100"]')).toBeVisible() + await waitForSmoothScroll(page, 'item-100') }) test('smooth scrolls to index 0 after scrolling away', async ({ page }) => { @@ -39,14 +60,11 @@ test('smooth scrolls to index 0 after scrolling away', async ({ page }) => { // First scroll down await page.click('#scroll-to-500') - await page.waitForTimeout(2000) - await expect(page.locator('[data-testid="item-500"]')).toBeVisible() + await waitForSmoothScroll(page, 'item-500') // Then smooth scroll back to top await page.click('#scroll-to-0') - await page.waitForTimeout(2000) - - await expect(page.locator('[data-testid="item-0"]')).toBeVisible() + await waitForSmoothScroll(page, 'item-0') const scrollTop = await page.evaluate(() => { const container = document.querySelector('#scroll-container') @@ -59,9 +77,7 @@ test('smooth scrolls to index 500 with start alignment', async ({ page }) => { await page.goto('/smooth-scroll/') await page.click('#scroll-to-500-start') - await page.waitForTimeout(2000) - - await expect(page.locator('[data-testid="item-500"]')).toBeVisible() + await waitForSmoothScroll(page, 'item-500') const delta = await page.evaluate( ([idx, align]) => { @@ -84,9 +100,7 @@ test('smooth scrolls to index 500 with center alignment', async ({ page }) => { await page.goto('/smooth-scroll/') await page.click('#scroll-to-500-center') - await page.waitForTimeout(2000) - - await expect(page.locator('[data-testid="item-500"]')).toBeVisible() + await waitForSmoothScroll(page, 'item-500') const delta = await page.evaluate( ([idx]) => { @@ -110,18 +124,15 @@ test('smooth scrolls sequentially to multiple targets', async ({ page }) => { // Scroll to 100 first await page.click('#scroll-to-100') - await page.waitForTimeout(2000) - await expect(page.locator('[data-testid="item-100"]')).toBeVisible() + await waitForSmoothScroll(page, 'item-100') // Then scroll to 500 await page.click('#scroll-to-500') - await page.waitForTimeout(2000) - await expect(page.locator('[data-testid="item-500"]')).toBeVisible() + await waitForSmoothScroll(page, 'item-500') // Then scroll to 1000 await page.click('#scroll-to-1000') - await page.waitForTimeout(2000) - await expect(page.locator('[data-testid="item-1000"]')).toBeVisible() + await waitForSmoothScroll(page, 'item-1000') }) test('interrupting smooth scroll with another smooth scroll', async ({ @@ -135,9 +146,5 @@ test('interrupting smooth scroll with another smooth scroll', async ({ await page.waitForTimeout(200) await page.click('#scroll-to-100') - // Wait for the second scroll to complete - await page.waitForTimeout(2000) - - // Should have ended at 100, not 1000 - await expect(page.locator('[data-testid="item-100"]')).toBeVisible() + await waitForSmoothScroll(page, 'item-100') }) From b1db2c0a4ef43bed22751655e3a1970958d4beee Mon Sep 17 00:00:00 2001 From: Damian Pieczynski Date: Mon, 14 Sep 2026 10:52:10 +0200 Subject: [PATCH 2/2] test(react-virtual): wait for reconcile to retire the scroll, not for equal samples Review follow-up. Two scrollTop samples 100ms apart can straddle the pause between two reconcileScroll re-drives, so equal samples do not prove the smooth scroll has completed. The virtualizer has the real signal: reconcileScroll retires `scrollState` only once the target is stable and reached. Expose the instance on the fixture page and wait for `scrollState === null && !isScrolling` after the target row is visible, keeping the 15s ceiling. Co-Authored-By: Claude Fable 5.1 --- .../e2e/app/smooth-scroll/main.tsx | 4 +++ .../e2e/app/test/smooth-scroll.spec.ts | 26 +++++++++---------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/react-virtual/e2e/app/smooth-scroll/main.tsx b/packages/react-virtual/e2e/app/smooth-scroll/main.tsx index 565d48c36..a23d4980e 100644 --- a/packages/react-virtual/e2e/app/smooth-scroll/main.tsx +++ b/packages/react-virtual/e2e/app/smooth-scroll/main.tsx @@ -27,6 +27,10 @@ const App = () => { getScrollElement: () => parentRef.current, estimateSize: () => 50, }) + // Test hook: lets the spec wait on the virtualizer's own settlement signal + // (reconcileScroll retiring `scrollState`) instead of guessing from + // scrollTop samples. + ;(window as any).__virtualizer = rowVirtualizer return (
diff --git a/packages/react-virtual/e2e/app/test/smooth-scroll.spec.ts b/packages/react-virtual/e2e/app/test/smooth-scroll.spec.ts index d6cc2bab6..a893dc958 100644 --- a/packages/react-virtual/e2e/app/test/smooth-scroll.spec.ts +++ b/packages/react-virtual/e2e/app/test/smooth-scroll.spec.ts @@ -2,26 +2,24 @@ import { expect, test } from '@playwright/test' import type { Page } from '@playwright/test' // A smooth scroll's duration scales with distance (index 1000 is ~50,000px), -// and reconcileScroll may re-drive it as rows measure. On a busy CI runner that -// can outlast a fixed 2s wait — the "index 1000" case has flaked exactly that -// way. Wait for the target row to render and the scroll position to stop -// moving instead. +// and reconcileScroll may re-drive it as rows measure, so neither a fixed wait +// nor a pair of equal scrollTop samples proves completion: the "index 1000" +// case flaked on a fixed 2s wait, and two samples can straddle the pause +// between two re-drives. The virtualizer has the real signal: reconcileScroll +// retires `scrollState` only once the target is stable and reached. Wait for +// the target row to render, then for that retirement. async function waitForSmoothScroll(page: Page, testId: string) { await expect(page.locator(`[data-testid="${testId}"]`)).toBeVisible({ timeout: 15_000, }) - let last = -1 await expect .poll( - async () => { - const cur = await page.evaluate( - () => document.querySelector('#scroll-container')!.scrollTop, - ) - const settled = cur === last - last = cur - return settled - }, - { timeout: 15_000, intervals: [100] }, + () => + page.evaluate(() => { + const v = (window as any).__virtualizer + return v.scrollState === null && v.isScrolling === false + }), + { timeout: 15_000, intervals: [50] }, ) .toBe(true) }