Skip to content

Conversation

@MTtankkeo
Copy link
Contributor

I'm currently working on a PR to port the new Android 12 overscroll effect. During this process, I encountered a test that was failing when it should have passed.

The root cause of the failure is that the test asserts a widget's absolute position immediately after a scroll gesture, without waiting for the overscroll physics animation to complete. A single pump() is insufficient for the animation to settle.
The solution is to replace the pump() call with pumpAndSettle(). This ensures that all animations are finished before the test makes its assertions, making the test reliable.

For more technical context on this new overscroll animation, please refer to the PR at:
flutter/flutter#173849

@chrisbobbe
Copy link
Collaborator

Thanks for the PR! Marking for @gnprice's review; chances are he'll make some tweaks and merge.

…nish

These tests scroll a long way, in order to be sure to scroll to the
end of the list.  That means they invoke overscroll.  The details of
how overscroll behaves are changing in an upstream PR:
  flutter/flutter#173849

The new behavior will take more than a single `pump` to reach the
end state where the subsequent checks behave as expected.  These
tests aren't about the overscroll behavior, so just gloss over any
wrinkles by calling `pumpAndSettle`.
@gnprice
Copy link
Member

gnprice commented Sep 4, 2025

Thanks @MTtankkeo! Looks good; merging.

As Chris predicted 🙂, I made a couple of tweaks: added comments to briefly justify the use here of pumpAndSettle, and expanded the commit message to do so in more detail:

emoji_reaction test: Use pumpAndSettle to let overscroll animation finish

These tests scroll a long way, in order to be sure to scroll to the
end of the list.  That means they invoke overscroll.  The details of
how overscroll behaves are changing in an upstream PR:
  https://github.com/flutter/flutter/pull/173849

The new behavior will take more than a single `pump` to reach the
end state where the subsequent checks behave as expected.  These
tests aren't about the overscroll behavior, so just gloss over any
wrinkles by calling `pumpAndSettle`.

(Using pumpAndSettle is an antipattern in general. The key here is that it's being used for a setup step of these tests, not a step that really involves any of the code these tests are meant to test.)

@gnprice gnprice merged commit 313f8ca into zulip:main Sep 4, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration review Added by maintainers when PR may be ready for integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants