Simplify session properties tests to not checked cached payload (#1412)

## Goal Getting the last saved session was flakey in the same way getting the last saved background activity was flakey. Fixed this by just not checking that, but instead checking when the session is done, which is not flakey because that save happens on the main test thread. At the same time, I made the non-flakey saved envelope payload validation function generic and usable for sessions, if we ever need it
embrace-io · Sep 23, 2024 · ccb96a4 · ccb96a4
1 parent 2dc5f93
commit ccb96a4
Show file tree

Hide file tree

Showing 3 changed files with 87 additions and 97 deletions.
diff --git a/...src/integrationTest/kotlin/io/embrace/android/embracesdk/IntegrationTestRuleExtensions.kt b/...src/integrationTest/kotlin/io/embrace/android/embracesdk/IntegrationTestRuleExtensions.kt
@@ -1,7 +1,6 @@
 package io.embrace.android.embracesdk
 
 import android.app.Activity
-import io.embrace.android.embracesdk.fakes.FakeDeliveryService
 import io.embrace.android.embracesdk.internal.payload.Envelope
 import io.embrace.android.embracesdk.internal.payload.EventMessage
 import io.embrace.android.embracesdk.internal.payload.Log
@@ -91,11 +90,20 @@ internal fun IntegrationTestRule.Harness.getSentBackgroundActivities(): List<Env
 }
 
 /**
- * Returns the last session that was saved by the SDK.
+ * Run some [action] and the validate the next saved session using [validationFn]. If no session is saved with
+ * 1 second, this fails.
  */
