Repository navigation
Added a composable waitForCondition Driver/extension API - #37736
Conversation
fluttergithubbot
added
a: tests
| final Map<String, String> jsonMap = <String, String>{ | ||
| 'conditionName': 'CombinedCondition' | ||
| }; | ||
| final List<Map<String, String>> jsonConditions = <Map<String, String>>[]; |
There was a problem hiding this comment.
Could the following few lines be simplified by using List.map? e.g. conditions.map((WaitCondition c) => ...);
There was a problem hiding this comment.
The deserialization logic may also simplify with something like that.
There was a problem hiding this comment.
Simplified serialize(). Kept the deserialize() the same, as I didn't find a good solution to handle the type casting from the json.decode() resultdynamic => Map<String, dynamic>...
| bool get condition => SchedulerBinding.instance.transientCallbackCount == 0; | ||
|
|
||
| @override | ||
| Future<void> wait() async { |
There was a problem hiding this comment.
I'd still argue that the default is to wait until the next frame if the condition is currently unmet. FirstFrameRasterizedCondition can still override that to provide a custom impl
Piinks
added
customer: espresso
t: flutter driver
|
Ping for comments? |
|
Is there a link to the breaking change announcment for this? |
dnfield
left a comment
dnfield
left a comment
There was a problem hiding this comment.
Sorry for the late review - most of these should be small.
Thanks for checking! Will work on the announcement. |
dnfield
left a comment
dnfield
left a comment
There was a problem hiding this comment.
LGTM, but we should wait until some time after an announcement has been posted to make sure this doesn't break anyone too badly.
|
For sure. I'm going to send an announcement tomorrow, but this change probably won't break any Flutter users, except Espressso :), as Espresso calls the extension APIs directly. |
goderbauer
left a comment
goderbauer
left a comment
There was a problem hiding this comment.
LGTM
Please link to the breaking change announcement.
|
This broke a bunch of tests on Flutter's devicelab. Please revert. |
|
Oh, sorry for the breakage! Working on a revert. |
…tter#37736)" This reverts commit 8edc3f7.
This reverts commit 0e8ead0.
7 participants
Description
This PR added a waitForCondition API that can be used to compose conditions that the client would like to wait for. Currently supported conditions are:
In favor of this change, we deprecate the
_waitUntilNoTransientCallbacks,_waitUntilNoPendingFrame,_waitUntilFirstFrameRasterizedextension APIs.Related Issues
#37409
Tests
I added the following tests:
flutter_driver_test.dart
extension_test.dart
wait_test.dart
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?
It deprecates a few Flutter Driver extension wait APIs in favor of the new
waitForConditionAPI:_waitUntilNoTransientCallbacks&WaitUntilNoTransientCallbackscommand;_waitUntilNoPendingFrame&WaitUntilNoPendingFramecommand;_waitUntilFirstFrameRasterized&WaitUntilFirstFrameRasterizedcommand.Note, the user facing Driver API
waitUntilNoTransientCallbacks,waitUntilFirstFrameRasterizedare NOT deprecated, but under the hood, it useswaitForConditionto talk to the Dart service extension API. Thus, we expect minimum user impact unless they use the extension APIs directly (e.g. a testing framework like Espresso/EarlGrey).