[iOS] Fix flaky keyboard animation test#189353
Conversation
testKeyboardAnimationWillWaitUIThreadVsync posted a `sleep(1)` task to the UI task runner and asserted that the keyboard animation callback did not fire until at least that delay had elapsed. Whether or not the callback actually observed the delay depended on thread scheduling, which made the test flaky. The property the old test was attempting to verify, that the callback does not run until in-flight UI-thread work completes, is not implemented by this code, so there is nothing here for a test to protect. This is guaranteed by the run-loop itself. The vsync client registers its CADisplayLink on the UI task runner's run loop (see setUpKeyboardAnimationVsyncClient in FlutterKeyboardInsetManager and VSyncClient.init), and a run loop services one source at a time, so the display-link callback can't possibly run while the UI thread is busy processing a begin-frame event. Asserting that ordering means asserting CFRunLoop and CADisplayLink scheduling semantics, which is not only outside our control but inherently racy, which causes this test to be flaky/fragile. What this code *does* enforce is that delivery is asynchronous. The callback is wrapped in `dispatch_async(dispatch_get_main_queue())` rather than invoked inline from `-onDisplayLink:`. This updates the test to trigger the display link directly, asserts the callback has not run synchronously, then waits on an `XCTestExpectation` and asserts that it's eventually delivered on the main queue. The view controller is now built with FlutterEnginePartialMock, whose uiTaskRunner runs on the test's own message loop, so the vsync client's display-link registration and invalidation execute deterministically on the current thread. I've also renamed the test to testKeyboardAnimationCallbackIsDeliveredAsynchronously to match what it actually verifies. As mentioned above, this intentionally drops coverage of the wait-for-UI-thread behavior originally added for flutter#120555. As noted, that's already guaranteed by the run-loop and unrelated to our own logic.
There was a problem hiding this comment.
Code Review
This pull request refactors a keyboard animation test in FlutterViewControllerTest.mm by renaming testKeyboardAnimationWillWaitUIThreadVsync to testKeyboardAnimationCallbackIsDeliveredAsynchronously. It replaces FlutterEngine with FlutterEnginePartialMock to run deterministically on the test message loop, removing the need to block the UI thread with a sleep task, and asserts that the keyboard animation callback is executed asynchronously rather than synchronously. There are no review comments, and I have no feedback to provide.
| bundle:nil]; | ||
| // Post a task to UI thread to block the thread. | ||
| const int delayTime = 1; | ||
| [[engine uiTaskRunner] postTask:^{ |
There was a problem hiding this comment.
Is it a recent flakiness? Curious if it is coming from our merged thread change.
There was a problem hiding this comment.
Not super recent; but I've run into it a bunch while tweaking keyboard inset manager, maybe due to tiny changes in timing due to either swift memory handling being more agressive?
flutter/flutter@cf9e8af...846664b 2026-07-14 [email protected] Roll Skia from dfcff99566c3 to 88954ef8f36d (1 revision) (flutter/flutter#189440) 2026-07-14 [email protected] refactor: remove material import from scrollable_semantics_test and selectable_region_context_menu_test (flutter/flutter#186611) 2026-07-14 [email protected] Roll Skia from 3d1fc554f1a2 to dfcff99566c3 (17 revisions) (flutter/flutter#189428) 2026-07-14 [email protected] Roll Dart SDK from 2c587df8f05a to 05bf153370c4 (5 revisions) (flutter/flutter#189426) 2026-07-14 [email protected] [flutter_tools] Remove web hot reload flag (flutter/flutter#185994) 2026-07-14 [email protected] [iOS] Fix flaky keyboard animation test (flutter/flutter#189353) 2026-07-14 [email protected] [Impeller] Playground expanded role (flutter/flutter#188889) 2026-07-14 [email protected] Roll pub packages (flutter/flutter#189409) 2026-07-13 [email protected] Update lock-threads dependency to 6.0.2 (flutter/flutter#189053) 2026-07-13 49699333+dependabot[bot]@users.noreply.github.com Bump actions/labeler from 6.1.0 to 6.2.0 in the all-github-actions group (flutter/flutter#189396) 2026-07-13 [email protected] Remove outdated todo about `analysis bug on Windows` and update condition to also perform `analysis on windows` (flutter/flutter#189283) 2026-07-13 [email protected] Add more 0x0 size tests part 4 (flutter/flutter#185187) 2026-07-13 [email protected] Roll Packages from 20928d5 to ad2eab1 (18 revisions) (flutter/flutter#189387) 2026-07-13 [email protected] [flutter_tools] Fix ADB device listing output parsing regression (flutter/flutter#189369) 2026-07-13 [email protected] Stop running most Mac x64 builders that have Mac ARM equivalents on master (flutter/flutter#189301) 2026-07-13 [email protected] Move a few benchmarks from x64 Intel Macs to ARM (flutter/flutter#189377) 2026-07-13 [email protected] Add note that `hcpp` needs impeller (flutter/flutter#189382) 2026-07-13 [email protected] Roll Fuchsia Linux SDK from vhIlDkWIy21IrlB9E... to oOETA0ISPouDt2xBo... (flutter/flutter#189349) 2026-07-13 [email protected] [flutter_tools] Respect mustMatchAppBuild on Windows native assets (flutter/flutter#186788) 2026-07-13 [email protected] Roll Skia from 8bf65996caba to 3d1fc554f1a2 (2 revisions) (flutter/flutter#189350) 2026-07-13 [email protected] Roll Dart SDK from 0fc1668c4af4 to 2c587df8f05a (9 revisions) (flutter/flutter#189351) 2026-07-13 [email protected] [web] Fall back to full CJK fonts for characters not covered by split slices (flutter/flutter#188890) 2026-07-13 [email protected] Take Mac tool_integration_tests_* out of bringup (flutter/flutter#189368) 2026-07-13 [email protected] [hooks] Roll record_use to 1.0 and unpin (flutter/flutter#189366) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
testKeyboardAnimationWillWaitUIThreadVsyncposted asleep(1)task to the UI task runner and asserted that the keyboard animation callback did not fire until at least that delay had elapsed. Whether or not the callback actually observed the delay depended on thread scheduling, which made the test flaky.The property the old test was attempting to verify, that the callback does not run until in-flight UI-thread work completes, is not implemented by this code, so there is nothing here for a test to protect. This is guaranteed by the run-loop itself. The vsync client registers its
CADisplayLinkon the UI task runner's run loop (seesetUpKeyboardAnimationVsyncClientinFlutterKeyboardInsetManagerandVSyncClient.init), and a run loop services one source at a time, so the display-link callback can't possibly run while the UI thread is busy processing a begin-frame event. Asserting that ordering means assertingCFRunLoopandCADisplayLinkscheduling semantics, which is not only outside our control but inherently racy/fragile, which causes flakes.What this code does enforce is that delivery is asynchronous. The callback is wrapped in
dispatch_async(dispatch_get_main_queue())rather than invoked inline from-onDisplayLink:. This updates the test to trigger the display link directly, asserts the callback has not run synchronously, then waits on anXCTestExpectationand asserts that it's eventually delivered on the main queue.The view controller is now built with
FlutterEnginePartialMock, whoseuiTaskRunnerruns on the test's own message loop, so the vsync client's display-link registration and invalidation execute deterministically on the current thread. I've also renamed the test totestKeyboardAnimationCallbackIsDeliveredAsynchronouslyto match what it actually verifies.As mentioned above, this intentionally drops coverage of the wait-for-UI-thread behavior originally added for
#120555. As noted, that's already guaranteed by the run-loop and unrelated to our own logic.
Issue: #189352
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.