From 6d7053ccd2c2e8d62eef5a8c26d63f4cc40ace2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABtan=20Muller?= Date: Wed, 18 Sep 2024 13:36:11 +0200 Subject: [PATCH] Fix `MonitoringTest` by using `PillarboxExoPlayer` directly (#714) --- .../commandersact/CommandersActStreaming.kt | 2 +- .../CommandersActStreamingTest.kt | 4 + .../ComScoreTrackerIntegrationTest.kt | 85 ++++++++----------- .../pillarbox/player/PillarboxExoPlayer.kt | 18 ++-- .../player/monitoring/MonitoringTest.kt | 60 +++++-------- 5 files changed, 74 insertions(+), 95 deletions(-) diff --git a/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreaming.kt b/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreaming.kt index 42cf627b0..40b62ebb7 100644 --- a/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreaming.kt +++ b/pillarbox-core-business/src/main/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreaming.kt @@ -30,7 +30,7 @@ internal class CommandersActStreaming( private val commandersAct: CommandersAct, private val player: ExoPlayer, var currentData: CommandersActTracker.Data, - private val coroutineContext: CoroutineContext, + coroutineContext: CoroutineContext, ) : AnalyticsListener { private enum class State { diff --git a/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreamingTest.kt b/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreamingTest.kt index 5542a8bfe..26c36812c 100644 --- a/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreamingTest.kt +++ b/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/tracker/commandersact/CommandersActStreamingTest.kt @@ -4,6 +4,7 @@ */ package ch.srgssr.pillarbox.core.business.tracker.commandersact +import android.content.Context import androidx.annotation.FloatRange import androidx.annotation.IntRange import androidx.media3.common.C @@ -13,6 +14,7 @@ import androidx.media3.common.MimeTypes import androidx.media3.common.TrackGroup import androidx.media3.common.Tracks import androidx.media3.exoplayer.ExoPlayer +import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import ch.srgssr.pillarbox.analytics.commandersact.CommandersAct import ch.srgssr.pillarbox.analytics.commandersact.MediaEventType @@ -262,6 +264,7 @@ class CommandersActStreamingTest { ): ExoPlayer { return mockk { val player = this + val looper = ApplicationProvider.getApplicationContext().mainLooper every { player.playWhenReady } returns true every { player.isPlaying } returns isPlaying @@ -272,6 +275,7 @@ class CommandersActStreamingTest { every { player.deviceVolume } returns deviceVolume every { player.duration } returns duration every { player.currentTracks } returns currentTracks + every { player.applicationLooper } returns looper } } diff --git a/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/tracker/comscore/ComScoreTrackerIntegrationTest.kt b/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/tracker/comscore/ComScoreTrackerIntegrationTest.kt index 08ce0faed..245e6cd72 100644 --- a/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/tracker/comscore/ComScoreTrackerIntegrationTest.kt +++ b/pillarbox-core-business/src/test/java/ch/srgssr/pillarbox/core/business/tracker/comscore/ComScoreTrackerIntegrationTest.kt @@ -5,6 +5,7 @@ package ch.srgssr.pillarbox.core.business.tracker.comscore import android.content.Context +import android.os.Looper import android.view.SurfaceView import android.view.ViewGroup import androidx.core.view.updateLayoutParams @@ -25,12 +26,15 @@ import com.comscore.streaming.AssetMetadata import com.comscore.streaming.StreamingAnalytics import io.mockk.Called import io.mockk.MockKVerificationScope +import io.mockk.clearAllMocks import io.mockk.confirmVerified import io.mockk.mockk import io.mockk.verify import io.mockk.verifyOrder import org.junit.runner.RunWith +import org.robolectric.Shadows.shadowOf import kotlin.coroutines.EmptyCoroutineContext +import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Ignore import kotlin.test.Test @@ -69,6 +73,13 @@ class ComScoreTrackerIntegrationTest { ) } + @AfterTest + fun tearDown() { + clearAllMocks() + player.release() + shadowOf(Looper.getMainLooper()).idle() + } + @Test fun `player unprepared`() { TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_IDLE) @@ -95,16 +106,14 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifyPlayEvent() verifyEndEvent() verifyPlayerInformation() verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifySeekEvent(0L) verifyPlayEvent() } @@ -181,8 +190,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifyPlayEvent() verifyPauseEvent() } @@ -202,8 +210,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() } confirmVerified(streamingAnalytics) } @@ -222,8 +229,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifyPlayEvent() } confirmVerified(streamingAnalytics) @@ -248,8 +254,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifyPlayEvent() verifyPauseEvent() } @@ -280,8 +285,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifyPlayEvent() verifyPauseEvent() verifyPlayEvent() @@ -308,8 +312,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifyPlayEvent() verifyEndEvent() } @@ -334,12 +337,10 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifyPlayEvent() verifySeekStart() - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifyPlayEvent() } confirmVerified(streamingAnalytics) @@ -353,13 +354,13 @@ class ComScoreTrackerIntegrationTest { TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_READY) + verifyLiveInformation(atLeast = 0) verifyOrder { verifyPlayerInformation() verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() } confirmVerified(streamingAnalytics) } @@ -389,8 +390,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() } confirmVerified(streamingAnalytics) } @@ -408,8 +408,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifySeekEvent(0L) verifyPlayEvent() } @@ -430,8 +429,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 2f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifySeekEvent(0L) verifyPlayEvent() } @@ -454,8 +452,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifySeekEvent(0L) verifyPlayEvent() verifyPlaybackRate(playbackRate = 2f) @@ -481,8 +478,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifySeekEvent(0L) verifyPlayEvent() verifyPauseEvent() @@ -513,8 +509,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifySeekEvent(0L) verifyPlayEvent() verifyPauseEvent() @@ -542,8 +537,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifySeekEvent(0L) verifyPlayEvent() verifyEndEvent() @@ -568,14 +562,12 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifySeekEvent(0L) verifyPlayEvent() verifySeekStart() verifySeekEvent(30_000L) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() verifySeekEvent(30_000L) verifyPlayEvent() } @@ -595,8 +587,7 @@ class ComScoreTrackerIntegrationTest { verifyCreatePlaybackSession() verifyMetadata() verifyPlaybackRate(playbackRate = 1f) - verifyBufferStartEvent() - verifyBufferStopEvent() + verifyBufferEvents() } confirmVerified(streamingAnalytics) } @@ -653,12 +644,8 @@ class ComScoreTrackerIntegrationTest { } @Suppress("UnusedReceiverParameter") - private fun MockKVerificationScope.verifyBufferStartEvent() { + private fun MockKVerificationScope.verifyBufferEvents() { streamingAnalytics.notifyBufferStart() - } - - @Suppress("UnusedReceiverParameter") - private fun MockKVerificationScope.verifyBufferStopEvent() { streamingAnalytics.notifyBufferStop() } @@ -672,8 +659,8 @@ class ComScoreTrackerIntegrationTest { streamingAnalytics.notifySeekStart() } - private fun verifyLiveInformation() { - verify { + private fun verifyLiveInformation(atLeast: Int = 1) { + verify(atLeast = atLeast) { streamingAnalytics.setDvrWindowLength(any()) streamingAnalytics.startFromDvrWindowOffset(any()) } diff --git a/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/PillarboxExoPlayer.kt b/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/PillarboxExoPlayer.kt index 241cd602d..f9664e886 100644 --- a/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/PillarboxExoPlayer.kt +++ b/pillarbox-player/src/main/java/ch/srgssr/pillarbox/player/PillarboxExoPlayer.kt @@ -81,6 +81,16 @@ class PillarboxExoPlayer internal constructor( internal val sessionManager = PlaybackSessionManager() private val window = Window() + @VisibleForTesting + internal val monitoring = Monitoring( + context = context, + player = this, + metricsCollector = metricsCollector, + messageHandler = monitoringMessageHandler, + sessionManager = sessionManager, + coroutineContext = coroutineContext, + ) + override var smoothSeekingEnabled: Boolean = false set(value) { if (value != field) { @@ -138,14 +148,6 @@ class PillarboxExoPlayer internal constructor( init { sessionManager.setPlayer(this) metricsCollector.setPlayer(this) - Monitoring( - context = context, - player = this, - metricsCollector = metricsCollector, - messageHandler = monitoringMessageHandler, - sessionManager = sessionManager, - coroutineContext = coroutineContext, - ) addListener(analyticsCollector) exoPlayer.addListener(ComponentListener()) itemPillarboxDataTracker.addCallback(timeRangeTracker) diff --git a/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/monitoring/MonitoringTest.kt b/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/monitoring/MonitoringTest.kt index c59ee8a96..30541151e 100644 --- a/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/monitoring/MonitoringTest.kt +++ b/pillarbox-player/src/test/java/ch/srgssr/pillarbox/player/monitoring/MonitoringTest.kt @@ -8,24 +8,22 @@ import android.content.Context import android.os.Looper import androidx.media3.common.MediaItem import androidx.media3.common.Player -import androidx.media3.exoplayer.DefaultLoadControl import androidx.media3.test.utils.FakeClock import androidx.media3.test.utils.robolectric.TestPlayerRunHelper import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 import ch.srgssr.pillarbox.player.PillarboxExoPlayer -import ch.srgssr.pillarbox.player.SeekIncrement -import ch.srgssr.pillarbox.player.analytics.metrics.MetricsCollector import ch.srgssr.pillarbox.player.monitoring.models.Message import ch.srgssr.pillarbox.player.monitoring.models.Message.EventName -import ch.srgssr.pillarbox.player.source.PillarboxMediaSourceFactory import io.mockk.clearAllMocks import io.mockk.confirmVerified import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestDispatcher +import kotlinx.coroutines.test.UnconfinedTestDispatcher import org.junit.runner.RunWith import org.robolectric.Shadows.shadowOf -import kotlin.coroutines.EmptyCoroutineContext import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -37,33 +35,20 @@ import kotlin.time.Duration.Companion.seconds @RunWith(AndroidJUnit4::class) class MonitoringTest { private lateinit var player: PillarboxExoPlayer - private lateinit var fakeClock: FakeClock - private lateinit var monitoring: Monitoring private lateinit var monitoringMessageHandler: MonitoringMessageHandler + private lateinit var testDispatcher: TestDispatcher @BeforeTest + @OptIn(ExperimentalCoroutinesApi::class) fun setUp() { val context = ApplicationProvider.getApplicationContext() - fakeClock = FakeClock(true) - player = PillarboxExoPlayer( - context = context, - seekIncrement = SeekIncrement(), - loadControl = DefaultLoadControl(), - clock = fakeClock, - coroutineContext = EmptyCoroutineContext, - mediaSourceFactory = PillarboxMediaSourceFactory(context), - ) - // Should be an input of ExoPlayer at least for test - val metricsCollector = MetricsCollector() - metricsCollector.setPlayer(player) monitoringMessageHandler = mockk(relaxed = true) - monitoring = Monitoring( + testDispatcher = UnconfinedTestDispatcher() + player = PillarboxExoPlayer( context = context, - player = player, - metricsCollector = metricsCollector, - sessionManager = player.sessionManager, - messageHandler = monitoringMessageHandler, - coroutineContext = EmptyCoroutineContext, + clock = FakeClock(true), + coroutineContext = testDispatcher, + monitoringMessageHandler = monitoringMessageHandler, ) player.prepare() player.play() @@ -82,8 +67,8 @@ class MonitoringTest { TestPlayerRunHelper.playUntilPosition(player, 0, 5.seconds.inWholeMilliseconds) - val qoeTimings = monitoring.getCurrentQoETimings() - val qosTimings = monitoring.getCurrentQoSTimings() + val qoeTimings = player.monitoring.getCurrentQoETimings() + val qosTimings = player.monitoring.getCurrentQoSTimings() TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED) // To ensure that the final `onSessionFinished` is triggered. @@ -92,7 +77,7 @@ class MonitoringTest { val messages = mutableListOf() - verify { + verify(exactly = 3) { monitoringMessageHandler.sendEvent(capture(messages)) } confirmVerified(monitoringMessageHandler) @@ -115,14 +100,14 @@ class MonitoringTest { TestPlayerRunHelper.playUntilPosition(player, 0, 5.seconds.inWholeMilliseconds) - val qoeTimings1 = monitoring.getCurrentQoETimings() - val qosTimings1 = monitoring.getCurrentQoSTimings() + val qoeTimings1 = player.monitoring.getCurrentQoETimings() + val qosTimings1 = player.monitoring.getCurrentQoSTimings() TestPlayerRunHelper.runUntilTimelineChanged(player) TestPlayerRunHelper.playUntilPosition(player, 1, 5.seconds.inWholeMilliseconds) - val qoeTimings2 = monitoring.getCurrentQoETimings() - val qosTimings2 = monitoring.getCurrentQoSTimings() + val qoeTimings2 = player.monitoring.getCurrentQoETimings() + val qosTimings2 = player.monitoring.getCurrentQoSTimings() TestPlayerRunHelper.runUntilPlaybackState(player, Player.STATE_ENDED) // To ensure that the final `onSessionFinished` is triggered. @@ -131,19 +116,21 @@ class MonitoringTest { val messages = mutableListOf() - verify { + verify(exactly = 6) { monitoringMessageHandler.sendEvent(capture(messages)) } confirmVerified(monitoringMessageHandler) + val messagesBySessionId = messages.groupBy { it.sessionId } + assertEquals( listOf( listOf(EventName.START, EventName.HEARTBEAT, EventName.STOP), listOf(EventName.START, EventName.HEARTBEAT, EventName.STOP) ), - messages.groupBy { it.sessionId }.map { entry -> entry.value.map { it.eventName } } + messagesBySessionId.map { entry -> entry.value.map { it.eventName } } ) - assertEquals(2, messages.distinctBy { it.sessionId }.count()) + assertEquals(2, messagesBySessionId.size) assertNotSame(qosTimings1, qosTimings2) assertNotSame(qoeTimings1, qoeTimings2) @@ -165,12 +152,11 @@ class MonitoringTest { val messages = mutableListOf() - verify { + verify(exactly = 2) { monitoringMessageHandler.sendEvent(capture(messages)) } confirmVerified(monitoringMessageHandler) - assertEquals(2, messages.size) assertEquals(listOf(EventName.START, EventName.ERROR), messages.map { it.eventName }) assertEquals(1, messages.distinctBy { it.sessionId }.count()) }