Migrate these tests to the "new" API#7189
Conversation
Hixie
requested review from
GaryQian,
LouiseHsu,
bparrishMines,
camsim99,
cbracken,
cyanglaz,
hellohuanlin,
jmagman,
stuartmorgan-g,
tarrinneal and
vashworth
as code owners
github-actions
bot
added
p: camera
p: file_selector
p: google_maps_flutter
p: google_sign_in
p: image_picker
p: in_app_purchase
p: ios_platform_images
p: local_auth
p: path_provider
p: quick_actions
p: shared_preferences
p: url_launcher
p: video_player
p: webview_flutter
ditman
left a comment
ditman
left a comment
There was a problem hiding this comment.
LGTM! I left a couple of notes/"bookmarks" in case somebody else wants to take a look.
| _ambiguate(TestDefaultBinaryMessengerBinding.instance)! | ||
| .defaultBinaryMessenger | ||
| .setMockMethodCallHandler( | ||
| channel, |
There was a problem hiding this comment.
Slight deviation of the pattern here, expected:
_ambiguate(TestDefaultBinaryMessengerBinding.instance)!
.defaultBinaryMessenger
.setMockMethodCallhandler(
maps.ensureChannelInitialized(mapId), (MethodCall methodCall) {...(I think it's better extracting the channel to its own local, tbh)
There was a problem hiding this comment.
weird, i wonder why that happened.
There was a problem hiding this comment.
oh, i think this was the file i ended up looking at first, before i automated it, so this was done by hand
There was a problem hiding this comment.
to clarify, did you want me to make the changes consistent, or should i just land it as-is?
(this code will all get refactored in a few months anyway when the _ambiguate is removed.)
There was a problem hiding this comment.
(i'm going to mark this autosubmit for now given the repo merge, happy to land a separate PR if you think it's worth making the call sites more consistent)
There was a problem hiding this comment.
@Hixie nah, I just left these comments in case others wanted to look at what was "different" from the more automatic/mechanic changes. Thanks for landing this!
| _ambiguate(TestDefaultBinaryMessengerBinding.instance)! | ||
| .defaultBinaryMessenger | ||
| .setMockMessageHandler( | ||
| 'flutter.io/videoPlayer/videoEvents123', | ||
| (ByteData? message) async { | ||
| final MethodCall methodCall = | ||
| const StandardMethodCodec().decodeMethodCall(message); | ||
| if (methodCall.method == 'listen') { | ||
| await _ambiguate(ServicesBinding.instance) | ||
| ?.defaultBinaryMessenger | ||
| await _ambiguate(TestDefaultBinaryMessengerBinding.instance)! | ||
| .defaultBinaryMessenger | ||
| .handlePlatformMessage( | ||
| 'flutter.io/videoPlayer/videoEvents123', | ||
| const StandardMethodCodec() |
There was a problem hiding this comment.
I wonder why these couple of tests are using .setMockMessageHandler instead of .setMockMethodCallHandler... Is it so this can use a custom codec to decode the method call line 243?
| /// | ||
| /// We use this so that APIs that have become non-nullable can still be used | ||
| /// with `!` and `?` on the stable branch. | ||
| T? _ambiguate<T>(T? value) => value; |
There was a problem hiding this comment.
yeah, and with a vengeance. :-/
it'll disappear again after the next stable, if this all goes according to plan.
| _ambiguate(TestDefaultBinaryMessengerBinding.instance)! | ||
| .defaultBinaryMessenger | ||
| .setMockMethodCallHandler( | ||
| plugin.channel, | ||
| (MethodCall methodCall) async { | ||
| log.add(methodCall); | ||
| return null; | ||
| }, | ||
| ); |
There was a problem hiding this comment.
This is what most of the changes on this PR look like:
- x.y.setMockMethodCallHandler(handler);
+ _ambiguate(TestDefaultBinaryMessengerBinding.instance)!
+ .defaultBinaryMessenger
+ .setMockMethodCallHandler(x.y, handler)
* 3c7d54bba [camerax] Implement camera preview (flutter/plugins#7112) * 48f50b4f1 [image_picker] Update NSPhotoLibraryUsageDescription description in README (flutter/plugins#6589) * eea17c996 Migrate these tests to the "new" API. (flutter/plugins#7189) * 190c6d916 Roll Flutter from 298d8c7 to 0be7c3f (21 revisions) (flutter/plugins#7194) * c0d4e8041 [google_sign_in] Endorses next web package. (flutter/plugins#7191) * cc4eaba0f [google_maps]: Bump org.mockito:mockito-core (flutter/plugins#7099) * 717a8bfef [image_picker]: Bump org.mockito:mockito-core (flutter/plugins#7097) * 8a09d8c13 [lifecycle]: Bump org.mockito:mockito-core (flutter/plugins#7096) * 40377a12a [in_app_pur]: Bump org.mockito:mockito-core (flutter/plugins#7094) * 6a4bbf1df [url_launcher]: Bump org.mockito:mockito-core (flutter/plugins#7098) * 96ab5cd12 Update codeowners (flutter/plugins#7188) * 00d5855cc Add missing CODEOWNER (flutter/plugins#7016) * c3e9d1ba3 [webview_flutter] Adds examples of accessing platform-specific features for each class (flutter/plugins#7089) * 1f7b57917 Roll Flutter from 0be7c3f to 33e4d21 (5 revisions) (flutter/plugins#7196) * 1acaf55c2 [plugins] Disables the AndroidGradlePluginVersion issue ID in all android packages (flutter/plugins#7045) * 132d9c77d [espresso] Update some dependencies (flutter/plugins#7195)
2 participants
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).