[go: nahoru, domu]

Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

Migrate these tests to the "new" API #7189

Merged
merged 1 commit into from
Feb 17, 2023
Merged

Conversation

Hixie
Copy link
Contributor
@Hixie Hixie commented Feb 16, 2023

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the relevant style guides and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
  • I signed the CLA.
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.
  • I updated CHANGELOG.md to add a description of the change, following repository CHANGELOG style.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

Copy link
Member
@ditman ditman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! I left a couple of notes/"bookmarks" in case somebody else wants to take a look.

_ambiguate(TestDefaultBinaryMessengerBinding.instance)!
.defaultBinaryMessenger
.setMockMethodCallHandler(
channel,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

weird, i wonder why that happened.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, i think this was the file i ended up looking at first, before i automated it, so this was done by hand

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(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)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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!

Comment on lines +237 to 249
_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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no idea.

///
/// 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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ambiguate is back!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, and with a vengeance. :-/
it'll disappear again after the next stable, if this all goes according to plan.

Comment on lines +19 to +27
_ambiguate(TestDefaultBinaryMessengerBinding.instance)!
.defaultBinaryMessenger
.setMockMethodCallHandler(
plugin.channel,
(MethodCall methodCall) async {
log.add(methodCall);
return null;
},
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what most of the changes on this PR look like:

- x.y.setMockMethodCallHandler(handler);
+ _ambiguate(TestDefaultBinaryMessengerBinding.instance)!
+  .defaultBinaryMessenger
+  .setMockMethodCallHandler(x.y, handler)

@Hixie Hixie added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 17, 2023
@auto-submit auto-submit bot merged commit eea17c9 into flutter:main Feb 17, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 20, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 20, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 20, 2023
* 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)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants