Android: checkpoint the applied pause position, fixing the flaky isolation test (#86) #22

Merged

backmeupplz merged 3 commits from fix/86-flaky-checkpoint-assert into main

2026-10-12 22:06:42 +00:00

backmeupplz commented

2026-10-12 17:38:23 +00:00

(Migrated from github.com)

Fixes agentboard #86 (https://agentboard.icefish-betta.ts.net/#86).

Cause

AccountPlayerIsolationTest.expiredSession… flaked at the checkpoint assertion (CI run on PR #20). ExoPlayer fires onIsPlayingChanged(false) synchronously from its masked state. If the app looper hasn't yet received the playback thread's update after play(), that masked state still holds the seek position (350 ms) while the playback thread has already rendered to 400 ms. The pause checkpoint then stores 0.35, and once the pause is applied the player reports 0.40 with no further callback. Locally the original test failed 8 of 51 iterations under CPU load with exactly atPause=0.35 stored=0.35 settled=0.4.

Fix

PlayerService.progressListener: keep the immediate pause checkpoint, then send an ExoPlayer PlayerMessage that is delivered on the app looper after the playback thread has applied the pause (its state update is posted to the app looper before the message). If the player is still paused and not ended, it checkpoints again, so the stored position matches the position playback actually stopped at. sync's existing scope and media-id guards still apply.
Test: replace drainUntil { player.isPlaying } with assertTrue(player.isPlaying) so the app looper does not idle between play() and pause(). This forces the stale-mask ordering every time instead of in about 15% of runs. The assertions are unchanged.

Verification

With the strengthened test looped 30× under CPU load, old code fails 30/30 (expected:<0.4> but was:<0.35>) and the fix passes 30/30.
./gradlew testDebugUnitTest locally (JDK 27 compile, JDK 17 test runtime): every test passes except 23 ProgressSyncTest cases. Those fail only because HttpClient isn't AutoCloseable on JDK 17 (the local runtime). CI uses JDK 21.

Fixes agentboard #86 (https://agentboard.icefish-betta.ts.net/#86). ## Cause `AccountPlayerIsolationTest.expiredSession…` flaked at the checkpoint assertion (CI run on PR #20). ExoPlayer fires `onIsPlayingChanged(false)` synchronously from its *masked* state. If the app looper hasn't yet received the playback thread's update after `play()`, that masked state still holds the seek position (350 ms) while the playback thread has already rendered to 400 ms. The pause checkpoint then stores 0.35, and once the pause is applied the player reports 0.40 with no further callback. Locally the original test failed 8 of 51 iterations under CPU load with exactly `atPause=0.35 stored=0.35 settled=0.4`. ## Fix - `PlayerService.progressListener`: keep the immediate pause checkpoint, then send an ExoPlayer `PlayerMessage` that is delivered on the app looper after the playback thread has applied the pause (its state update is posted to the app looper before the message). If the player is still paused and not ended, it checkpoints again, so the stored position matches the position playback actually stopped at. `sync`'s existing scope and media-id guards still apply. - Test: replace `drainUntil { player.isPlaying }` with `assertTrue(player.isPlaying)` so the app looper does not idle between `play()` and `pause()`. This forces the stale-mask ordering every time instead of in about 15% of runs. The assertions are unchanged. ## Verification - With the strengthened test looped 30× under CPU load, old code fails 30/30 (`expected:<0.4> but was:<0.35>`) and the fix passes 30/30. - `./gradlew testDebugUnitTest` locally (JDK 27 compile, JDK 17 test runtime): every test passes except 23 `ProgressSyncTest` cases. Those fail only because `HttpClient` isn't `AutoCloseable` on JDK 17 (the local runtime). CI uses JDK 21.