[audioplayers] Update integration tests to upstream v6.6.0#1036
Open
seungsoo47 wants to merge 10 commits into
Open
[audioplayers] Update integration tests to upstream v6.6.0#1036seungsoo47 wants to merge 10 commits into
seungsoo47 wants to merge 10 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the integration tests and Tizen platform implementation for the audioplayers plugin. Key changes include restricting data-driven tests to local asset sources on Tizen, introducing platform-specific feature flags, ensuring callbacks are executed on the main loop, and returning null for duration and position when the source is not prepared. Feedback on these changes suggests optimizing the test data list to avoid unnecessary network downloads, removing a redundant seek operation during player stop, correcting a misleading test name, and awaiting asynchronous stream expectations to prevent test flakiness.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
20 tasks
seungsoo47
force-pushed
the
audioplayers-integration-test-upstream-v6.6.0
branch
from
38f0cb1 to
e523ad7
Compare
Wholesale-replace the Tizen integration test with the upstream audioplayers v6.6.0 main test (example/integration_test/lib_test.dart) and its helper files, keeping only test cases that actually run on Tizen. Mode: wholesale replace (Path A). The previous Tizen-specific 'asset audio' test group was superseded by the upstream-derived suite. Only the upstream main test file was ported; the sub files (platform_test.dart, app_test.dart) are not yet consolidated. Tizen reports as TargetPlatform.linux at runtime, so PlatformFeatures resolves to linuxPlatformFeatures (bytes/lowLatency/respectSilence/dataUri = false). Ported helper files: - source_test_data.dart, test_utils.dart, platform_features.dart - lib/lib_source_test_data.dart, lib/lib_test_utils.dart Added assets (copies of coins.wav, matching upstream): coins_no_extension, coins_non_ascii_и.wav. Passing test cases (validated on TV/rpi4 device): - test asset source with special char - test device file source with special char - test url source with no extension - data URI source - AP events: #positionEvent with Timer/FramePositionUpdater (asset source) - play multiple sources: simultaneously / consecutively Skipped (platform-gated, not supported on Tizen/linux): - bytes array source, Audio Context (respectSilence / LOW_LATENCY), Android-only LOW_LATENCY release Removed after on-device validation (fail on Tizen; not rolled back wholesale): - test url source with special char (remote URL playback never completes) - AP events #positionEvent for URL/HLS/live-stream sources (timeout / position never resets) -> data-driven loops restricted to local asset sources - play multiple sources with URL sources (timeout) - Set global AudioContextConfig on unsupported platforms (MissingPluginException: setAudioContext not implemented on the global channel) - Race condition on play and pause (#1687) (remote mp3 + animation teardown) Note: only Tizen-runnable tests are included. Network-based cases were dropped even though the device has connectivity, because remote playback does not emit reliable complete/position events on Tizen. CI policy unchanged (recipe.yaml keeps audioplayers under ["tv-9.0"]). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Platform-specific improvements: - Fix position not resetting to 0 after Stop() on Tizen (audio_player.cc) - Handle setAudioContext gracefully instead of throwing MissingPluginException - Add tests for unsupported platform AudioContext and race condition scenarios Changes: 1. audio_player.cc: Call Seek(0) after Stop() to match other platforms 2. audioplayers_tizen_plugin.cc: Return Success() for setAudioContext calls with log emit 3. audioplayers_test.dart: Add 2 new test cases - Set global AudioContextConfig on unsupported platforms - Race condition on play and pause with asset source Test results: All tests passed (11 passed, 4 skipped) Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
… Mux stream The Apple test stream (https://ll-hls-test.cdn-apple.com/) returns HTTP 403 on most devices, preventing HLS testing. Replace with Mux's public test stream which is accessible globally. This enables flutter-tizen#4 (positionEvent with FramePositionUpdater: m3u8) test to run when re-added. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Remove the 4 test cases that are always skipped when run via flutter-tizen drive, since Tizen resolves to linuxPlatformFeatures (hasBytesSource, hasRespectSilence, hasLowLatency = false): - bytes array source - Audio Context: test changing AudioContextConfigs - Audio Context: test changing AudioContextConfigs in LOW_LATENCY mode - Android only: Released wrong source on LOW_LATENCY (#1672) Adding a Tizen-specific PlatformFeatures branch to force these on was evaluated on device: bytes source aborts test registration (remote bytes download during setup), the LOW_LATENCY AudioContext test fails (Tizen does not suppress the complete event), and AudioContext is a no-op on Tizen. So the skips are correct and the tests are removed rather than enabled. Validated on TV/rpi4: all tests passed (10 passed, 0 skipped, 0 failed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…epared On Tizen, release() unprepares the player (PLAYER_STATE_IDLE), where player_get_duration() / player_get_play_position() return 0 without error. As a result getDuration / getCurrentPosition returned 0 after release, instead of null as on other platforms. Add AudioPlayer::IsSourcePrepared() (true only in READY/PLAYING/PAUSED) and return null from the getDuration / getCurrentPosition method-channel handlers when no source is prepared. stop() (ReleaseMode.stop, READY state) still returns position 0, matching other platforms. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… v6.6.0 Consolidate Tizen-runnable cases from upstream platform_test.dart into the single audioplayers_test.dart (lib_test remains the main entry point). Platform method channel: #create and #dispose (Tizen-specific exception message), #volume, #playbackRate, #seek with millisecond precision, #ReleaseMode.loop, #ReleaseMode.release, #release. Platform event channel: #completeEvent, Listen and cancel twice, Emit platform log, Emit global platform log, Emit platform error, Emit global platform error. Source-driven cases use the asset-only list to avoid unreliable remote playback on Tizen. Excluded as unsupported on Tizen: - #balance (setBalance not implemented: MissingPluginException) - loading an invalid file (no failure event emitted: times out) Adds audioplayers_platform_interface as a direct dependency. The #ReleaseMode.release / #release cases rely on the null duration/position fix in the previous commit. Validated on TV/rpi4: all tests passed (23 passed, 0 skipped, 0 failed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two issues observed on TV with the example app: 1. Crash: stopping a (network) source threw PlatformException(player_set_play_position failed, Invalid state) from Stop() -> Seek(0). The position reset after stop is best-effort, so wrap it in try/catch and log instead of letting it propagate and crash the app. 2. Threading: OnError and OnInterrupted ran on the player's callback thread (not the main loop on TV) and emitted log events directly, triggering "channel sent a message from native to Flutter on a non-platform thread". Marshal both to the main loop via ecore_main_loop_thread_safe_call_async, like the other player callbacks. OnError carries its message through a heap-allocated context. Validated on TV and rpi4: integration tests still pass (23 passed, 0 skipped, 0 failed). The fixed paths (network-source stop, player error/interrupt) are exercised by the example app rather than the asset-based integration tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ator CI The integration test failed on the tv-9.0 emulator with: Bad state: Can't call test() once tests have begun running. main() awaited getAudioTestDataList() before declaring any testWidgets(), yielding to the event loop. On the slower TV emulator the test runner started before the post-await declarations ran, so testWidgets() was rejected. Real devices (TV 10.0, RPI4) happened to win the race and passed. Only local asset sources are used anyway (remote playback is unreliable on Tizen), so build the asset list synchronously and drop the await. main() is now synchronous and all tests are declared before the runner starts. This also removes the now-unused remote/bytes path from getAudioTestDataList(). Validated on TV and RPI4: all tests pass (23 passed, 0 skipped, 0 failed). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
seungsoo47
force-pushed
the
audioplayers-integration-test-upstream-v6.6.0
branch
from
e523ad7 to
e73c530
Compare
ecore_main_loop_thread_safe_call_async was removed in the Ecore API cleanup (flutter-tizen#1033). Replace the two remaining usages in OnInterrupted and OnError with g_idle_add_full, consistent with OnPrepared, OnSeekCompleted, and OnPlayCompleted. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
JSUYA
reviewed
Jun 18, 2026
Comment on lines
+57
to
+60
| // Whether a source is prepared, i.e. duration and position are meaningful. | ||
| // After release (or before a source is prepared) this is false, so the | ||
| // plugin returns null for duration/position to match other platforms. | ||
| bool IsSourcePrepared(); |
Member
There was a problem hiding this comment.
The function's internal code and name sufficiently explain its behavior, so automatically generated comments are not necessary. Please delete any automatically generated comments unnecessary.
| // player_set_play_position right after stop can fail with an invalid | ||
| // state, which must not crash the app. | ||
| try { | ||
| Seek(0); |
Member
There was a problem hiding this comment.
This affects existing behavior. I think, if it is a platform-dependent issue, it should be handled within the player_stop() API itself.
|
|
||
| /// Specify supported features for a platform. | ||
| class PlatformFeatures { | ||
| static const webPlatformFeatures = PlatformFeatures( |
Member
There was a problem hiding this comment.
We do not need to have test code related to other platforms within our test code.
Comment on lines
+147
to
+161
| factory PlatformFeatures.instance() { | ||
| return kIsWeb | ||
| ? webPlatformFeatures | ||
| : defaultTargetPlatform == TargetPlatform.android | ||
| ? androidPlatformFeatures | ||
| : defaultTargetPlatform == TargetPlatform.iOS | ||
| ? iosPlatformFeatures | ||
| : defaultTargetPlatform == TargetPlatform.macOS | ||
| ? macPlatformFeatures | ||
| : defaultTargetPlatform == TargetPlatform.linux | ||
| ? linuxPlatformFeatures | ||
| : defaultTargetPlatform == TargetPlatform.windows | ||
| ? windowsPlatformFeatures | ||
| : const PlatformFeatures(); | ||
| } |
Member
There was a problem hiding this comment.
It seems it will return Linux here. You need to check if the support range is correct. For example, hasBalance
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Updates the audioplayers integration tests to match upstream v6.6.0, and fixes
the plugin issues surfaced while validating them on device (Tizen TV / RPI4).
Integration tests
lib_test+platform_test) into a singleaudioplayers_test.dart, keeping only Tizen-runnable cases.playback does not emit reliable complete/position events on Tizen.
semantics,
setBalance, invalid-file failure event).Plugin fixes
nullfor duration/position when no source is prepared (e.g. afterrelease()), matching other platforms.Stop()→Seek(0)threwInvalid statefor networksources; the position reset after stop is now best-effort.
OnError/OnInterruptedon the platform thread to avoid the"channel sent a message ... on a non-platform thread" warning.
Tested on Tizen TV (10.0) and Raspberry Pi 4: all integration tests pass
(23 passed, 0 skipped, 0 failed).
#1039