Skip to content

Fix flaky persist-scroll-position gesture test#5607

Open
habdelra wants to merge 1 commit into
mainfrom
fix-persist-scroll-position-gesture-flake
Open

Fix flaky persist-scroll-position gesture test#5607
habdelra wants to merge 1 commit into
mainfrom
fix-persist-scroll-position-gesture-flake

Conversation

@habdelra

Copy link
Copy Markdown
Contributor

Integration | modifier | persist-scroll-position: records the offset only after a genuine user scroll gesture is flaky — it surfaces on host shards (which main CI skips, so it goes unnoticed there) and reproduces locally under load at roughly a two-in-three failure rate.

Root cause

A programmatic scrollTop assignment fires its scroll event on the next rendering frame, not synchronously, and settled() does not await that frame. The test's gesture-less phase sets scrollTop = 90 (asserting it is not recorded, since no gesture has happened). That assignment's native scroll event stays pending. Under load it lands during the following await triggerEvent(scroller, 'wheel') settle — after the wheel has flipped the modifier from restoring into recording — so the modifier records 90 as a user offset. recorded becomes [90, 120, …] and the every(v === 120) assertion fails.

Confirmed by logging each onChange and the wheel boundary: on a failing run, onChange 90 fires between the pre-wheel and post-wheel markers.

Fix

Await one animation frame after the gesture-less scroll, so its pending scroll event drains while there is still no gesture — the modifier ignores it, as intended — before the wheel switches on recording. The recording path is otherwise unchanged.

Also surface the recorded array in the assertion message, so if this ever regresses the failure output names exactly what leaked instead of a bare boolean.

Verification

Reproduced the failure locally (full-module run under load), then confirmed the fix: 5 consecutive green full-module runs, and the single test green in isolation. The drain consumes precisely the event that was leaking.

🤖 Generated with Claude Code

…sition test

`records the offset only after a genuine user scroll gesture` flaked: a
programmatic `scrollTop` assignment fires its `scroll` event on the next
rendering frame, not synchronously, and `settled()` does not await that
frame. The gesture-less scroll set to 90 could therefore land during the
subsequent `await triggerEvent(scroller, 'wheel')` settle — after the wheel
flipped the modifier from restoring into recording — and be recorded as a
spurious user offset, so `recorded` became `[90, 120, ...]` and the
`every(v === 120)` check failed. Confirmed by logging: `onChange 90` fired
between the pre-wheel and post-wheel markers.

Await one animation frame after the gesture-less scroll so its event drains
while there is still no gesture (and is correctly ignored). Also surface the
recorded array in the assertion message so any future miss is diagnosable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Preview deployments

Host Test Results

    1 files      1 suites   2h 56m 48s ⏱️
3 654 tests 3 639 ✅ 15 💤 0 ❌
3 673 runs  3 658 ✅ 15 💤 0 ❌

Results for commit 29dd20d.

Realm Server Test Results

    1 files      1 suites   13m 33s ⏱️
1 935 tests 1 935 ✅ 0 💤 0 ❌
2 014 runs  2 014 ✅ 0 💤 0 ❌

Results for commit 29dd20d.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants