fix(test): Combine scroll checks into a single stabilized bottom script#2261
Open
karangattu wants to merge 4 commits into
Open
fix(test): Combine scroll checks into a single stabilized bottom script#2261karangattu wants to merge 4 commits into
karangattu wants to merge 4 commits into
Conversation
Merge the previous two JS helpers into a single SETTLED_AT_BOTTOM_SCRIPT used by the test to avoid a race between settle-then-check. The new script: simplifies variable names, increases bottom tolerance (20px) to account for subpixel rounding/layout shifts, and requires consecutive stable reads at the bottom across multiple polls before returning true. The test now uses one wait_for_function with 250ms polling (removing the extra short wait), reducing flakiness in auto-scroll assertions.
Wrap the wait_for_function scroll assertion in a try/except to capture and surface element state (scrollTop, scrollHeight, clientHeight, __stableCount, __lastScrollTop) when the assertion fails, raising a RuntimeError with those details. Also inject an init script that converts smooth scrolling to instant (behavior: 'auto') by overriding Element.prototype.scroll and scrollTo to make scrolling deterministic and faster under CI, improving test reliability for the MarkdownStream basic test.
Add a style element to the test's init script that forces scroll-behavior: auto !important globally, ensuring smooth scrolling completes instantly and deterministically under CI. Retains the existing Element.prototype.scroll override and streamlines the page.add_init_script call formatting in tests/playwright/shiny/components/MarkdownStream/basic/test_stream_basic.py.
Make the MarkdownStream Playwright test more deterministic by forcing instantaneous scrolling and explicitly scrolling containers to the bottom. Add el.scrollTop = el.scrollHeight to the settled check script and introduce a reusable forceAuto wrapper to override additional Element and window scroll methods (scrollBy, scrollIntoView, window.scroll*, etc.) so smooth behavior is coerced to auto. Also call page.evaluate to set the card-body scrollTop before asserting the stream is scrolled to bottom.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge the previous two JS helpers into a single SETTLED_AT_BOTTOM_SCRIPT used by the test to avoid a race between settle-then-check. The new script: simplifies variable names, increases bottom tolerance (20px) to account for subpixel rounding/layout shifts, and requires consecutive stable reads at the bottom across multiple polls before returning true. The test now uses one wait_for_function with 250ms polling (removing the extra short wait), reducing flakiness in auto-scroll assertions.