From 10a31276f9fa935ff7511891f9322a7f798658f3 Mon Sep 17 00:00:00 2001 From: Hanson Ho Date: Wed, 21 Aug 2024 08:25:27 -0700 Subject: [PATCH] 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. --- .../IntegrationTestRuleExtensions.kt | 2 +- .../features/SessionPropertiesTest.kt | 28 +++++++++---------- .../embracesdk/fakes/FakeDeliveryService.kt | 8 ++++-- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/IntegrationTestRuleExtensions.kt b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/IntegrationTestRuleExtensions.kt index d9a93d3a2a..9a354a5be0 100644 --- a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/IntegrationTestRuleExtensions.kt +++ b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/IntegrationTestRuleExtensions.kt @@ -76,7 +76,7 @@ internal fun IntegrationTestRule.Harness.getSentSessions(): List> { return overriddenDeliveryModule.deliveryService.getSentBackgroundActivities() diff --git a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/features/SessionPropertiesTest.kt b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/features/SessionPropertiesTest.kt index 60f33b05e3..12368fde16 100644 --- a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/features/SessionPropertiesTest.kt +++ b/embrace-android-sdk/src/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/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeDeliveryService.kt b/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeDeliveryService.kt index eb0dc295d8..a9670ef03b 100644 --- a/embrace-test-fakes/src/main/kotlin/io/embrace/android/embracesdk/fakes/FakeDeliveryService.kt +++ b/embrace-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, SessionSnapshotType>> = mutableListOf() - public val savedSessionEnvelopes: MutableList, SessionSnapshotType>> = mutableListOf() + public val sentSessionEnvelopes: Queue, SessionSnapshotType>> = + ConcurrentLinkedQueue() + public val savedSessionEnvelopes: Queue, SessionSnapshotType>> = + ConcurrentLinkedQueue() override fun sendSession(envelope: Envelope, snapshotType: SessionSnapshotType) { if (snapshotType != SessionSnapshotType.PERIODIC_CACHE) {