Test suite

.github/workflows/test.yml

@@ -0,0 +1,29 @@
+name: test
+
+# Consuming apps run their own test suites, not this package's, so without this
+# workflow nothing enforces them. bcc-media-app's Semaphore `Test` block does
+# fire when `/submodules/` changes, but it only runs that app's own `test/` —
+# a regression in here reaches every consuming app unchallenged.
+on:
+  push:
+    branches: [main]
+  pull_request:
+
+jobs:
+  bccm_player:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: subosito/flutter-action@v2
+        with:
+          # Matches the `flutter: ">=3.44.0"` constraint in pubspec.yaml.
+          flutter-version: 3.44.0
+          channel: stable
+          cache: true
+
+      - run: flutter pub get
+
+      - run: flutter analyze
+
+      - run: flutter test

analysis_options.yaml

@@ -6,4 +6,9 @@ include: package:flutter_lints/flutter.yaml
 analyzer:
   errors:
     body_might_complete_normally_nullable: ignore
-  exclude: [lib/**.freezed.dart, lib/**.g.dart]
+  exclude:
+    - lib/**.freezed.dart
+    - lib/**.g.dart
+    - build/**
+    - android/**
+    - ios/**

doc/contributing/basics.md

@@ -27,3 +27,33 @@ dart run pigeon --input pigeons/chromecast_pigeon.dart
 ```
 
 You will likely need to add things to the pigeons if you are building new features that require writing native code in swift/kotlin.
+
+#### Running tests
+
+The Dart suite lives in `test/` and runs on every PR via `.github/workflows/test.yml`:
+
+```sh
+flutter test
+
+# Warnings and errors are fatal; the package still carries some pre-existing
+# deprecation infos, hence the flag. This is what CI runs.
+flutter analyze --no-fatal-infos
+```
+
+There are also Kotlin unit tests (Robolectric) under `android/src/test/`. They need the
+Flutter-generated Gradle project, so they run from the example app rather than from
+`android/` directly:
+
+```sh
+cd example/android && ./gradlew :bccm_player:testDebugUnitTest
+```
+
+These are not in CI yet — run them by hand if you touch `ExoPlayerController` or the
+player-view lifecycle. iOS has no test target.
+
+When adding tests, note two things that will bite you otherwise:
+
+- `MediaItem` and `Track` are generated pigeon classes with no `==`/`hashCode`, so they
+  compare by identity. Assert on `id`/`url`, not on whole objects.
+- `PlayerStateNotifier`'s constructor starts a periodic timer. Call `dispose(force: true)`
+  or run inside `fakeAsync`, or the test fails on a pending timer.

doc/contributing/todo.md

@@ -0,0 +1,45 @@
+# TODO
+
+Known gaps, roughly in order of value. Each is self-contained — none blocks the others.
+
+Queue and audio work is tracked separately in [audio-support-plan.md](audio-support-plan.md); this list is everything outside that.
+
+## Migrate the web player off `dart:html`
+
+[`lib/src/web/video_js_player.dart`](../../lib/src/web/video_js_player.dart) is ~120 lines of DOM code on the deprecated `dart:html`. Moving to `package:web` + `dart:js_interop` needs a new dependency, `ui_web.platformViewRegistry` in place of `dart:ui`'s, and a replacement for `NodeTreeSanitizer.trusted`.
+
+Only a real web build can verify it — no Dart test reaches this file. It carries the single inline `// ignore: deprecated_member_use` in the package, so this is the last thing between us and an unqualified strict `flutter analyze`.
+
+## Extract track selection out of `_SettingsBottomSheet`
+
+[`lib/src/widgets/controls/default/settings.dart`](../../lib/src/widgets/controls/default/settings.dart) hides real logic inside a widget build method: offline filtering (`downloaded == null || downloaded == true`), unique-height video-track dedupe, the selected-track lookup, re-adding a selected-but-filtered audio track, and `autoTrackId` handling.
+
+All of it is pure and none of it is testable where it sits. Pulling it into a function is the biggest remaining coverage win in the package.
+
+## Make `tv_controls.dart` DVR-aware
+
+[`lib/src/widgets/controls/tv/tv_controls.dart`](../../lib/src/widgets/controls/tv/tv_controls.dart) reimplements `useTimeline` inline and ignores `seekableRangeStartMs` / `seekableRangeEndMs` entirely, so seeking a live DVR window is wrong on TV.
+
+Unlike the bug fixed in `default_controls`, it is at least self-consistent — its thumb and its drag agree with each other — so this is a missing feature rather than a mismatch. The fix is to make it call `useTimeline` and `positionFromFraction`, which also removes the duplication.
+
+## Stop the riverpod providers leaking notifiers
+
+[`lib/src/plugins/riverpod/providers/player_provider.dart`](../../lib/src/plugins/riverpod/providers/player_provider.dart) — both `playerProviderFor` and `primaryPlayerProvider` fall back to `PlayerStateNotifier(keepAlive: false)` when the player is absent. That constructor starts a periodic 1 s timer, and a fresh notifier is built on every rebuild, so each one leaks.
+
+## Native tests
+
+The six Robolectric tests in `android/src/test/` still aren't in CI. They need a JDK and the Android SDK, and run through the Flutter-generated Gradle project rather than from `android/` directly:
+
+```sh
+cd example/android && ./gradlew :bccm_player:testDebugUnitTest
+```
+
+iOS has no test target at all — `ios/bccm_player.podspec` has no `test_spec`.
+
+## Minor
+
+- `lib/src/widgets/utils/bccm_player_plugin_state_builder.dart` is dead code returning `Placeholder()` and isn't exported. Delete it.
+- `PlayerPluginStateNotifier._removePlayer` calls `debugPrint` unconditionally — noisy in test output and in release logs.
+- `StateNotifierSelectBuilder` compares selections with `!identical` rather than `!=`. Fine for enums, bools and small ints; a `select` that builds a `String` rebuilds on every notification regardless. Pinned by a test today, not a correctness bug.
+- `useWakelockWhilePlaying` holds the wakelock in every state except `paused` — including `stopped` and `error`. Needs a product decision, not just a code change.
+- The filename `lib/src/utils/use_wakelock_while_palying.dart` is misspelled.

example/analysis_options.yaml

@@ -7,6 +7,12 @@
 
 # The following line activates a set of recommended lints for Flutter apps,
 # packages, and plugins designed to encourage good coding practices.
+analyzer:
+  exclude:
+    - build/**
+    - android/**
+    - ios/**
+    - web/**
 include: package:flutter_lints/flutter.yaml
 
 linter:

example/lib/examples/downloader.dart

@@ -5,6 +5,7 @@ import 'package:bccm_player/bccm_player.dart';
 import 'package:bccm_player/controls.dart';
 import 'package:bccm_player_example/example_videos.dart';
 import 'package:flutter/material.dart';
+import 'package:flutter/rendering.dart';
 import 'package:flutter_hooks/flutter_hooks.dart';
 
 class Downloader extends StatefulWidget {
@@ -162,7 +163,7 @@ class _TrackSelection extends HookWidget {
     final selectedAudioTracks = useState<List<Track>>([]);
     final selectedVideoTracks = useState<List<Track>>([]);
     return ListView(
-      cacheExtent: 10000,
+      scrollCacheExtent: const ScrollCacheExtent.pixels(10000),
       shrinkWrap: true,
       children: [
         const Text("Media info"),

example/lib/examples/queue.dart

@@ -155,7 +155,10 @@ class QueueExample extends HookWidget {
                   }),
               Text('Queue items', style: Theme.of(context).textTheme.titleLarge),
               ReorderableListView.builder(
-                onReorder: (oldIndex, newIndex) {
+                // onReorderItem rather than the deprecated onReorder: it hands
+                // us a newIndex already adjusted for the removed item, which is
+                // what the body below (and QueueManager.moveQueueItem) assumes.
+                onReorderItem: (oldIndex, newIndex) {
                   tempQueueItems.value.insert(newIndex, tempQueueItems.value.removeAt(oldIndex));
                   controller.queue.moveQueueItem(oldIndex, newIndex);
                 },

example/pubspec.lock

@@ -39,7 +39,7 @@ packages:
       path: ".."
       relative: true
     source: path
-    version: "1.2.1"
+    version: "1.2.2"
   boolean_selector:
     dependency: transitive
     description:
@@ -60,10 +60,10 @@ packages:
     dependency: transitive
     description:
       name: build_config
-      sha256: "94eaf6708fe64408c632ef2689ca3777b112f9421306ccf4f8c84d7c5c9f83f8"
+      sha256: d466ed2dc9c6cd1d169948879b84ee061eb5e22c64a7c6089879c6296d272a8d
       url: "https://pub.dev"
     source: hosted
-    version: "1.3.2"
+    version: "1.3.3"
   characters:
     dependency: transitive
     description:
@@ -84,10 +84,10 @@ packages:
     dependency: transitive
     description:
       name: clock
-      sha256: fddb70d9b5277016c77a80201021d40a2247104d9f4aa7bab7157b7e3f05b84b
+      sha256: e51d50bca3217c9a9fa2b41a30e4a38971133f5f9ec7a3d57bae095007f1d28e
       url: "https://pub.dev"
     source: hosted
-    version: "1.1.2"
+    version: "1.1.3"
   collection:
     dependency: transitive
     description:
@@ -283,10 +283,10 @@ packages:
     dependency: transitive
     description:
       name: glob
-      sha256: c3f1ee72c96f8f78935e18aa8cecced9ab132419e8625dc187e1c2408efc20de
+      sha256: "218aeb56050c714f62a3182775320dfa04602b55074873e24e31bbd39bda96fb"
       url: "https://pub.dev"
     source: hosted
-    version: "2.1.3"
+    version: "2.2.0"
   hooks_riverpod:
     dependency: "direct main"
     description:
@@ -371,10 +371,10 @@ packages:
     dependency: transitive
     description:
       name: matcher
-      sha256: dc0b7dc7651697ea4ff3e69ef44b0407ea32c487a39fff6a4004fa585e901861
+      sha256: "31bd099b47c10cd1aeb55146a2d46ce0277630ecef3f7dae54ad7873f36696cd"
       url: "https://pub.dev"
     source: hosted
-    version: "0.12.19"
+    version: "0.12.20"
   material_color_utilities:
     dependency: transitive
     description:
@@ -387,10 +387,10 @@ packages:
     dependency: transitive
     description:
       name: meta
-      sha256: "1741988757a65eb6b36abe716829688cf01910bbf91c34354ff7ec1c3de2b349"
+      sha256: "307249ce4ff29d58a18e97f6345f539382eb9c9c29ecda628900f31de0443dd9"
       url: "https://pub.dev"
     source: hosted
-    version: "1.18.0"
+    version: "1.19.0"
   nested:
     dependency: transitive
     description:
@@ -475,18 +475,18 @@ packages:
     dependency: transitive
     description:
       name: pub_semver
-      sha256: "5bfcf68ca79ef689f8990d1160781b4bad40a3bd5e5218ad4076ddb7f4081585"
+      sha256: "261236774e8b1d69cfc6b9eabbc96c40f25e7a2d6b171f3385d4f65d5734fb24"
       url: "https://pub.dev"
     source: hosted
-    version: "2.2.0"
+    version: "2.2.1"
   pubspec_parse:
     dependency: transitive
     description:
       name: pubspec_parse
-      sha256: "0560ba233314abbed0a48a2956f7f022cce7c3e1e73df540277da7544cad4082"
+      sha256: c38b81cbf34450b67e0265d73433569d12e34782e30ed769c9cc99c9d5f2e796
       url: "https://pub.dev"
     source: hosted
-    version: "1.5.0"
+    version: "1.6.0"
   riverpod:
     dependency: "direct main"
     description:
@@ -520,10 +520,10 @@ packages:
     dependency: transitive
     description:
       name: stack_trace
-      sha256: "8b27215b45d22309b5cddda1aa2b19bdfec9df0e765f2de506401c071d38d1b1"
+      sha256: "277654b3034d17ac6f9f1cb5595db011b1d5d41e8806866db28e0abaa101c490"
       url: "https://pub.dev"
     source: hosted
-    version: "1.12.1"
+    version: "1.12.2"
   state_notifier:
     dependency: transitive
     description:
@@ -560,10 +560,10 @@ packages:
     dependency: transitive
     description:
       name: test_api
-      sha256: "949a932224383300f01be9221c39180316445ecb8e7547f70a41a35bf421fb9e"
+      sha256: "2a122cbe059f8b610d3a5415f42e255b6c17b1f21eee1d960f31080237fb4f11"
       url: "https://pub.dev"
     source: hosted
-    version: "0.7.11"
+    version: "0.7.12"
   typed_data:
     dependency: transitive
     description:
@@ -616,10 +616,10 @@ packages:
     dependency: transitive
     description:
       name: vector_math
-      sha256: d530bd74fea330e6e364cda7a85019c434070188383e1cd8d9777ee586914c5b
+      sha256: f36f9f3be64c6198714492bb455c11056e33e2f85d9a0b676a48301e44fdcf47
       url: "https://pub.dev"
     source: hosted
-    version: "2.2.0"
+    version: "2.4.2"
   vm_service:
     dependency: transitive
     description:
@@ -632,18 +632,18 @@ packages:
     dependency: transitive
     description:
       name: wakelock_plus
-      sha256: "7253bca0fcf40d8413ddfcf4d2a1fa0a82475e79be25a4f2c564b695c9351486"
+      sha256: "22b3e7e937721de70e63c85e7139f4ac781dc22863b9262431a53ac030eb074b"
       url: "https://pub.dev"
     source: hosted
-    version: "1.7.0"
+    version: "1.8.0"
   wakelock_plus_platform_interface:
     dependency: transitive
     description:
       name: wakelock_plus_platform_interface
-      sha256: "0618d1799f0b28bcf98255b4ee8313e6fc4d38589dc4ee5fe5840d57d1aff6da"
+      sha256: "764c25504562abc8ac3406f5d175f126b276e6ba916c7c2675690b73368ca384"
       url: "https://pub.dev"
     source: hosted
-    version: "1.6.0"
+    version: "1.7.0"
   watcher:
     dependency: transitive
     description:
@@ -680,10 +680,10 @@ packages:
     dependency: transitive
     description:
       name: yaml
-      sha256: b9da305ac7c39faa3f030eccd175340f968459dae4af175130b3fc47e40d76ce
+      sha256: f67cdd8e07d3c6329146aaef1ba043542b3134c12489f553ca9a7435d1068aea
       url: "https://pub.dev"
     source: hosted
-    version: "3.1.3"
+    version: "3.1.4"
 sdks:
   dart: ">=3.12.0 <4.0.0"
   flutter: ">=3.44.0"

lib/bccm_player_web.dart

@@ -4,7 +4,6 @@
 
 import 'package:bccm_player/src/native/root_pigeon_playback_listener.dart';
 import 'package:bccm_player/src/pigeon/playback_platform_pigeon.g.dart' as pigeon;
-import 'package:bccm_player/src/pigeon/playback_platform_pigeon.g.dart';
 import 'package:flutter_web_plugins/flutter_web_plugins.dart';
 import 'bccm_player.dart';
 import 'src/web/video_js_player.dart';

lib/src/downloader_platform_interface.dart

@@ -1,6 +1,5 @@
 import 'dart:async';
 import 'package:bccm_player/src/pigeon/downloader_pigeon.g.dart';
-import 'package:collection/collection.dart';
 import 'package:plugin_platform_interface/plugin_platform_interface.dart';
 
 class DownloaderListener implements DownloaderListenerPigeon {
@@ -56,7 +55,7 @@ class DownloaderNative extends DownloaderInterface {
 
   @override
   Future<List<Download>> getDownloads() async {
-    return (await _pigeon.getDownloads()).whereNotNull().toList();
+    return (await _pigeon.getDownloads()).nonNulls.toList();
   }
 
   @override

lib/src/model/player_view_config.dart

@@ -68,7 +68,7 @@ class BccmPlayerViewConfig {
   /// * [deviceOrientationsFullscreen] is a callback used upon **entering** fullscreen to get the orientations to set. Return null for defaults.
   /// * [castPlayerBuilder] is a builder that will be used to build the cast player.
   const BccmPlayerViewConfig({
-    BccmPlayerControlsConfig? controlsConfig,
+    this._controlsConfig,
     this.useSurfaceView = false,
     this.allowSystemGestures = false,
     this.castPlayerBuilder,
@@ -80,7 +80,7 @@ class BccmPlayerViewConfig {
     this.pipOnLeave,
     this.videoFit,
     this.allowsVideoFrameAnalysis,
-  }) : _controlsConfig = controlsConfig;
+  });
 
   BccmPlayerViewConfig copyWith({
     BccmPlayerControlsConfig? controlsConfig,

lib/src/native/root_pigeon_playback_listener.dart

@@ -69,9 +69,10 @@ class RootPigeonPlaybackListener implements PlaybackListenerPigeon {
   }
 
   @override
-  void onPrimaryPlayerChanged(playerId) {
+  void onPrimaryPlayerChanged(event) {
+    _streamController.add(event);
     for (var listener in _listeners) {
-      listener.onPrimaryPlayerChanged(playerId);
+      listener.onPrimaryPlayerChanged(event);
     }
   }
 }