605 bad commandersact stop position after removing item (#606)

SRGSSR · Jun 25, 2024 · 641dae4 · 641dae4
1 parent 5876f46
commit 641dae4
Show file tree

Hide file tree

Showing 5 changed files with 155 additions and 56 deletions.
diff --git a/pillarbox-core-business/build.gradle.kts b/pillarbox-core-business/build.gradle.kts
@@ -42,6 +42,7 @@ dependencies {
     implementation(libs.okhttp.logging.interceptor)
     api(libs.tagcommander.core)
 
+    testImplementation(project(":pillarbox-player-testutils"))
     testImplementation(libs.androidx.media3.test.utils)
     testImplementation(libs.androidx.media3.test.utils.robolectric)
     testImplementation(libs.androidx.test.core)

diff --git a/...gssr/pillarbox/core/business/tracker/commandersact/CommandersActTrackerIntegrationTest.kt b/...gssr/pillarbox/core/business/tracker/commandersact/CommandersActTrackerIntegrationTest.kt
@@ -31,6 +31,7 @@ import ch.srgssr.pillarbox.core.business.integrationlayer.service.HttpMediaCompo
 import ch.srgssr.pillarbox.core.business.integrationlayer.service.MediaCompositionService
 import ch.srgssr.pillarbox.core.business.tracker.DefaultMediaItemTrackerRepository
 import ch.srgssr.pillarbox.core.business.tracker.comscore.ComScoreTracker
+import ch.srgssr.pillarbox.player.test.utils.TestPillarboxRunHelper
 import ch.srgssr.pillarbox.player.tracker.MediaItemTrackerRepository
 import io.mockk.Called
 import io.mockk.confirmVerified
@@ -141,17 +142,23 @@ class CommandersActTrackerIntegrationTest {
 
         assertEquals(3, tcMediaEvents.size)
 
-        assertEquals(Play, tcMediaEvents[0].eventType)
-        assertTrue(tcMediaEvents[0].assets.isNotEmpty())
-        assertNull(tcMediaEvents[0].sourceId)
+        tcMediaEvents[0].let {
+            assertEquals(Play, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
 
-        assertEquals(Stop, tcMediaEvents[1].eventType)
-        assertTrue(tcMediaEvents[1].assets.isNotEmpty())
-        assertNull(tcMediaEvents[1].sourceId)
+        tcMediaEvents[1].let {
+            assertEquals(Stop, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
 
-        assertEquals(Play, tcMediaEvents[2].eventType)
-        assertTrue(tcMediaEvents[2].assets.isNotEmpty())
-        assertNull(tcMediaEvents[2].sourceId)
+        tcMediaEvents[2].let {
+            assertEquals(Play, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
     }
 
     @Test
@@ -309,13 +316,16 @@ class CommandersActTrackerIntegrationTest {
 
         assertEquals(2, tcMediaEvents.size)
 
-        assertEquals(Pause, tcMediaEvents[0].eventType)
-        assertTrue(tcMediaEvents[0].assets.isNotEmpty())
-        assertNull(tcMediaEvents[0].sourceId)
-
-        assertEquals(Play, tcMediaEvents[1].eventType)
-        assertTrue(tcMediaEvents[1].assets.isNotEmpty())
-        assertNull(tcMediaEvents[1].sourceId)
+        tcMediaEvents[0].let {
+            assertEquals(Pause, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
+        tcMediaEvents[1].let {
+            assertEquals(Play, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
     }
 
     @Test
@@ -351,33 +361,39 @@ class CommandersActTrackerIntegrationTest {
 
         assertEquals(3, tcMediaEvents.size)
 
-        assertEquals(Play, tcMediaEvents[0].eventType)
-        assertTrue(tcMediaEvents[0].assets.isNotEmpty())
-        assertNull(tcMediaEvents[0].sourceId)
-
-        assertEquals(Pause, tcMediaEvents[1].eventType)
-        assertTrue(tcMediaEvents[1].assets.isNotEmpty())
-        assertNull(tcMediaEvents[1].sourceId)
-
-        assertEquals(Play, tcMediaEvents[2].eventType)
-        assertTrue(tcMediaEvents[2].assets.isNotEmpty())
-        assertNull(tcMediaEvents[2].sourceId)
+        tcMediaEvents[0].let {
+            assertEquals(Play, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
+        tcMediaEvents[1].let {
+            assertEquals(Pause, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
+        tcMediaEvents[2].let {
+            assertEquals(Play, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
     }
 
     @Test
     fun `player prepared, playing and stopped`() {
         val tcMediaEvents = mutableListOf<TCMediaEvent>()
 
-        player.setMediaItem(SRGMediaItemBuilder(URN_LIVE_VIDEO).build())
+        player.setMediaItem(SRGMediaItemBuilder(URN_NOT_LIVE_VIDEO).build())
         player.prepare()
         player.playWhenReady = true
 
         TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_READY)
         TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player)
 
-        clock.advanceTime(2.minutes.inWholeMilliseconds)
+        val position = 2.minutes
+        TestPillarboxRunHelper.runUntilPosition(player, position = position, clock = clock)
         player.stop()
 
+        TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player)
         TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_IDLE)
 
         verifyOrder {
@@ -389,13 +405,59 @@ class CommandersActTrackerIntegrationTest {
 
         assertEquals(2, tcMediaEvents.size)
 
-        assertEquals(Stop, tcMediaEvents[0].eventType)
-        assertTrue(tcMediaEvents[0].assets.isNotEmpty())
-        assertNull(tcMediaEvents[0].sourceId)
+        tcMediaEvents[0].let {
+            assertEquals(Stop, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+            assertEquals(position, it.mediaPosition)
+        }
 
-        assertEquals(Play, tcMediaEvents[1].eventType)
-        assertTrue(tcMediaEvents[1].assets.isNotEmpty())
-        assertNull(tcMediaEvents[1].sourceId)
+        tcMediaEvents[1].let {
+            assertEquals(Play, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
+    }
+
+    @Test
+    fun `player prepared, playing and remove last item`() {
+        val tcMediaEvents = mutableListOf<TCMediaEvent>()
+
+        player.setMediaItem(SRGMediaItemBuilder(URN_NOT_LIVE_VIDEO).build())
+        player.prepare()
+        player.playWhenReady = true
+
+        TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_READY)
+        TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player)
+
+        val position = 2.minutes
+        TestPillarboxRunHelper.runUntilPosition(player, position = position, clock = clock)
+        player.removeMediaItem(0)
+
+        TestPlayerRunHelper.runUntilPendingCommandsAreFullyHandled(player)
+        TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED)
+
+        verifyOrder {
+            commandersAct.enableRunningInBackground()
+            commandersAct.sendTcMediaEvent(capture(tcMediaEvents))
+            commandersAct.sendTcMediaEvent(capture(tcMediaEvents))
+        }
+        confirmVerified(commandersAct)
+
+        assertEquals(2, tcMediaEvents.size)
+
+        tcMediaEvents[0].let {
+            assertEquals(Stop, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+            assertEquals(position, it.mediaPosition)
+        }
+
+        tcMediaEvents[1].let {
+            assertEquals(Play, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
     }
 
     @Test
@@ -424,17 +486,21 @@ class CommandersActTrackerIntegrationTest {
 
         assertEquals(3, tcMediaEvents.size)
 
-        assertEquals(Play, tcMediaEvents[0].eventType)
-        assertTrue(tcMediaEvents[0].assets.isNotEmpty())
-        assertNull(tcMediaEvents[0].sourceId)
-
-        assertEquals(Seek, tcMediaEvents[1].eventType)
-        assertTrue(tcMediaEvents[1].assets.isNotEmpty())
-        assertNull(tcMediaEvents[1].sourceId)
-
-        assertEquals(Play, tcMediaEvents[2].eventType)
-        assertTrue(tcMediaEvents[2].assets.isNotEmpty())
-        assertNull(tcMediaEvents[2].sourceId)
+        tcMediaEvents[0].let {
+            assertEquals(Play, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
+        tcMediaEvents[1].let {
+            assertEquals(Seek, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
+        tcMediaEvents[2].let {
+            assertEquals(Play, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
     }
 
     @Test
@@ -508,17 +574,23 @@ class CommandersActTrackerIntegrationTest {
 
         assertEquals(3, tcMediaEvents.size)
 
-        assertEquals(Pause, tcMediaEvents[0].eventType)
-        assertTrue(tcMediaEvents[0].assets.isNotEmpty())
-        assertNull(tcMediaEvents[0].sourceId)
+        tcMediaEvents[0].let {
+            assertEquals(Pause, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
 
-        assertEquals(Pos, tcMediaEvents[1].eventType)
-        assertTrue(tcMediaEvents[1].assets.isNotEmpty())
-        assertNull(tcMediaEvents[1].sourceId)
+        tcMediaEvents[1].let {
+            assertEquals(Pos, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
 
-        assertEquals(Play, tcMediaEvents[2].eventType)
-        assertTrue(tcMediaEvents[2].assets.isNotEmpty())
-        assertNull(tcMediaEvents[2].sourceId)
+        tcMediaEvents[2].let {
+            assertEquals(Play, it.eventType)
+            assertTrue(it.assets.isNotEmpty())
+            assertNull(it.sourceId)
+        }
     }
 
     @Test

diff --git a/pillarbox-player-testutils/build.gradle.kts b/pillarbox-player-testutils/build.gradle.kts
@@ -11,6 +11,7 @@ plugins {
 dependencies {
     api(libs.androidx.media3.common)
     compileOnly(libs.androidx.media3.exoplayer)
+    implementation(libs.androidx.media3.test.utils)
     implementation(libs.androidx.media3.test.utils.robolectric)
     implementation(libs.guava)
     runtimeOnly(libs.kotlinx.coroutines.android)

diff --git a/...r-testutils/src/main/java/ch/srgssr/pillarbox/player/test/utils/TestPillarboxRunHelper.kt b/...r-testutils/src/main/java/ch/srgssr/pillarbox/player/test/utils/TestPillarboxRunHelper.kt
@@ -10,9 +10,11 @@ import androidx.media3.common.Player
 import androidx.media3.common.util.Assertions
 import androidx.media3.common.util.Clock
 import androidx.media3.exoplayer.ExoPlayer
+import androidx.media3.test.utils.FakeClock
 import androidx.media3.test.utils.robolectric.RobolectricUtil
 import java.util.concurrent.TimeoutException
 import java.util.concurrent.atomic.AtomicBoolean
+import kotlin.time.Duration
 
 object TestPillarboxRunHelper {
     private fun verifyMainTestThread(player: Player) {
@@ -82,4 +84,23 @@ object TestPillarboxRunHelper {
             throw IllegalStateException(player.playerError)
         }
     }
+
+    /**
+     * Run and wait until [position] is reached
+     *
+     * @param player The [Player].
+     * @param position The position to wait for.
+     * @param clock The [FakeClock].
+     */
+    @Throws(TimeoutException::class)
+    fun runUntilPosition(player: Player, position: Duration, clock: FakeClock) {
+        verifyMainTestThread(player)
+        if (player is ExoPlayer) {
+            verifyPlaybackThreadIsAlive(player)
+        }
+        clock.advanceTime(position.inWholeMilliseconds)
+        RobolectricUtil.runMainLooperUntil {
+            player.currentPosition >= position.inWholeMilliseconds
+        }
+    }
 }
diff --git a/.../src/main/java/ch/srgssr/pillarbox/player/tracker/CurrentMediaItemPillarboxDataTracker.kt b/.../src/main/java/ch/srgssr/pillarbox/player/tracker/CurrentMediaItemPillarboxDataTracker.kt
@@ -79,6 +79,7 @@ internal class CurrentMediaItemPillarboxDataTracker(private val player: ExoPlaye
     }
 
     private inner class CurrentMediaItemListener : Player.Listener {
+
         override fun onMediaItemTransition(
             mediaItem: MediaItem?,
             @Player.MediaItemTransitionReason reason: Int,
@@ -92,7 +93,10 @@ internal class CurrentMediaItemPillarboxDataTracker(private val player: ExoPlaye
             timeline: Timeline,
             @Player.TimelineChangeReason reason: Int,
         ) {
-            notifyPillarboxDataChange(player.currentMediaItem)
+            // PillarboxData are loaded when event TIMELINE_CHANGE_REASON_SOURCE_UPDATE is send.
+            if (reason == Player.TIMELINE_CHANGE_REASON_SOURCE_UPDATE) {
+                notifyPillarboxDataChange(player.currentMediaItem)
+            }
         }
     }
 }