Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: suppress some ad events when outside of an ad break #102

Merged
merged 12 commits into from
Nov 21, 2024
Merged

fix: suppress some ad events when outside of an ad break #102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
cb9496e
fix: suppress adplaying when outside of an ad break
andrewjl-mux Nov 20, 2024
5ee699f
filter out additional events
andrewjl-mux Nov 20, 2024
99e819b
fix import
andrewjl-mux Nov 20, 2024
eda3532
add test
andrewjl-mux Nov 20, 2024
2f3b5f9
more tests
andrewjl-mux Nov 20, 2024
be3a1e0
whoops
andrewjl-mux Nov 20, 2024
5a05ac5
test: empty collected dispatched events across ad platying state tran…
andrewjl-mux Nov 20, 2024
9c0298c
fix logic
andrewjl-mux Nov 20, 2024
6325f34
test: a few more tests and improve test failure messages
andrewjl-mux Nov 20, 2024
c525b07
maybe this
andrewjl-mux Nov 20, 2024
c1899b3
remove unnecessary import
andrewjl-mux Nov 20, 2024
760e7d6
remove nested if
andrewjl-mux Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion library/src/main/java/com/mux/stats/sdk/muxstats/MuxStatsSdkMedia3.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,18 @@ import androidx.annotation.OptIn
import androidx.media3.common.MediaLibraryInfo
import androidx.media3.common.Player
import androidx.media3.common.util.UnstableApi
import com.mux.android.util.noneOf
import com.mux.stats.sdk.core.CustomOptions
import com.mux.stats.sdk.core.events.EventBus
import com.mux.stats.sdk.core.events.playback.AdBreakEndEvent
import com.mux.stats.sdk.core.events.playback.AdEndedEvent
import com.mux.stats.sdk.core.events.playback.AdEvent
import com.mux.stats.sdk.core.events.playback.AdFirstQuartileEvent
import com.mux.stats.sdk.core.events.playback.AdMidpointEvent
import com.mux.stats.sdk.core.events.playback.AdPauseEvent
import com.mux.stats.sdk.core.events.playback.AdPlayEvent
import com.mux.stats.sdk.core.events.playback.AdPlayingEvent
import com.mux.stats.sdk.core.events.playback.AdThirdQuartileEvent
import com.mux.stats.sdk.core.model.CustomerData
import com.mux.stats.sdk.core.model.CustomerVideoData
import com.mux.stats.sdk.core.util.MuxLogger
Expand Down Expand Up @@ -145,7 +154,24 @@ class AdCollector private constructor(
}

fun dispatch(event: AdEvent) {
eventBus.dispatch(event)

if (
muxPlayerState != MuxPlayerState.PLAYING_ADS &&
event.type.noneOf(
AdPlayingEvent.TYPE,
AdPlayEvent.TYPE,
AdFirstQuartileEvent.TYPE,
AdMidpointEvent.TYPE,
AdThirdQuartileEvent.TYPE,
AdEndedEvent.TYPE,
AdBreakEndEvent.TYPE,
AdPauseEvent.TYPE
)
) {
eventBus.dispatch(event)
} else if (muxPlayerState == MuxPlayerState.PLAYING_ADS) {
eventBus.dispatch(event)
}
}

companion object {
Expand Down
182 changes: 182 additions & 0 deletions library/src/test/java/com/mux/stats/sdk/muxstats/AdCollectorTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ package com.mux.stats.sdk.muxstats
import com.mux.stats.media3.test.tools.AbsRobolectricTest
import com.mux.stats.sdk.core.events.EventBus
import com.mux.stats.sdk.core.events.IEvent
import com.mux.stats.sdk.core.events.playback.AdErrorEvent
import com.mux.stats.sdk.core.events.playback.AdPlayEvent
import com.mux.stats.sdk.core.events.playback.AdPlayingEvent
import com.mux.stats.sdk.core.events.playback.AdRequestEvent
import com.mux.stats.sdk.core.events.playback.AdResponseEvent
import com.mux.stats.sdk.core.events.playback.RebufferEndEvent
import io.mockk.every
import io.mockk.just
Expand Down Expand Up @@ -45,4 +50,181 @@ class AdCollectorTests : AbsRobolectricTest() {
)

}

