Skip to content

Commit 6531740

Browse files
stuartmorgan-gstuartmorgan-g
authored
[video_player] Remove test code from platform interface (flutter#4589)
This is a breaking change to video_player_platform_interface to remove Pigeon mocks from the public interface. These should never have been there, as tests in packages outside of the platform interface should be mocking the platform interface rather than the method channel, but the app-facing tests weren't rewritten when they should have been so they ended up published to support the legacy tests. This creates an unwanted non-dev dependency on `flutter_test`, in addition to promoting an anti-pattern. While making the breaking change, this also adopts `PlatformInterface`, removing `isMock` in favor of the standard mixin pattern provided by the base class. Part of flutter/flutter#83562
1 parent 29b6373 commit 6531740

File tree

5 files changed

+25
-41
lines changed

5 files changed

+25
-41
lines changed

‎packages/video_player/video_player_platform_interface/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
## 5.0.0
2+
3+
* **BREAKING CHANGES**:
4+
* Updates to extending `PlatformInterface`. Removes `isMock`, in favor of the
5+
now-standard `MockPlatformInterfaceMixin`.
6+
* Removes test.dart from the public interface. Tests in other packages should
7+
mock `VideoPlatformInterface` rather than the method channel.
8+
19
## 4.2.0
210

311
* Add `contentUri` to `DataSourceType`.

‎packages/video_player/video_player_platform_interface/lib/video_player_platform_interface.dart

Lines changed: 10 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,9 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5-
import 'dart:async';
6-
import 'dart:ui';
7-
85
import 'package:flutter/foundation.dart';
96
import 'package:flutter/widgets.dart';
10-
import 'package:meta/meta.dart' show visibleForTesting;
7+
import 'package:plugin_platform_interface/plugin_platform_interface.dart';
118

129
import 'method_channel_video_player.dart';
1310

@@ -18,37 +15,24 @@ import 'method_channel_video_player.dart';
1815
/// (using `extends`) ensures that the subclass will get the default implementation, while
1916
/// platform implementations that `implements` this interface will be broken by newly added
2017
/// [VideoPlayerPlatform] methods.
21-
abstract class VideoPlayerPlatform {
22-
/// Only mock implementations should set this to true.
23-
///
24-
/// Mockito mocks are implementing this class with `implements` which is forbidden for anything
25-
/// other than mocks (see class docs). This property provides a backdoor for mockito mocks to
26-
/// skip the verification that the class isn't implemented with `implements`.
27-
@visibleForTesting
28-
bool get isMock => false;
18+
abstract class VideoPlayerPlatform extends PlatformInterface {
19+
/// Constructs a VideoPlayerPlatform.
20+
VideoPlayerPlatform() : super(token: _token);
21+
22+
static final Object _token = Object();
2923

3024
static VideoPlayerPlatform _instance = MethodChannelVideoPlayer();
3125

3226
/// The default instance of [VideoPlayerPlatform] to use.
3327
///
34-
/// Platform-specific plugins should override this with their own
35-
/// platform-specific class that extends [VideoPlayerPlatform] when they
36-
/// register themselves.
37-
///
3828
/// Defaults to [MethodChannelVideoPlayer].
3929
static VideoPlayerPlatform get instance => _instance;
4030

41-
// TODO(amirh): Extract common platform interface logic.
42-
// https://github.com/flutter/flutter/issues/43368
31+
/// Platform-specific plugins should override this with their own
32+
/// platform-specific class that extends [VideoPlayerPlatform] when they
33+
/// register themselves.
4334
static set instance(VideoPlayerPlatform instance) {
44-
if (!instance.isMock) {
45-
try {
46-
instance._verifyProvidesDefaultImplementations();
47-
} on NoSuchMethodError catch (_) {
48-
throw AssertionError(
49-
'Platform interfaces must not be implemented with `implements`');
50-
}
51-
}
35+
PlatformInterface.verifyToken(instance, _token);
5236
_instance = instance;
5337
}
5438

@@ -119,14 +103,6 @@ abstract class VideoPlayerPlatform {
119103
Future<void> setMixWithOthers(bool mixWithOthers) {
120104
throw UnimplementedError('setMixWithOthers() has not been implemented.');
121105
}
122-
123-
// This method makes sure that VideoPlayer isn't implemented with `implements`.
124-
//
125-
// See class doc for more details on why implementing this class is forbidden.
126-
//
127-
// This private method is called by the instance setter, which fails if the class is
128-
// implemented with `implements`.
129-
void _verifyProvidesDefaultImplementations() {}
130106
}
131107

132108
/// Description of the data source used to create an instance of

‎packages/video_player/video_player_platform_interface/pubspec.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ repository: https://github.com/flutter/plugins/tree/master/packages/video_player
44
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+video_player%22
55
# NOTE: We strongly prefer non-breaking changes, even at the expense of a
66
# less-clean API. See https://flutter.dev/go/platform-interface-breaking-changes
7-
version: 4.2.0
7+
version: 5.0.0
88

99
environment:
1010
sdk: ">=2.12.0 <3.0.0"
@@ -13,9 +13,9 @@ environment:
1313
dependencies:
1414
flutter:
1515
sdk: flutter
16-
flutter_test:
17-
sdk: flutter
18-
meta: ^1.3.0
16+
plugin_platform_interface: ^2.0.0
1917

2018
dev_dependencies:
19+
flutter_test:
20+
sdk: flutter
2121
pedantic: ^1.10.0

‎packages/video_player/video_player_platform_interface/test/method_channel_video_player_test.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@ import 'package:flutter/services.dart';
88
import 'package:flutter_test/flutter_test.dart';
99
import 'package:video_player_platform_interface/messages.dart';
1010
import 'package:video_player_platform_interface/method_channel_video_player.dart';
11-
import 'package:video_player_platform_interface/test.dart';
1211
import 'package:video_player_platform_interface/video_player_platform_interface.dart';
1312

13+
import 'test.dart';
14+
1415
class _ApiLogger implements TestHostVideoPlayerApi {
1516
final List<String> log = [];
1617
TextureMessage? textureMessage;

‎packages/video_player/video_player_platform_interface/lib/test.dart renamed to ‎packages/video_player/video_player_platform_interface/test/test.dart

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,7 @@ import 'dart:async';
1010
import 'dart:typed_data' show Uint8List, Int32List, Int64List, Float64List;
1111
import 'package:flutter/services.dart';
1212
import 'package:flutter_test/flutter_test.dart';
13-
14-
import 'messages.dart';
13+
import 'package:video_player_platform_interface/messages.dart';
1514

1615
abstract class TestHostVideoPlayerApi {
1716
void initialize();

0 commit comments

Comments
 (0)