From 937879e7bffe6eceeb2ed339971da869fd0df542 Mon Sep 17 00:00:00 2001 From: Stefano Date: Wed, 27 Dec 2023 17:05:43 +0100 Subject: [PATCH] Fix Ui tests receiving empty requests (#3094) * cleaned up MockRelay, removing requests that are not envelopes (we don't have such) * added a check to discard already received envelopes, sent twice for any reason) --- .../io/sentry/uitest/android/EnvelopeTests.kt | 3 - .../io/sentry/uitest/android/SdkInitTests.kt | 1 - .../uitest/android/mockservers/MockRelay.kt | 39 ++++++--- .../android/mockservers/RelayAsserter.kt | 79 ++++++++++--------- 4 files changed, 66 insertions(+), 56 deletions(-) diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt index fddd0680d1..4244d56774 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt @@ -42,7 +42,6 @@ class EnvelopeTests : BaseUiTest() { assertTrue(event.message?.formatted == "Message captured during test") } assertNoOtherEnvelopes() - assertNoOtherRequests() } } @@ -170,7 +169,6 @@ class EnvelopeTests : BaseUiTest() { assertNotNull(transactionData) } assertNoOtherEnvelopes() - assertNoOtherRequests() } } @@ -205,7 +203,6 @@ class EnvelopeTests : BaseUiTest() { assertEquals(ProfilingTraceData.TRUNCATION_REASON_TIMEOUT, profilingTraceData.truncationReason) } assertNoOtherEnvelopes() - assertNoOtherRequests() } } diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt index 8a5549b91c..89c269eaff 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt @@ -72,7 +72,6 @@ class SdkInitTests : BaseUiTest() { assertEquals("e2etests2", profilingTraceData.transactionName) } assertNoOtherEnvelopes() - assertNoOtherRequests() } } } diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/MockRelay.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/MockRelay.kt index ae28eb8e57..86e032e007 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/MockRelay.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/MockRelay.kt @@ -6,6 +6,7 @@ import okhttp3.mockwebserver.Dispatcher import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.RecordedRequest +import kotlin.test.assertNotNull /** Mocks a relay server. */ class MockRelay( @@ -22,15 +23,20 @@ class MockRelay( /** List of unasserted requests sent to the [envelopePath]. */ private val unassertedEnvelopes = mutableListOf() - /** List of unasserted requests not contained in [unassertedEnvelopes]. */ - private val unassertedRequests = mutableListOf() - /** List of responses to return when a request is sent. */ private val responses = mutableListOf<(RecordedRequest) -> MockResponse?>() + /** Set to check already received envelopes, to avoid duplicates due to e.g. retrying. */ + private val receivedEnvelopes: MutableSet = HashSet() + init { relay.dispatcher = object : Dispatcher() { override fun dispatch(request: RecordedRequest): MockResponse { + // If a request with a body size of 0 is received, we drop it. + // This shouldn't happen in reality, but it rarely happens in tests. + if (request.bodySize == 0L || request.failure != null) { + return MockResponse() + } // We check if there is any custom response previously set to return to this request, // otherwise we return a successful MockResponse. val response = responses.asSequence() @@ -38,15 +44,19 @@ class MockRelay( .firstOrNull() ?: MockResponse() - // Based on the path of the request, we populate the right list. - val relayResponse = RelayAsserter.RelayResponse(request, response) - when (request.path) { - envelopePath -> { - unassertedEnvelopes.add(relayResponse) - } - else -> { - unassertedRequests.add(relayResponse) + // We should receive only envelopes on this path. + if (request.path == envelopePath) { + val relayResponse = RelayAsserter.RelayResponse(request, response) + assertNotNull(relayResponse.envelope) + val envelopeId: String = relayResponse.envelope!!.header.eventId!!.toString() + // If we already received the envelope (e.g. retrying mechanism) we drop it + if (receivedEnvelopes.contains(envelopeId)) { + return MockResponse() } + receivedEnvelopes.add(envelopeId) + unassertedEnvelopes.add(relayResponse) + } else { + throw AssertionError("Expected $envelopePath, but the request path was ${request.path}") } // If we are waiting for requests to be received, we decrement the associated counter. @@ -62,7 +72,10 @@ class MockRelay( fun createMockDsn() = "http://key@${relay.hostName}:${relay.port}/$dsnProject" /** Starts the mock relay server. */ - fun start() = relay.start() + fun start() { + receivedEnvelopes.clear() + relay.start() + } /** Shutdown the mock relay server and clear everything. */ fun shutdown() { @@ -98,6 +111,6 @@ class MockRelay( if (waitForRequests) { waitUntilIdle() } - assertion(RelayAsserter(unassertedEnvelopes, unassertedRequests)) + assertion(RelayAsserter(unassertedEnvelopes)) } } diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/RelayAsserter.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/RelayAsserter.kt index 47e8f09da4..064bf724db 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/RelayAsserter.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/RelayAsserter.kt @@ -1,8 +1,14 @@ package io.sentry.uitest.android.mockservers import io.sentry.EnvelopeReader +import io.sentry.JsonSerializer +import io.sentry.ProfilingTraceData import io.sentry.Sentry import io.sentry.SentryEnvelope +import io.sentry.SentryEvent +import io.sentry.SentryItemType +import io.sentry.SentryOptions +import io.sentry.protocol.SentryTransaction import okhttp3.mockwebserver.MockResponse import okhttp3.mockwebserver.RecordedRequest import okio.GzipSource @@ -12,40 +18,18 @@ import java.util.zip.GZIPInputStream /** Class used to assert requests sent to [MockRelay]. */ class RelayAsserter( - private val unassertedEnvelopes: MutableList, - private val unassertedRequests: MutableList + private val unassertedEnvelopes: MutableList ) { - - /** - * Asserts an envelope request exists and allows to make other assertions on the first one and on its response. - * The asserted envelope request is then removed from internal list of unasserted envelopes. - */ - fun assertFirstRawEnvelope(assertion: ((relayResponse: RelayResponse) -> Unit)? = null) { - val relayResponse = unassertedEnvelopes.removeFirstOrNull() - ?: throw AssertionError("No envelope request found") - assertion?.let { it(relayResponse) } - } - - /** - * Asserts a request exists and makes other assertions on the first one and on its response. - * The asserted request is then removed from internal list of unasserted requests. - */ - fun assertRawRequest(assertion: ((request: RecordedRequest, response: MockResponse) -> Unit)? = null) { - val relayResponse = unassertedRequests.removeFirstOrNull() - ?: throw AssertionError("No raw request found") - assertion?.let { - it(relayResponse.request, relayResponse.response) - } - } + private val originalUnassertedEnvelopes: MutableList = ArrayList(unassertedEnvelopes) /** * Parses the first request as an envelope and makes other assertions through a [EnvelopeAsserter]. * The asserted envelope is then removed from internal list of unasserted envelopes. */ fun assertFirstEnvelope(assertion: (asserter: EnvelopeAsserter) -> Unit) { - assertFirstRawEnvelope { relayResponse -> - relayResponse.assert(assertion) - } + val relayResponse = unassertedEnvelopes.removeFirstOrNull() + ?: throw AssertionError("No envelope request found") + relayResponse.assert(assertion) } /** @@ -63,22 +47,39 @@ class RelayAsserter( /** Asserts no other envelopes were sent. */ fun assertNoOtherEnvelopes() { if (unassertedEnvelopes.isNotEmpty()) { - throw AssertionError("There were other ${unassertedEnvelopes.size} envelope requests: $unassertedEnvelopes") + throw AssertionError( + "There was a total of ${originalUnassertedEnvelopes.size} envelopes: " + + originalUnassertedEnvelopes.joinToString { describeEnvelope(it.envelope!!) } + ) } } - /** Asserts no other raw requests were sent. */ - fun assertNoOtherRawRequests() { - assertNoOtherEnvelopes() - if (unassertedRequests.isNotEmpty()) { - throw AssertionError("There were other ${unassertedRequests.size} requests: $unassertedRequests") + /** Function used to describe the content of the envelope to print in the logs. For debugging purposes only. */ + private fun describeEnvelope(envelope: SentryEnvelope): String { + var descr = "" + envelope.items.forEach { item -> + when (item.header.type) { + SentryItemType.Event -> { + val deserialized = JsonSerializer(SentryOptions()) + .deserialize(item.data.inputStream().reader(), SentryEvent::class.java)!! + descr += "Event (${deserialized.eventId}) - message: ${deserialized.message!!.formatted} -- " + } + SentryItemType.Transaction -> { + val deserialized = JsonSerializer(SentryOptions()) + .deserialize(item.data.inputStream().reader(), SentryTransaction::class.java)!! + descr += "Transaction (${deserialized.eventId}) - transaction: ${deserialized.transaction} - spans: ${deserialized.spans.joinToString { "${it.op} ${it.description}" }} -- " + } + SentryItemType.Profile -> { + val deserialized = JsonSerializer(SentryOptions()) + .deserialize(item.data.inputStream().reader(), ProfilingTraceData::class.java)!! + descr += "Profile (${deserialized.profileId}) - transactionName: ${deserialized.transactionName} -- " + } + else -> { + descr += "${item.header.type} -- " + } + } } - } - - /** Asserts no other requests or envelopes were sent. */ - fun assertNoOtherRequests() { - assertNoOtherEnvelopes() - assertNoOtherRawRequests() + return "*** Envelope: $descr ***" } data class RelayResponse(val request: RecordedRequest, val response: MockResponse) {