-
Notifications
You must be signed in to change notification settings - Fork 308
test: Make home-page tests robust to changes in route animation duration #1547
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: Make home-page tests robust to changes in route animation duration #1547
Conversation
This will be useful for some other tests.
This test doesn't need to check the widget tree after waiting for the route animation to complete. The navigator observer is alerted when the navigation action is dispatched, not when its animation completes, so it's fine to check pushedRoutes before waiting through the animation. (It still needs a Duration.zero wait so that a FakeApiConnection timer isn't still pending at the end of the test.)
This duplicates the `find.descendant`s in `tester.tap` callsites, but that's temporary; we'll deduplicate with a new helper function, coming up.
This is robust to changes in the entrance-animation duration, and it checks the state more thoroughly.
As in the previous commit, this removes some more hard-coding of route-animation durations.
This should fix all the failing tests in flutter/flutter#165832. Discussion: flutter/flutter#165832 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, LGTM! Just one non-blocking comment.
|
||
final sheetPopDuration = (topRouteBeforePress as ModalBottomSheetRoute<void>) | ||
.reverseTransitionDuration; | ||
// TODO not sure why a 1ms fudge is needed; investigate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the example provided upstream also has the same 1ms hack: flutter/flutter#165832 (comment).
Also the value of sheetPopDuration
here turns out to be 200ms
, different from the previous 250ms
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we tried 200ms when writing the test, but it failed for the same (unknown) reason it fails now, and we added 50ms to hack around instead of just 1ms.
The CI failure is #1545; I have #1546 open for that. (That failure shouldn't affect the customer-tests run, because that's done with I'll go ahead and merge this and send a flutter/tests PR bumping the Zulip commit ID. Thanks for the review! Normally I wouldn't merge without Greg's review, but the changes are only in tests, Greg is OOO through Monday, and I'd like to unblock the upstream work. |
…ration This updates to the following change in Zulip: zulip/zulip-flutter@834834b zulip/zulip-flutter#1547 in order to unblock this PR: flutter/flutter#165832
…ration This updates to the following change in Zulip: zulip/zulip-flutter@834834b zulip/zulip-flutter#1547 in order to unblock this PR: flutter/flutter#165832
…ration (#460) This updates to the following change in Zulip: zulip/zulip-flutter@834834b zulip/zulip-flutter#1547 in order to unblock this PR: flutter/flutter#165832 > [!IMPORTANT] > This repository is a testing suite that contains references to tests (in the `registry` directory) that are run with every commit to Flutter > to verify that no breaking changes have been introduced (in the "customer_testing" shards). Your merged PR is _not_ automatically run in the > Flutter CI tree. > > After merging this PR, you must send another PR to flutter/flutter updating `dev/customer_testing/tests.version` in that repo with the latest git commit SHA of the flutter/test repo. > > See <flutter/flutter#162048> for details.
This would fix some tests to allow an upstream change to land.
Discussion; see Greg's comment: flutter/flutter#165832 (comment)
There are more places we use hard-coded durations, but those don't seem to be affected—the four test failures are all in
home_test.dart
.