-internal fun IntegrationTestRule.Harness.getLastSavedSession(): Envelope<SessionPayload>? {
-    return overriddenDeliveryModule.deliveryService.getLastSavedSession()
-}
+internal fun IntegrationTestRule.Harness.checkNextSavedSession(
+    action: () -> Unit,
+    validationFn: (Envelope<SessionPayload>) -> Unit
+): Envelope<SessionPayload> =
+    with(overriddenDeliveryModule.deliveryService) {
+        checkNextSavedSessionEnvelope(
+            dataProvider = ::getSavedSessions,
+            action = action,
+            validationFn = validationFn,
+        )
+    }
 
 /**
  * Run some [action] and the validate the next saved background activity using [validationFn]. If no background activity is saved with
@@ -104,36 +112,14 @@ internal fun IntegrationTestRule.Harness.getLastSavedSession(): Envelope<Session
 internal fun IntegrationTestRule.Harness.checkNextSavedBackgroundActivity(
     action: () -> Unit,
     validationFn: (Envelope<SessionPayload>) -> Unit
-): Envelope<SessionPayload> {
+): Envelope<SessionPayload> =
     with(overriddenDeliveryModule.deliveryService) {
-        val startingSize = getSavedBackgroundActivities().size
-        overriddenClock.tick(10_000L)
-        action()
-        return when (getSavedBackgroundActivities().size) {
-            startingSize -> {
-                returnIfConditionMet(
-                    desiredValueSupplier = {
-                        getNthBackgroundActivity(startingSize)
-                    },
-                    condition = { data ->
-                        data.size > startingSize
-                    },
-                    dataProvider = ::getSavedBackgroundActivities
-                )
-            }
-            else -> {
-                getNthBackgroundActivity(startingSize)
-            }
-        }.apply {
-            validationFn(this)
-        }
+        checkNextSavedSessionEnvelope(
+            dataProvider = ::getSavedBackgroundActivities,
+            action = action,
+            validationFn = validationFn,
+        )
     }
-}
-
-private fun FakeDeliveryService.getNthBackgroundActivity(n: Int) =
-    getSavedBackgroundActivities().filterIndexed { index, _ ->
-        index == n
-    }.single()
 
 /**
  * Returns the last session that was sent by the SDK.
@@ -235,4 +221,35 @@ internal fun <T, R> returnIfConditionMet(
     throw TimeoutException("Timeout period elapsed before condition met")
 }
 
+private fun IntegrationTestRule.Harness.checkNextSavedSessionEnvelope(
+    dataProvider: () -> List<Envelope<SessionPayload>>,
+    action: () -> Unit,
+    validationFn: (Envelope<SessionPayload>) -> Unit
+): Envelope<SessionPayload> {
+    val startingSize = dataProvider().size
+    overriddenClock.tick(10_000L)
+    action()
+    return when (dataProvider().size) {
+        startingSize -> {
+            returnIfConditionMet(
+                desiredValueSupplier = {
+                    dataProvider().getNth(startingSize)
+                },
+                condition = { data ->
+                    data.size > startingSize
+                },
+                dataProvider = dataProvider
+            )
+        }
+
+        else -> {
+            dataProvider().getNth(startingSize)
+        }
+    }.apply {
+        validationFn(this)
+    }
+}
+
+private fun <T> List<T>.getNth(n: Int) = filterIndexed { index, _ -> index == n }.single()
+
 private const val CHECK_INTERVAL_MS: Int = 10
diff --git a/...rc/integrationTest/kotlin/io/embrace/android/embracesdk/features/SessionPropertiesTest.kt b/...rc/integrationTest/kotlin/io/embrace/android/embracesdk/features/SessionPropertiesTest.kt
@@ -4,7 +4,6 @@ import androidx.test.ext.junit.runners.AndroidJUnit4
 import io.embrace.android.embracesdk.IntegrationTestRule
 import io.embrace.android.embracesdk.checkNextSavedBackgroundActivity
 import io.embrace.android.embracesdk.findSessionSpan
-import io.embrace.android.embracesdk.getLastSavedSession
 import io.embrace.android.embracesdk.getLastSentBackgroundActivity
 import io.embrace.android.embracesdk.getSentBackgroundActivities
 import io.embrace.android.embracesdk.getSessionId
@@ -168,19 +167,12 @@ internal class SessionPropertiesTest {
                 }
             )
 
-            harness.recordSession {
-                with(checkNotNull(harness.getLastSavedSession()?.getSessionSpan())) {
-                    assertNotEquals(lastSessionId, embrace.currentSessionId)
-                    assertPropertyExistence(
-                        exist = listOf("perm")
-                    )
-                    lastSessionId = checkNotNull(embrace.currentSessionId)
-                }
+            val session = checkNotNull(harness.recordSession {
                 embrace.addSessionProperty("perm2", "value", true)
-                checkNotNull(harness.getLastSavedSession()?.getSessionSpan()).assertPropertyExistence(
-                    exist = listOf("perm", "perm2")
-                )
-            }
+            })
+            checkNotNull(session.getSessionSpan()).assertPropertyExistence(
+                exist = listOf("perm", "perm2")
+            )
 
             harness.checkNextSavedBackgroundActivity(
                 action = {
@@ -197,15 +189,10 @@ internal class SessionPropertiesTest {
                 }
             )
 
-            harness.recordSession {
-                with(checkNotNull(harness.getLastSavedSession()?.getSessionSpan())) {
-                    assertNotEquals(lastSessionId, embrace.currentSessionId)
-                    assertPropertyExistence(
-                        exist = listOf("perm", "perm2", "perm3")
-                    )
-                    lastSessionId = checkNotNull(embrace.currentSessionId)
-                }
-            }
+            val session2 = checkNotNull(harness.recordSession())
+            checkNotNull(session2.getSessionSpan()).assertPropertyExistence(
+                exist = listOf("perm", "perm2", "perm3")
+            )
         }
     }
 
@@ -215,33 +202,19 @@ internal class SessionPropertiesTest {
             harness.overriddenConfigService.backgroundActivityCaptureEnabled = false
             startSdk()
             embrace.addSessionProperty("perm", "value", true)
-            var lastSessionId = checkNotNull(harness.recordSession()).getSessionId()
-            harness.recordSession {
-                with(checkNotNull(harness.getLastSavedSession()?.getSessionSpan())) {
-                    assertNotEquals(lastSessionId, embrace.currentSessionId)
-                    assertPropertyExistence(
-                        exist = listOf("perm")
-                    )
-                    lastSessionId = checkNotNull(embrace.currentSessionId)
-                }
+            val session = checkNotNull(harness.recordSession {
                 embrace.addSessionProperty("perm2", "value", true)
-                checkNotNull(harness.getLastSavedSession()?.getSessionSpan()).assertPropertyExistence(
-                    exist = listOf("perm", "perm2")
-                )
-            }
-            harness.recordSession {
-                with(checkNotNull(harness.getLastSavedSession()?.getSessionSpan())) {
-                    assertNotEquals(lastSessionId, embrace.currentSessionId)
-                    assertPropertyExistence(
-                        exist = listOf("perm", "perm2")
-                    )
-                    lastSessionId = checkNotNull(embrace.currentSessionId)
-                }
+            })
+            checkNotNull(session.getSessionSpan()).assertPropertyExistence(
+                exist = listOf("perm", "perm2")
+            )
+
+            val session2 = checkNotNull(harness.recordSession {
                 embrace.addSessionProperty("perm3", "value", true)
-                checkNotNull(harness.getLastSavedSession()?.getSessionSpan()).assertPropertyExistence(
-                    exist = listOf("perm", "perm2", "perm3")
-                )
-            }
+            })
+            checkNotNull(session2.getSessionSpan()).assertPropertyExistence(
+                exist = listOf("perm", "perm2", "perm3")
+            )
         }
     }
 

diff --git a/...ace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeDeliveryService.kt b/...ace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeDeliveryService.kt
@@ -61,30 +61,30 @@ open class FakeDeliveryService : DeliveryService {
         lastSavedLogPayloads.add(logEnvelope)
     }
 
-    fun getSentSessions(): List<Envelope<SessionPayload>> {
-        return sentSessionEnvelopes.filter { it.first.findAppState() == ApplicationState.FOREGROUND }.map { it.first }
-    }
+    fun getSentSessions(): List<Envelope<SessionPayload>> =
+        sentSessionEnvelopes.filterSessionEnvelopes(appState = ApplicationState.FOREGROUND)
 
-    fun getSentBackgroundActivities(): List<Envelope<SessionPayload>> {
-        return sentSessionEnvelopes.filter { it.first.findAppState() == ApplicationState.BACKGROUND }.map { it.first }
-    }
+    fun getSentBackgroundActivities(): List<Envelope<SessionPayload>> =
+        sentSessionEnvelopes.filterSessionEnvelopes(appState = ApplicationState.BACKGROUND)
 
-    fun getSavedBackgroundActivities(): List<Envelope<SessionPayload>> {
-        return savedSessionEnvelopes.filter { it.first.findAppState() == ApplicationState.BACKGROUND }.map { it.first }
-    }
+    fun getSavedSessions(): List<Envelope<SessionPayload>> =
+        savedSessionEnvelopes.filterSessionEnvelopes(appState = ApplicationState.FOREGROUND)
 
-    private fun Envelope<SessionPayload>.findAppState(): ApplicationState {
-        val value = findSessionSpan().attributes?.findAttributeValue(embState.name)?.uppercase(Locale.ENGLISH)
-        return ApplicationState.valueOf(checkNotNull(value))
-    }
+    fun getSavedBackgroundActivities(): List<Envelope<SessionPayload>> =
+        savedSessionEnvelopes.filterSessionEnvelopes(appState = ApplicationState.BACKGROUND)
 
     fun getLastSentSession(): Envelope<SessionPayload>? {
         return getSentSessions().lastOrNull()
     }
 
-    fun getLastSavedSession(): Envelope<SessionPayload>? {
-        return savedSessionEnvelopes.map { it.first }.lastOrNull {
-            it.findAppState() == ApplicationState.FOREGROUND
-        }
+    private fun Queue<Pair<Envelope<SessionPayload>, SessionSnapshotType>>.filterSessionEnvelopes(
+        appState: ApplicationState
+    ): List<Envelope<SessionPayload>> {
+        return filter { it.first.findAppState() == appState }.map { it.first }
+    }
+
+    private fun Envelope<SessionPayload>.findAppState(): ApplicationState {
+        val value = findSessionSpan().attributes?.findAttributeValue(embState.name)?.uppercase(Locale.ENGLISH)
+        return ApplicationState.valueOf(checkNotNull(value))
     }
 }