Open Improve ui and unit tests (#2992)

removed ui test EnvelopeTests.checkProfileNotSentIfEmpty. Added unit test with same goal: SentryEnvelopeItemTest.fromProfilingTrace with empty file throws added description messages to eventually failing benchmark tests added small sleeps in SentryWrapperTest to avoid race conditions
getsentry · Oct 18, 2023 · 12704a9 · 12704a9
1 parent 0bf143e
commit 12704a9
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 64 deletions.
diff --git a/...oid-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/SdkBenchmarkTest.kt b/...oid-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/SdkBenchmarkTest.kt
@@ -40,8 +40,8 @@ class SdkBenchmarkTest : BaseBenchmarkTest() {
         val perfProfilingSdkResult = perfProfilingSdkResults.getSummaryResult()
         perfProfilingSdkResult.printResults()
 
-        assertTrue(simpleSdkResult.cpuTimeIncreaseNanos in 0..TimeUnit.MILLISECONDS.toNanos(100))
-        assertTrue(perfProfilingSdkResult.cpuTimeIncreaseNanos in 0..TimeUnit.MILLISECONDS.toNanos(100))
+        assertTrue(simpleSdkResult.cpuTimeIncreaseNanos in 0..TimeUnit.MILLISECONDS.toNanos(100), "Expected ${simpleSdkResult.cpuTimeIncreaseNanos} to be in range 0 < x < 100000000")
+        assertTrue(perfProfilingSdkResult.cpuTimeIncreaseNanos in 0..TimeUnit.MILLISECONDS.toNanos(100), "Expected ${perfProfilingSdkResult.cpuTimeIncreaseNanos} to be in range 0 < x < 100000000")
     }
 
     private fun getOperation(init: (() -> Unit)? = null) = BenchmarkOperation(

diff --git a/...-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/SentryBenchmarkTest.kt b/...-benchmark/src/androidTest/java/io/sentry/uitest/android/benchmark/SentryBenchmarkTest.kt
@@ -46,7 +46,7 @@ class SentryBenchmarkTest : BaseBenchmarkTest() {
         comparisonResult.printResults()
 
         // Currently we just want to assert the cpu overhead
-        assertTrue(comparisonResult.cpuTimeIncreasePercentage in -2F..2F)
+        assertTrue(comparisonResult.cpuTimeIncreasePercentage in -2F..2F, "Expected ${comparisonResult.cpuTimeIncreasePercentage} to be in range -2 < x < 2")
         // The fps decrease comparison is skipped, due to approximation: 59.51 and 59.49 fps are considered 60 and 59,
         // respectively. Also, if the average fps is 20 or 60, a difference of 1 fps becomes 5% or 1.66% respectively.
     }
@@ -90,7 +90,7 @@ class SentryBenchmarkTest : BaseBenchmarkTest() {
         comparisonResult.printResults()
 
         // Currently we just want to assert the cpu overhead
-        assertTrue(comparisonResult.cpuTimeIncreasePercentage in 0F..5F)
+        assertTrue(comparisonResult.cpuTimeIncreasePercentage in 0F..5F, "Expected ${comparisonResult.cpuTimeIncreasePercentage} to be in range 0 < x < 5")
     }
 
     /**

diff --git a/...sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/AutomaticSpansTest.kt b/...sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/AutomaticSpansTest.kt
@@ -5,51 +5,47 @@ import androidx.test.core.app.launchActivity
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import io.sentry.Sentry
 import io.sentry.SentryLevel
-import io.sentry.SentryOptions
 import io.sentry.android.core.SentryAndroidOptions
 import io.sentry.protocol.SentryTransaction
 import org.junit.runner.RunWith
 import kotlin.test.Test
-import kotlin.test.assertEquals
 import kotlin.test.assertTrue
 
 @RunWith(AndroidJUnit4::class)
 class AutomaticSpansTest : BaseUiTest() {
 
     @Test
     fun ttidTtfdSpans() {
-        val transactions = mutableListOf<SentryTransaction>()
-
-        initSentry(false) { options: SentryAndroidOptions ->
+        initSentry(true) { options: SentryAndroidOptions ->
             options.isDebug = true
             options.setDiagnosticLevel(SentryLevel.DEBUG)
             options.tracesSampleRate = 1.0
             options.profilesSampleRate = 1.0
             options.isEnableAutoActivityLifecycleTracing = true
             options.isEnableTimeToFullDisplayTracing = true
-            options.beforeSendTransaction =
-                SentryOptions.BeforeSendTransactionCallback { transaction, _ ->
-                    transactions.add(transaction)
-                    transaction
-                }
         }
 
+        relayIdlingResource.increment()
         val activity = launchActivity<ComposeActivity>()
         activity.moveToState(Lifecycle.State.RESUMED)
         activity.onActivity {
             Sentry.reportFullyDisplayed()
         }
         activity.moveToState(Lifecycle.State.DESTROYED)
 
-        assertEquals(1, transactions.size)
-        assertTrue("TTID span missing") {
-            transactions.first().spans.any {
-                it.op == "ui.load.initial_display"
-            }
-        }
-        assertTrue("TTFD span missing") {
-            transactions.first().spans.any {
-                it.op == "ui.load.full_display"
+        relay.assert {
+            assertFirstEnvelope {
+                val transactionItem: SentryTransaction = it.assertItem()
+                assertTrue("TTID span missing") {
+                    transactionItem.spans.any { span ->
+                        span.op == "ui.load.initial_display"
+                    }
+                }
+                assertTrue("TTFD span missing") {
+                    transactionItem.spans.any { span ->
+                        span.op == "ui.load.full_display"
+                    }
+                }
             }
         }
     }

diff --git a/...ests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt b/...ests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt
@@ -17,7 +17,6 @@ import io.sentry.profilemeasurements.ProfileMeasurement
 import io.sentry.protocol.SentryTransaction
 import org.junit.Assume.assumeNotNull
 import org.junit.runner.RunWith
-import java.io.File
 import java.util.concurrent.TimeUnit
 import kotlin.test.Test
 import kotlin.test.assertEquals
@@ -171,44 +170,6 @@ class EnvelopeTests : BaseUiTest() {
         }
     }
 
-    @Test
-    fun checkProfileNotSentIfEmpty() {
-        initSentry(true) { options: SentryAndroidOptions ->
-            options.tracesSampleRate = 1.0
-            options.profilesSampleRate = 1.0
-        }
-        relayIdlingResource.increment()
-        val profilesDirPath = Sentry.getCurrentHub().options.profilingTracesDirPath
-        val transaction = Sentry.startTransaction("emptyProfileTransaction", "test empty")
-
-        var finished = false
-        Thread {
-            while (!finished) {
-                // Let's modify the trace file to be empty, so that the profile will actually be empty.
-                val origProfileFile = File(profilesDirPath!!).listFiles()?.maxByOrNull { f -> f.lastModified() }
-                origProfileFile?.writeBytes(ByteArray(0))
-            }
-        }.start()
-        transaction.finish()
-        // The profiler is stopped in background on the executor service, so we can stop deleting the trace file
-        // only after the profiler is stopped. This means we have to stop the deletion in the executorService
-        Sentry.getCurrentHub().options.executorService.submit {
-            finished = true
-        }
-
-        relay.assert {
-            findEnvelope {
-                assertEnvelopeItem<SentryTransaction>(it.items.toList()).transaction == "emptyProfileTransaction"
-            }.assert {
-                val transactionItem: SentryTransaction = it.assertItem()
-                it.assertNoOtherItems()
-                assertEquals("emptyProfileTransaction", transactionItem.transaction)
-            }
-            assertNoOtherEnvelopes()
-            assertNoOtherRequests()
-        }
-    }
-
     @Test
     fun checkTimedOutProfile() {
         // We increase the IdlingResources timeout to exceed the profiling timeout
@@ -235,7 +196,7 @@ class EnvelopeTests : BaseUiTest() {
                 assertEquals("timedOutProfile", transactionItem.transaction)
                 assertEquals("timedOutProfile", profilingTraceData.transactionName)
                 // The profile should timeout after 30 seconds
-                assertTrue(profilingTraceData.durationNs.toLong() < TimeUnit.SECONDS.toNanos(31))
+                assertTrue(profilingTraceData.durationNs.toLong() < TimeUnit.SECONDS.toNanos(31), "Profile duration expected to be less than 31 seconds. It was ${profilingTraceData.durationNs.toLong()} ns")
                 assertEquals(ProfilingTraceData.TRUNCATION_REASON_TIMEOUT, profilingTraceData.truncationReason)
             }
             assertNoOtherEnvelopes()

diff --git a/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt b/sentry/src/test/java/io/sentry/SentryEnvelopeItemTest.kt
@@ -307,14 +307,35 @@ class SentryEnvelopeItemTest {
         assertFailsWith<SentryEnvelopeException>("Dropping profiling trace data, because the file ${file.path} doesn't exists") {
             SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock()).data
         }
+    }
+
+    @Test
+    fun `fromProfilingTrace with unreadable file throws`() {
+        val file = File(fixture.pathname)
+        val profilingTraceData = mock<ProfilingTraceData> {
+            whenever(it.traceFile).thenReturn(file)
+        }
         file.writeBytes(fixture.bytes)
-        assertNotNull(SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock()).data)
         file.setReadable(false)
         assertFailsWith<SentryEnvelopeException>("Dropping profiling trace data, because the file ${file.path} doesn't exists") {
             SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock()).data
         }
     }
 
+    @Test
+    fun `fromProfilingTrace with empty file throws`() {
+        val file = File(fixture.pathname)
+        file.writeBytes(ByteArray(0))
+        val profilingTraceData = mock<ProfilingTraceData> {
+            whenever(it.traceFile).thenReturn(file)
+        }
+
+        val traceData = SentryEnvelopeItem.fromProfilingTrace(profilingTraceData, fixture.maxAttachmentSize, mock())
+        assertFailsWith<SentryEnvelopeException>("Profiling trace file is empty") {
+            traceData.data
+        }
+    }
+
     @Test
     fun `fromProfilingTrace with file too big`() {
         val file = File(fixture.pathname)

diff --git a/sentry/src/test/java/io/sentry/SentryWrapperTest.kt b/sentry/src/test/java/io/sentry/SentryWrapperTest.kt
@@ -37,6 +37,7 @@ class SentryWrapperTest {
         val callableFuture =
             CompletableFuture.supplyAsync(
                 SentryWrapper.wrapSupplier {
+                    Thread.sleep(20)
                     Sentry.addBreadcrumb("MyClonedBreadcrumb")
                     Sentry.captureMessage("ClonedMessage")
                     "Result 1"
@@ -47,6 +48,7 @@ class SentryWrapperTest {
         val callableFuture2 =
             CompletableFuture.supplyAsync(
                 SentryWrapper.wrapSupplier {
+                    Thread.sleep(10)
                     Sentry.addBreadcrumb("MyClonedBreadcrumb2")
                     Sentry.captureMessage("ClonedMessage2")
                     "Result 2"
@@ -87,6 +89,7 @@ class SentryWrapperTest {
 
         val future1 = executor.submit(
             SentryWrapper.wrapCallable {
+                Thread.sleep(20)
                 Sentry.addBreadcrumb("MyClonedBreadcrumb")
                 Sentry.captureMessage("ClonedMessage")
                 "Result 1"
@@ -95,6 +98,7 @@ class SentryWrapperTest {
 
         val future2 = executor.submit(
             SentryWrapper.wrapCallable {
+                Thread.sleep(10)
                 Sentry.addBreadcrumb("MyClonedBreadcrumb2")
                 Sentry.captureMessage("ClonedMessage2")
                 "Result 2"