[video_player] add allowBackgroundPlayback option platform interface changes #4807
Conversation
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
IlyaMax
changed the title
|
@stuartmorgan this part is ready for review |
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
Actually, sorry, this should have a simple test.
Historically we do test coverage in the platform interface packages via the method channel implementation, but since we're no longer using the method channel in this package and don't plan to update it I think it would be good to do simple tests for the type classes.
Could you add a new unit test file for VideoPlayerOptions with some very simple tests (defaults to false, passing true works)? That gives us coverage here, and a foundation for any logic that might be added in the future (e.g., if we implement == for this at some point).
stuartmorgan-g
added
last mile
waiting for tree to go green
|
@godofredoc Do you have any idea how this landed without any of the presubmit Cirrus tests having actually run? |
The autosubmit bot does not check explicitly for tests it just assumes the relevant checks are there and validate they all passed. |
|
So does that mean none of the Cirrus tests was even queued within the hour between posting and landing? That seems like a really bad failure case. |
* google/master: (340 commits) [camera]remove "selfRef" for SavePhotoDelegate and ensure thread safety (flutter#4780) Roll Flutter from 919d205 to adafd66 (5 revisions) (flutter#4876) [google_sign_in] Update platform interface analysis options (flutter#4872) Roll Flutter from b623279 to 919d205 (2 revisions) (flutter#4871) Roll Flutter from 14a2b13 to b623279 (5 revisions) (flutter#4870) [image_picker] Update platform interface analysis options (flutter#4837) [ci] Re-enable stable webview Android tests (flutter#4867) Roll Flutter from 286c975 to 14a2b13 (1 revision) (flutter#4865) [local_auth] support localizedFallbackTitle in IOSAuthMessages (flutter#3806) [image_picker] Update app-facing and web analysis options (flutter#4838) Roll Flutter from 8386344 to 286c975 (4 revisions) (flutter#4864) Roll Flutter from e9f83cf to 8386344 (6 revisions) (flutter#4862) [camera] Switch web package to new analysis options (flutter#4834) [flutter_plugin_tool] Fix iOS/macOS naming (flutter#4861) [webview_flutter] Fix debuggingEnabled on Android (flutter#4859) Roll Flutter from ca2a751 to e9f83cf (10 revisions) (flutter#4857) fix license (flutter#4858) [video_player] add allowBackgroundPlayback option platform interface changes (flutter#4807) [video_player] Updated Pigeon version (flutter#4726) Roll Flutter from 381cb28 to ca2a751 (5 revisions) (flutter#4850) ... # Conflicts: # packages/camera/camera/CHANGELOG.md # packages/camera/camera/android/build.gradle # packages/camera/camera/android/src/main/java/io/flutter/plugins/camera/Camera.java # packages/camera/camera/android/src/main/java/io/flutter/plugins/camera/MethodCallHandlerImpl.java # packages/camera/camera/ios/Classes/CameraPlugin.m # packages/camera/camera/ios/camera.podspec # packages/camera/camera/lib/camera.dart # packages/camera/camera/lib/src/camera_controller.dart # packages/camera/camera/pubspec.yaml
5 participants
Part of this pr #4324
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
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.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.