@Test
fun testDispatchesAdPlayAndAdPlayingOnlyDuringAdBreaks() {
val dispatchedEvents = mutableListOf<IEvent>()
val eventBus = mockk<EventBus> {
every { addListener(any()) } just runs
every { removeListener(any()) } just runs
every { removeAllListeners() } just runs
every { dispatch(any()) } answers { call ->
dispatchedEvents += (call.invocation.args.first() as IEvent)
}
}
val stateCollector = MuxStateCollector(mockk<MuxStats>(relaxed = true), eventBus)
val adCollector = AdCollector.create(stateCollector, eventBus)

adCollector.dispatch(AdPlayEvent(null))
adCollector.dispatch(AdPlayingEvent(null))

Assert.assertTrue(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion(non-blocking): use a loop + all expected filtered out ad events for your assertions. Alternatively, just write individual tests for every ad event like you did for the "allowed" ones, below (which allows test to better isolate an issue)

"adplay should not be sent to event bus",
dispatchedEvents.find { it is AdPlayEvent } == null
)

Assert.assertTrue(
"adplaying should not be sent to event bus",
dispatchedEvents.find { it is AdPlayingEvent } == null
)

dispatchedEvents.clear()
adCollector.onStartPlayingAds()

adCollector.dispatch(AdPlayEvent(null))
adCollector.dispatch(AdPlayingEvent(null))

Assert.assertTrue(
"adplay should not be sent to event bus",
dispatchedEvents.find { it is AdPlayEvent } != null
)

Assert.assertTrue(
"adplaying should not be sent to event bus",
dispatchedEvents.find { it is AdPlayingEvent } != null
)

dispatchedEvents.clear()
adCollector.onFinishPlayingAds(willPlay = true)

adCollector.dispatch(AdPlayEvent(null))
adCollector.dispatch(AdPlayingEvent(null))

Assert.assertTrue(
"adplay should not be sent to event bus",
dispatchedEvents.find { it is AdPlayEvent } == null
)

Assert.assertTrue(
"adplaying should not be sent to event bus",
dispatchedEvents.find { it is AdPlayingEvent } == null
)
}

@Test
fun testDispatchesAdErrorBothDuringAndOutsideOfAdBreaks() {
val dispatchedEvents = mutableListOf<IEvent>()
val eventBus = mockk<EventBus> {
every { addListener(any()) } just runs
every { removeListener(any()) } just runs
every { removeAllListeners() } just runs
every { dispatch(any()) } answers { call ->
dispatchedEvents += (call.invocation.args.first() as IEvent)
}
}
val stateCollector = MuxStateCollector(mockk<MuxStats>(relaxed = true), eventBus)
val adCollector = AdCollector.create(stateCollector, eventBus)

adCollector.dispatch(AdErrorEvent(null))
Assert.assertTrue(
"aderror should be sent to event bus before the start of an ad break",
dispatchedEvents.find { it is AdErrorEvent } != null
)

dispatchedEvents.clear()
adCollector.onStartPlayingAds()

adCollector.dispatch(AdErrorEvent(null))
Assert.assertTrue(
"aderror should be sent to event bus during an ad break",
dispatchedEvents.find { it is AdErrorEvent } != null
)

dispatchedEvents.clear()
adCollector.onFinishPlayingAds(willPlay = true)

adCollector.dispatch(AdErrorEvent(null))
Assert.assertTrue(
"aderror should be sent to event bus after an ad break is finished",
dispatchedEvents.find { it is AdErrorEvent } != null
)
}

@Test
fun testDispatchesAdRequestBothDuringAndOutsideOfAdBreaks() {
val dispatchedEvents = mutableListOf<IEvent>()
val eventBus = mockk<EventBus> {
every { addListener(any()) } just runs
every { removeListener(any()) } just runs
every { removeAllListeners() } just runs
every { dispatch(any()) } answers { call ->
dispatchedEvents += (call.invocation.args.first() as IEvent)
}
}
val stateCollector = MuxStateCollector(mockk<MuxStats>(relaxed = true), eventBus)
val adCollector = AdCollector.create(stateCollector, eventBus)

adCollector.dispatch(AdRequestEvent(null))
Assert.assertTrue(
"adrequest should be sent to event bus before the start of an ad break",
dispatchedEvents.find { it is AdRequestEvent } != null
)

dispatchedEvents.clear()
adCollector.onStartPlayingAds()

adCollector.dispatch(AdRequestEvent(null))
Assert.assertTrue(
"adrequest should be sent to event bus during an ad break",
dispatchedEvents.find { it is AdRequestEvent } != null
)

dispatchedEvents.clear()
adCollector.onFinishPlayingAds(willPlay = true)

adCollector.dispatch(AdRequestEvent(null))
Assert.assertTrue(
"adrequest should be sent to event bus after an ad break is finished",
dispatchedEvents.find { it is AdRequestEvent } != null
)
}

@Test
fun testDispatchesAdResponseBothDuringAndOutsideOfAdBreaks() {
val dispatchedEvents = mutableListOf<IEvent>()
val eventBus = mockk<EventBus> {
every { addListener(any()) } just runs
every { removeListener(any()) } just runs
every { removeAllListeners() } just runs
every { dispatch(any()) } answers { call ->
dispatchedEvents += (call.invocation.args.first() as IEvent)
}
}
val stateCollector = MuxStateCollector(mockk<MuxStats>(relaxed = true), eventBus)
val adCollector = AdCollector.create(stateCollector, eventBus)

adCollector.dispatch(AdResponseEvent(null))
Assert.assertTrue(
"adresponse should be sent to event bus before the start of an ad break",
dispatchedEvents.find { it is AdResponseEvent } != null
)

dispatchedEvents.clear()
adCollector.onStartPlayingAds()

adCollector.dispatch(AdResponseEvent(null))
Assert.assertTrue(
"adresponse should be sent to event bus during an ad break",
dispatchedEvents.find { it is AdResponseEvent } != null
)

dispatchedEvents.clear()
adCollector.onFinishPlayingAds(willPlay = true)

adCollector.dispatch(AdResponseEvent(null))
Assert.assertTrue(
"adresponse should be sent to event bus after an ad break is finished",
dispatchedEvents.find { it is AdResponseEvent } != null
)
}
}