Fix session properties integration tests (#1284)

## Goal The test was comparing sessionId to spanId, which is wrong. Looking through the delivery service, it probably works better to have the saved sessions stored in a threadsafe manner, so I made that change too, even though the integration test will do the saving on the main thread.
embrace-io · Aug 21, 2024 · 10a3127 · 10a3127
1 parent ba0eb43
commit 10a3127
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 17 deletions.
diff --git a/...src/integrationTest/kotlin/io/embrace/android/embracesdk/IntegrationTestRuleExtensions.kt b/...src/integrationTest/kotlin/io/embrace/android/embracesdk/IntegrationTestRuleExtensions.kt
@@ -76,7 +76,7 @@ internal fun IntegrationTestRule.Harness.getSentSessions(): List<Envelope<Sessio
 }
 
 /**
- * Returns a list of [BackgroundActivityMessage] that were sent by the SDK since startup.
+ * Returns a list of background activity payloads that were sent by the SDK since startup.
  */
 internal fun IntegrationTestRule.Harness.getSentBackgroundActivities(): List<Envelope<SessionPayload>> {
     return overriddenDeliveryModule.deliveryService.getSentBackgroundActivities()

diff --git a/...rc/integrationTest/kotlin/io/embrace/android/embracesdk/features/SessionPropertiesTest.kt b/...rc/integrationTest/kotlin/io/embrace/android/embracesdk/features/SessionPropertiesTest.kt
@@ -146,23 +146,23 @@ internal class SessionPropertiesTest {
     fun `permanent properties are persisted in cached payloads`() {
         with(testRule) {
             startSdk()
-            var lastSessionSpanId = checkNotNull(harness.recordSession()).getSessionId()
+            var lastSessionId = checkNotNull(harness.recordSession()).getSessionId()
             embrace.addSessionProperty("perm", "value", true)
             with(checkNotNull(harness.getLastSavedBackgroundActivity()?.getSessionSpan())) {
-                assertNotEquals(lastSessionSpanId, spanId)
+                assertNotEquals(lastSessionId, embrace.currentSessionId)
                 assertPropertyExistence(
                     exist = listOf("perm")
                 )
-                lastSessionSpanId = checkNotNull(spanId)
+                lastSessionId = checkNotNull(embrace.currentSessionId)
             }
 
             harness.recordSession {
                 with(checkNotNull(harness.getLastSavedSession()?.getSessionSpan())) {
-                    assertNotEquals(lastSessionSpanId, spanId)
+                    assertNotEquals(lastSessionId, embrace.currentSessionId)
                     assertPropertyExistence(
                         exist = listOf("perm")
                     )
-                    lastSessionSpanId = checkNotNull(spanId)
+                    lastSessionId = checkNotNull(embrace.currentSessionId)
                 }
                 embrace.addSessionProperty("perm2", "value", true)
                 checkNotNull(harness.getLastSavedSession()?.getSessionSpan()).assertPropertyExistence(
@@ -171,11 +171,11 @@ internal class SessionPropertiesTest {
             }
 
             with(checkNotNull(harness.getLastSavedBackgroundActivity()?.getSessionSpan())) {
-                assertNotEquals(lastSessionSpanId, spanId)
+                assertNotEquals(lastSessionId, embrace.currentSessionId)
                 assertPropertyExistence(
                     exist = listOf("perm", "perm2")
                 )
-                lastSessionSpanId = checkNotNull(spanId)
+                lastSessionId = checkNotNull(embrace.currentSessionId)
             }
 
             embrace.addSessionProperty("perm3", "value", true)
@@ -185,11 +185,11 @@ internal class SessionPropertiesTest {
 
             harness.recordSession {
                 with(checkNotNull(harness.getLastSavedSession()?.getSessionSpan())) {
-                    assertNotEquals(lastSessionSpanId, spanId)
+                    assertNotEquals(lastSessionId, embrace.currentSessionId)
                     assertPropertyExistence(
                         exist = listOf("perm", "perm2", "perm3")
                     )
-                    lastSessionSpanId = checkNotNull(spanId)
+                    lastSessionId = checkNotNull(embrace.currentSessionId)
                 }
             }
         }
@@ -201,14 +201,14 @@ internal class SessionPropertiesTest {
             harness.overriddenConfigService.backgroundActivityCaptureEnabled = false
             startSdk()
             embrace.addSessionProperty("perm", "value", true)
-            var lastSessionSpanId = checkNotNull(harness.recordSession()).getSessionId()
+            var lastSessionId = checkNotNull(harness.recordSession()).getSessionId()
             harness.recordSession {
                 with(checkNotNull(harness.getLastSavedSession()?.getSessionSpan())) {
-                    assertNotEquals(lastSessionSpanId, spanId)
+                    assertNotEquals(lastSessionId, embrace.currentSessionId)
                     assertPropertyExistence(
                         exist = listOf("perm")
                     )
-                    lastSessionSpanId = checkNotNull(spanId)
+                    lastSessionId = checkNotNull(embrace.currentSessionId)
                 }
                 embrace.addSessionProperty("perm2", "value", true)
                 checkNotNull(harness.getLastSavedSession()?.getSessionSpan()).assertPropertyExistence(
@@ -217,11 +217,11 @@ internal class SessionPropertiesTest {
             }
             harness.recordSession {
                 with(checkNotNull(harness.getLastSavedSession()?.getSessionSpan())) {
-                    assertNotEquals(lastSessionSpanId, spanId)
+                    assertNotEquals(lastSessionId, embrace.currentSessionId)
                     assertPropertyExistence(
                         exist = listOf("perm", "perm2")
                     )
-                    lastSessionSpanId = checkNotNull(spanId)
+                    lastSessionId = checkNotNull(embrace.currentSessionId)
                 }
                 embrace.addSessionProperty("perm3", "value", true)
                 checkNotNull(harness.getLastSavedSession()?.getSessionSpan()).assertPropertyExistence(

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
@@ -14,6 +14,8 @@ import io.embrace.android.embracesdk.internal.session.orchestrator.SessionSnapsh
 import io.embrace.android.embracesdk.internal.spans.findAttributeValue
 import io.embrace.android.embracesdk.internal.utils.Provider
 import java.util.Locale
+import java.util.Queue
+import java.util.concurrent.ConcurrentLinkedQueue
 
 /**
  * A [DeliveryService] that records the last parameters used to invoke each method, and for the ones that need it, count the number of
@@ -28,8 +30,10 @@ public open class FakeDeliveryService : DeliveryService {
     public var eventSentAsyncInvokedCount: Int = 0
     public var lastSavedCrash: EventMessage? = null
     public var lastSentCachedSession: String? = null
-    public val sentSessionEnvelopes: MutableList<Pair<Envelope<SessionPayload>, SessionSnapshotType>> = mutableListOf()
-    public val savedSessionEnvelopes: MutableList<Pair<Envelope<SessionPayload>, SessionSnapshotType>> = mutableListOf()
+    public val sentSessionEnvelopes: Queue<Pair<Envelope<SessionPayload>, SessionSnapshotType>> =
+        ConcurrentLinkedQueue()
+    public val savedSessionEnvelopes: Queue<Pair<Envelope<SessionPayload>, SessionSnapshotType>> =
+        ConcurrentLinkedQueue()
 
     override fun sendSession(envelope: Envelope<SessionPayload>, snapshotType: SessionSnapshotType) {
         if (snapshotType != SessionSnapshotType.PERIODIC_CACHE) {