From 7d4a15266c217b81abe1e5c6327989e42dd031c3 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 9 Nov 2023 12:31:13 +0100 Subject: [PATCH 1/4] moved several integrations to the background to reduce main thread usage on SDK init: -AnrIntegration -PhoneStateBreadcrumbsIntegration -SystemEventsBreadcrumbsIntegration -TempSensorBreadcrumbsIntegration --- .../sentry/android/core/AnrIntegration.java | 57 ++++++++++++------- .../PhoneStateBreadcrumbsIntegration.java | 47 +++++++++------ .../SystemEventsBreadcrumbsIntegration.java | 44 +++++++++----- .../TempSensorBreadcrumbsIntegration.java | 50 +++++++++------- .../sentry/android/core/AnrIntegrationTest.kt | 15 ++++- .../PhoneStateBreadcrumbsIntegrationTest.kt | 40 ++++++++----- .../SystemEventsBreadcrumbsIntegrationTest.kt | 14 ++++- .../TempSensorBreadcrumbsIntegrationTest.kt | 51 +++++++++++------ .../api/sentry-test-support.api | 1 + .../main/kotlin/io/sentry/test/Reflection.kt | 7 +++ sentry/src/test/java/io/sentry/SentryTest.kt | 2 + 11 files changed, 224 insertions(+), 104 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java index 4c6ac6373a..56f5c28e5b 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java @@ -55,27 +55,42 @@ private void register(final @NotNull IHub hub, final @NotNull SentryAndroidOptio .log(SentryLevel.DEBUG, "AnrIntegration enabled: %s", options.isAnrEnabled()); if (options.isAnrEnabled()) { - synchronized (watchDogLock) { - if (anrWatchDog == null) { - options - .getLogger() - .log( - SentryLevel.DEBUG, - "ANR timeout in milliseconds: %d", - options.getAnrTimeoutIntervalMillis()); - - anrWatchDog = - new ANRWatchDog( - options.getAnrTimeoutIntervalMillis(), - options.isAnrReportInDebug(), - error -> reportANR(hub, options, error), - options.getLogger(), - context); - anrWatchDog.start(); - - options.getLogger().log(SentryLevel.DEBUG, "AnrIntegration installed."); - addIntegrationToSdkVersion(); - } + addIntegrationToSdkVersion(); + try { + options.getExecutorService().submit(() -> startAnrWatchdog(hub, options)); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Failed to start AnrIntegration on executor thread. Starting on the calling thread.", + e); + startAnrWatchdog(hub, options); + } + } + } + + private void startAnrWatchdog( + final @NotNull IHub hub, final @NotNull SentryAndroidOptions options) { + synchronized (watchDogLock) { + if (anrWatchDog == null) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "ANR timeout in milliseconds: %d", + options.getAnrTimeoutIntervalMillis()); + + anrWatchDog = + new ANRWatchDog( + options.getAnrTimeoutIntervalMillis(), + options.isAnrReportInDebug(), + error -> reportANR(hub, options, error), + options.getLogger(), + context); + anrWatchDog.start(); + + options.getLogger().log(SentryLevel.DEBUG, "AnrIntegration installed."); } } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java index be23c668c7..11e2210292 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java @@ -28,7 +28,6 @@ public PhoneStateBreadcrumbsIntegration(final @NotNull Context context) { this.context = Objects.requireNonNull(context, "Context is required"); } - @SuppressWarnings("deprecation") @Override public void register(final @NotNull IHub hub, final @NotNull SentryOptions options) { Objects.requireNonNull(hub, "Hub is required"); @@ -46,22 +45,38 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio if (this.options.isEnableSystemEventBreadcrumbs() && Permissions.hasPermission(context, READ_PHONE_STATE)) { - telephonyManager = (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE); - if (telephonyManager != null) { - try { - listener = new PhoneStateChangeListener(hub); - telephonyManager.listen(listener, android.telephony.PhoneStateListener.LISTEN_CALL_STATE); - - options.getLogger().log(SentryLevel.DEBUG, "PhoneStateBreadcrumbsIntegration installed."); - addIntegrationToSdkVersion(); - } catch (Throwable e) { - this.options - .getLogger() - .log(SentryLevel.INFO, e, "TelephonyManager is not available or ready to use."); - } - } else { - this.options.getLogger().log(SentryLevel.INFO, "TelephonyManager is not available"); + try { + options.getExecutorService().submit(() -> startTelephonyListener(hub, options)); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Failed to start PhoneStateBreadcrumbsIntegration on executor thread. Starting on the calling thread.", + e); + startTelephonyListener(hub, options); + } + } + } + + @SuppressWarnings("deprecation") + private void startTelephonyListener( + final @NotNull IHub hub, final @NotNull SentryOptions options) { + telephonyManager = (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE); + if (telephonyManager != null) { + try { + listener = new PhoneStateChangeListener(hub); + telephonyManager.listen(listener, android.telephony.PhoneStateListener.LISTEN_CALL_STATE); + + options.getLogger().log(SentryLevel.DEBUG, "PhoneStateBreadcrumbsIntegration installed."); + addIntegrationToSdkVersion(); + } catch (Throwable e) { + options + .getLogger() + .log(SentryLevel.INFO, e, "TelephonyManager is not available or ready to use."); } + } else { + options.getLogger().log(SentryLevel.INFO, "TelephonyManager is not available"); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index 9f7343fc8f..a8d299f6b2 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -92,27 +92,43 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio this.options.isEnableSystemEventBreadcrumbs()); if (this.options.isEnableSystemEventBreadcrumbs()) { - receiver = new SystemEventsBroadcastReceiver(hub, this.options.getLogger()); - final IntentFilter filter = new IntentFilter(); - for (String item : actions) { - filter.addAction(item); - } + try { - // registerReceiver can throw SecurityException but it's not documented in the official docs - ContextUtils.registerReceiver(context, options, receiver, filter); - this.options - .getLogger() - .log(SentryLevel.DEBUG, "SystemEventsBreadcrumbsIntegration installed."); - addIntegrationToSdkVersion(); + options + .getExecutorService() + .submit(() -> startSystemEventsReceiver(hub, (SentryAndroidOptions) options)); } catch (Throwable e) { - this.options.setEnableSystemEventBreadcrumbs(false); - this.options + options .getLogger() - .log(SentryLevel.ERROR, "Failed to initialize SystemEventsBreadcrumbsIntegration.", e); + .log( + SentryLevel.DEBUG, + "Failed to start SystemEventsBreadcrumbsIntegration on executor thread. Starting on the calling thread.", + e); + startSystemEventsReceiver(hub, (SentryAndroidOptions) options); } } } + private void startSystemEventsReceiver( + final @NotNull IHub hub, final @NotNull SentryAndroidOptions options) { + receiver = new SystemEventsBroadcastReceiver(hub, options.getLogger()); + final IntentFilter filter = new IntentFilter(); + for (String item : actions) { + filter.addAction(item); + } + try { + // registerReceiver can throw SecurityException but it's not documented in the official docs + ContextUtils.registerReceiver(context, options, receiver, filter); + options.getLogger().log(SentryLevel.DEBUG, "SystemEventsBreadcrumbsIntegration installed."); + addIntegrationToSdkVersion(); + } catch (Throwable e) { + options.setEnableSystemEventBreadcrumbs(false); + options + .getLogger() + .log(SentryLevel.ERROR, "Failed to initialize SystemEventsBreadcrumbsIntegration.", e); + } + } + @SuppressWarnings("deprecation") private static @NotNull List getDefaultActions() { final List actions = new ArrayList<>(); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java index e4577b43ec..f8b24dbecb 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java @@ -34,7 +34,6 @@ public TempSensorBreadcrumbsIntegration(final @NotNull Context context) { this.context = Objects.requireNonNull(context, "Context is required"); } - @SuppressWarnings("deprecation") @Override public void register(final @NotNull IHub hub, final @NotNull SentryOptions options) { this.hub = Objects.requireNonNull(hub, "Hub is required"); @@ -51,29 +50,40 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio this.options.isEnableSystemEventBreadcrumbs()); if (this.options.isEnableSystemEventBreadcrumbs()) { + try { - sensorManager = (SensorManager) context.getSystemService(SENSOR_SERVICE); - if (sensorManager != null) { - final Sensor defaultSensor = - sensorManager.getDefaultSensor(Sensor.TYPE_AMBIENT_TEMPERATURE); - if (defaultSensor != null) { - sensorManager.registerListener(this, defaultSensor, SensorManager.SENSOR_DELAY_NORMAL); - - options - .getLogger() - .log(SentryLevel.DEBUG, "TempSensorBreadcrumbsIntegration installed."); - addIntegrationToSdkVersion(); - } else { - this.options - .getLogger() - .log(SentryLevel.INFO, "TYPE_AMBIENT_TEMPERATURE is not available."); - } + options.getExecutorService().submit(() -> startSensorListener(options)); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Failed to start TempSensorBreadcrumbsIntegration on executor thread. Starting on the calling thread.", + e); + startSensorListener(options); + } + } + } + + private void startSensorListener(final @NotNull SentryOptions options) { + try { + sensorManager = (SensorManager) context.getSystemService(SENSOR_SERVICE); + if (sensorManager != null) { + final Sensor defaultSensor = + sensorManager.getDefaultSensor(Sensor.TYPE_AMBIENT_TEMPERATURE); + if (defaultSensor != null) { + sensorManager.registerListener(this, defaultSensor, SensorManager.SENSOR_DELAY_NORMAL); + + options.getLogger().log(SentryLevel.DEBUG, "TempSensorBreadcrumbsIntegration installed."); + addIntegrationToSdkVersion(); } else { - this.options.getLogger().log(SentryLevel.INFO, "SENSOR_SERVICE is not available."); + options.getLogger().log(SentryLevel.INFO, "TYPE_AMBIENT_TEMPERATURE is not available."); } - } catch (Throwable e) { - options.getLogger().log(SentryLevel.ERROR, e, "Failed to init. the SENSOR_SERVICE."); + } else { + options.getLogger().log(SentryLevel.INFO, "SENSOR_SERVICE is not available."); } + } catch (Throwable e) { + options.getLogger().log(SentryLevel.ERROR, e, "Failed to init. the SENSOR_SERVICE."); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt index 55386b1c7a..1149ffe7a0 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt @@ -6,6 +6,7 @@ import io.sentry.IHub import io.sentry.SentryLevel import io.sentry.android.core.AnrIntegration.AnrHint import io.sentry.exception.ExceptionMechanismException +import io.sentry.test.ImmediateExecutorService import io.sentry.util.HintUtils import org.mockito.kotlin.any import org.mockito.kotlin.check @@ -23,7 +24,7 @@ class AnrIntegrationTest { private class Fixture { val context = mock() val hub = mock() - val options = SentryAndroidOptions().apply { + var options: SentryAndroidOptions = SentryAndroidOptions().apply { setLogger(mock()) } @@ -44,6 +45,7 @@ class AnrIntegrationTest { @Test fun `When ANR is enabled, ANR watch dog should be started`() { + fixture.options.executorService = ImmediateExecutorService() val sut = fixture.getSut() sut.register(fixture.hub, fixture.options) @@ -52,6 +54,16 @@ class AnrIntegrationTest { assertTrue((sut.anrWatchDog as ANRWatchDog).isAlive) } + @Test + fun `ANR watch dog should be started in the executorService`() { + fixture.options.executorService = mock() + val sut = fixture.getSut() + + sut.register(fixture.hub, fixture.options) + + assertNull(sut.anrWatchDog) + } + @Test fun `When ANR is disabled, ANR should not be started`() { val sut = fixture.getSut() @@ -82,6 +94,7 @@ class AnrIntegrationTest { @Test fun `When ANR integration is closed, watch dog should stop`() { val sut = fixture.getSut() + fixture.options.executorService = ImmediateExecutorService() sut.register(fixture.hub, fixture.options) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt index 00bddf7a6a..1f6c5090e3 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt @@ -5,7 +5,9 @@ import android.telephony.PhoneStateListener import android.telephony.TelephonyManager import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.ISentryExecutorService import io.sentry.SentryLevel +import io.sentry.test.ImmediateExecutorService import org.mockito.kotlin.any import org.mockito.kotlin.check import org.mockito.kotlin.eq @@ -23,8 +25,10 @@ class PhoneStateBreadcrumbsIntegrationTest { private class Fixture { val context = mock() val manager = mock() + val options = SentryAndroidOptions() - fun getSut(): PhoneStateBreadcrumbsIntegration { + fun getSut(executorService: ISentryExecutorService = ImmediateExecutorService()): PhoneStateBreadcrumbsIntegration { + options.executorService = executorService whenever(context.getSystemService(eq(Context.TELEPHONY_SERVICE))).thenReturn(manager) return PhoneStateBreadcrumbsIntegration(context) @@ -36,21 +40,31 @@ class PhoneStateBreadcrumbsIntegrationTest { @Test fun `When system events breadcrumb is enabled, it registers callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) verify(fixture.manager).listen(any(), eq(PhoneStateListener.LISTEN_CALL_STATE)) assertNotNull(sut.listener) } + @Test + fun `Phone state callback is registered in the executorService`() { + val sut = fixture.getSut(mock()) + val hub = mock() + sut.register(hub, fixture.options) + + assertNull(sut.listener) + } + @Test fun `When system events breadcrumb is disabled, it doesn't register callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions().apply { - isEnableSystemEventBreadcrumbs = false - } val hub = mock() - sut.register(hub, options) + sut.register( + hub, + fixture.options.apply { + isEnableSystemEventBreadcrumbs = false + } + ) verify(fixture.manager, never()).listen(any(), any()) assertNull(sut.listener) } @@ -58,9 +72,8 @@ class PhoneStateBreadcrumbsIntegrationTest { @Test fun `When ActivityBreadcrumbsIntegration is closed, it should unregister the callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) sut.close() verify(fixture.manager).listen(any(), eq(PhoneStateListener.LISTEN_NONE)) assertNull(sut.listener) @@ -69,9 +82,8 @@ class PhoneStateBreadcrumbsIntegrationTest { @Test fun `When on call state received, added breadcrumb with type and category`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) sut.listener!!.onCallStateChanged(TelephonyManager.CALL_STATE_RINGING, null) verify(hub).addBreadcrumb( @@ -87,9 +99,8 @@ class PhoneStateBreadcrumbsIntegrationTest { @Test fun `When on idle state received, added breadcrumb with type and category`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) sut.listener!!.onCallStateChanged(TelephonyManager.CALL_STATE_IDLE, null) verify(hub, never()).addBreadcrumb(any()) } @@ -97,9 +108,8 @@ class PhoneStateBreadcrumbsIntegrationTest { @Test fun `When on offhook state received, added breadcrumb with type and category`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) sut.listener!!.onCallStateChanged(TelephonyManager.CALL_STATE_OFFHOOK, null) verify(hub, never()).addBreadcrumb(any()) } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index c0f7ce5abf..dd9dcd8e3d 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -4,7 +4,9 @@ import android.content.Context import android.content.Intent import io.sentry.Breadcrumb import io.sentry.IHub +import io.sentry.ISentryExecutorService import io.sentry.SentryLevel +import io.sentry.test.ImmediateExecutorService import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check @@ -25,9 +27,10 @@ class SystemEventsBreadcrumbsIntegrationTest { var options = SentryAndroidOptions() val hub = mock() - fun getSut(enableSystemEventBreadcrumbs: Boolean = true): SystemEventsBreadcrumbsIntegration { + fun getSut(enableSystemEventBreadcrumbs: Boolean = true, executorService: ISentryExecutorService = ImmediateExecutorService()): SystemEventsBreadcrumbsIntegration { options = SentryAndroidOptions().apply { isEnableSystemEventBreadcrumbs = enableSystemEventBreadcrumbs + this.executorService = executorService } return SystemEventsBreadcrumbsIntegration(context) } @@ -45,6 +48,15 @@ class SystemEventsBreadcrumbsIntegrationTest { assertNotNull(sut.receiver) } + @Test + fun `system events callback is registered in the executorService`() { + val sut = fixture.getSut(executorService = mock()) + val hub = mock() + sut.register(hub, fixture.options) + + assertNull(sut.receiver) + } + @Test fun `When system events breadcrumb is disabled, it doesn't register callback`() { val sut = fixture.getSut(enableSystemEventBreadcrumbs = false) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt index 29cebed6bd..96b5bb912a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt @@ -6,8 +6,14 @@ import android.hardware.SensorEvent import android.hardware.SensorEventListener import android.hardware.SensorManager import io.sentry.Breadcrumb +import io.sentry.Hint import io.sentry.IHub +import io.sentry.ISentryExecutorService import io.sentry.SentryLevel +import io.sentry.TypeCheckHint +import io.sentry.test.ImmediateExecutorService +import io.sentry.test.getDeclaredCtor +import io.sentry.test.injectForField import org.mockito.kotlin.any import org.mockito.kotlin.check import org.mockito.kotlin.eq @@ -15,7 +21,6 @@ import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify import org.mockito.kotlin.whenever -import kotlin.test.Ignore import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -26,8 +31,10 @@ class TempSensorBreadcrumbsIntegrationTest { val context = mock() val manager = mock() val sensor = mock() + val options = SentryAndroidOptions() - fun getSut(): TempSensorBreadcrumbsIntegration { + fun getSut(executorService: ISentryExecutorService = ImmediateExecutorService()): TempSensorBreadcrumbsIntegration { + options.executorService = executorService whenever(context.getSystemService(Context.SENSOR_SERVICE)).thenReturn(manager) whenever(manager.getDefaultSensor(Sensor.TYPE_AMBIENT_TEMPERATURE)).thenReturn(sensor) return TempSensorBreadcrumbsIntegration(context) @@ -39,21 +46,31 @@ class TempSensorBreadcrumbsIntegrationTest { @Test fun `When system events breadcrumb is enabled, it registers callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) verify(fixture.manager).registerListener(any(), any(), eq(SensorManager.SENSOR_DELAY_NORMAL)) assertNotNull(sut.sensorManager) } + @Test + fun `temp sensor listener is registered in the executorService`() { + val sut = fixture.getSut(executorService = mock()) + val hub = mock() + sut.register(hub, fixture.options) + + assertNull(sut.sensorManager) + } + @Test fun `When system events breadcrumb is disabled, it should not register a callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions().apply { - isEnableSystemEventBreadcrumbs = false - } val hub = mock() - sut.register(hub, options) + sut.register( + hub, + fixture.options.apply { + isEnableSystemEventBreadcrumbs = false + } + ) verify(fixture.manager, never()).registerListener(any(), any(), any()) assertNull(sut.sensorManager) } @@ -61,28 +78,31 @@ class TempSensorBreadcrumbsIntegrationTest { @Test fun `When TempSensorBreadcrumbsIntegration is closed, it should unregister the callback`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) sut.close() verify(fixture.manager).unregisterListener(any()) assertNull(sut.sensorManager) } - @Ignore("SensorEvent.values is always null, even when mocking it") @Test fun `When onSensorChanged received, add a breadcrumb with type and category`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) - sut.onSensorChanged(mock()) + sut.register(hub, fixture.options) + val sensorCtor = "android.hardware.SensorEvent".getDeclaredCtor(emptyArray()) + val sensorEvent: SensorEvent = sensorCtor.newInstance() as SensorEvent + sensorEvent.injectForField("values", FloatArray(2) { 1F }) + sut.onSensorChanged(sensorEvent) verify(hub).addBreadcrumb( check { assertEquals("device.event", it.category) assertEquals("system", it.type) assertEquals(SentryLevel.INFO, it.level) + }, + check { + assertEquals(sensorEvent, it.get(TypeCheckHint.ANDROID_SENSOR_EVENT)) } ) } @@ -90,9 +110,8 @@ class TempSensorBreadcrumbsIntegrationTest { @Test fun `When onSensorChanged received and null values, do not add a breadcrumb`() { val sut = fixture.getSut() - val options = SentryAndroidOptions() val hub = mock() - sut.register(hub, options) + sut.register(hub, fixture.options) val event = mock() assertNull(event.values) sut.onSensorChanged(event) diff --git a/sentry-test-support/api/sentry-test-support.api b/sentry-test-support/api/sentry-test-support.api index 5de79d4a85..f98d2e4bdb 100644 --- a/sentry-test-support/api/sentry-test-support.api +++ b/sentry-test-support/api/sentry-test-support.api @@ -24,5 +24,6 @@ public final class io/sentry/test/ReflectionKt { public static final fun containsMethod (Ljava/lang/Class;Ljava/lang/String;Ljava/lang/Class;)Z public static final fun containsMethod (Ljava/lang/Class;Ljava/lang/String;[Ljava/lang/Class;)Z public static final fun getCtor (Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Constructor; + public static final fun getDeclaredCtor (Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Constructor; } diff --git a/sentry-test-support/src/main/kotlin/io/sentry/test/Reflection.kt b/sentry-test-support/src/main/kotlin/io/sentry/test/Reflection.kt index f76d86e817..690dcc8725 100644 --- a/sentry-test-support/src/main/kotlin/io/sentry/test/Reflection.kt +++ b/sentry-test-support/src/main/kotlin/io/sentry/test/Reflection.kt @@ -52,3 +52,10 @@ fun String.getCtor(ctorTypes: Array>): Constructor<*> { val clazz = Class.forName(this) return clazz.getConstructor(*ctorTypes) } + +fun String.getDeclaredCtor(ctorTypes: Array>): Constructor<*> { + val clazz = Class.forName(this) + val constructor = clazz.getDeclaredConstructor(*ctorTypes) + constructor.isAccessible = true + return constructor +} diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index af33994542..a9a96936f5 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -285,6 +285,8 @@ class SentryTest { dir.mkdirs() oldProfile.createNewFile() newProfile.createNewFile() + // Make the old profile look like it's created earlier + oldProfile.setLastModified(System.currentTimeMillis() - 10000) // Make the new profile look like it's created later newProfile.setLastModified(System.currentTimeMillis() + 10000) From ea568f208cee2a14c2d62e2a2eafb67535b70110 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 9 Nov 2023 12:41:12 +0100 Subject: [PATCH 2/4] updated changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 33cbdb1ecf..05d1afc572 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Reduce main thread work on init ([#3036](https://github.com/getsentry/sentry-java/pull/3036)) +- Move Integrations registration to background on init ([#3043](https://github.com/getsentry/sentry-java/pull/3043)) ## 6.33.1 From dc086942330f68a7ab448ec769e486da487f6a3c Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 17 Nov 2023 15:27:32 +0100 Subject: [PATCH 3/4] added synchronization between register() and close() methods of Integrations created a DeferredExecutorService for tests --- .../sentry/android/core/AnrIntegration.java | 16 +++++++- .../PhoneStateBreadcrumbsIntegration.java | 16 +++++++- .../SystemEventsBreadcrumbsIntegration.java | 14 ++++++- .../TempSensorBreadcrumbsIntegration.java | 16 +++++++- .../core/ActivityLifecycleIntegrationTest.kt | 38 ++++--------------- .../sentry/android/core/AnrIntegrationTest.kt | 13 +++++++ .../PhoneStateBreadcrumbsIntegrationTest.kt | 12 ++++++ .../SystemEventsBreadcrumbsIntegrationTest.kt | 12 ++++++ .../TempSensorBreadcrumbsIntegrationTest.kt | 12 ++++++ .../okhttp/SentryOkHttpEventListenerTest.kt | 15 +++----- .../android/okhttp/SentryOkHttpEventTest.kt | 11 +----- .../api/sentry-test-support.api | 1 + .../src/main/kotlin/io/sentry/test/Mocks.kt | 12 +++++- ...aultTransactionPerformanceCollectorTest.kt | 21 ++-------- 14 files changed, 137 insertions(+), 72 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java index 56f5c28e5b..3ead05dd3e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrIntegration.java @@ -27,6 +27,8 @@ public final class AnrIntegration implements Integration, Closeable { private final @NotNull Context context; + private boolean isClosed = false; + private final @NotNull Object startLock = new Object(); public AnrIntegration(final @NotNull Context context) { this.context = context; @@ -57,7 +59,16 @@ private void register(final @NotNull IHub hub, final @NotNull SentryAndroidOptio if (options.isAnrEnabled()) { addIntegrationToSdkVersion(); try { - options.getExecutorService().submit(() -> startAnrWatchdog(hub, options)); + options + .getExecutorService() + .submit( + () -> { + synchronized (startLock) { + if (!isClosed) { + startAnrWatchdog(hub, options); + } + } + }); } catch (Throwable e) { options .getLogger() @@ -141,6 +152,9 @@ ANRWatchDog getANRWatchDog() { @Override public void close() throws IOException { + synchronized (startLock) { + isClosed = true; + } synchronized (watchDogLock) { if (anrWatchDog != null) { anrWatchDog.interrupt(); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java index 11e2210292..8f766375ee 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegration.java @@ -23,6 +23,8 @@ public final class PhoneStateBreadcrumbsIntegration implements Integration, Clos private @Nullable SentryAndroidOptions options; @TestOnly @Nullable PhoneStateChangeListener listener; private @Nullable TelephonyManager telephonyManager; + private boolean isClosed = false; + private final @NotNull Object startLock = new Object(); public PhoneStateBreadcrumbsIntegration(final @NotNull Context context) { this.context = Objects.requireNonNull(context, "Context is required"); @@ -46,7 +48,16 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio if (this.options.isEnableSystemEventBreadcrumbs() && Permissions.hasPermission(context, READ_PHONE_STATE)) { try { - options.getExecutorService().submit(() -> startTelephonyListener(hub, options)); + options + .getExecutorService() + .submit( + () -> { + synchronized (startLock) { + if (!isClosed) { + startTelephonyListener(hub, options); + } + } + }); } catch (Throwable e) { options .getLogger() @@ -83,6 +94,9 @@ private void startTelephonyListener( @SuppressWarnings("deprecation") @Override public void close() throws IOException { + synchronized (startLock) { + isClosed = true; + } if (telephonyManager != null && listener != null) { telephonyManager.listen(listener, android.telephony.PhoneStateListener.LISTEN_NONE); listener = null; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index a8d299f6b2..d7254155e3 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -65,6 +65,8 @@ public final class SystemEventsBreadcrumbsIntegration implements Integration, Cl private @Nullable SentryAndroidOptions options; private final @NotNull List actions; + private boolean isClosed = false; + private final @NotNull Object startLock = new Object(); public SystemEventsBreadcrumbsIntegration(final @NotNull Context context) { this(context, getDefaultActions()); @@ -96,7 +98,14 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio try { options .getExecutorService() - .submit(() -> startSystemEventsReceiver(hub, (SentryAndroidOptions) options)); + .submit( + () -> { + synchronized (startLock) { + if (!isClosed) { + startSystemEventsReceiver(hub, (SentryAndroidOptions) options); + } + } + }); } catch (Throwable e) { options .getLogger() @@ -180,6 +189,9 @@ private void startSystemEventsReceiver( @Override public void close() throws IOException { + synchronized (startLock) { + isClosed = true; + } if (receiver != null) { context.unregisterReceiver(receiver); receiver = null; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java index f8b24dbecb..caff147df5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/TempSensorBreadcrumbsIntegration.java @@ -29,6 +29,8 @@ public final class TempSensorBreadcrumbsIntegration private @Nullable SentryAndroidOptions options; @TestOnly @Nullable SensorManager sensorManager; + private boolean isClosed = false; + private final @NotNull Object startLock = new Object(); public TempSensorBreadcrumbsIntegration(final @NotNull Context context) { this.context = Objects.requireNonNull(context, "Context is required"); @@ -52,7 +54,16 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio if (this.options.isEnableSystemEventBreadcrumbs()) { try { - options.getExecutorService().submit(() -> startSensorListener(options)); + options + .getExecutorService() + .submit( + () -> { + synchronized (startLock) { + if (!isClosed) { + startSensorListener(options); + } + } + }); } catch (Throwable e) { options .getLogger() @@ -89,6 +100,9 @@ private void startSensorListener(final @NotNull SentryOptions options) { @Override public void close() throws IOException { + synchronized (startLock) { + isClosed = true; + } if (sensorManager != null) { sensorManager.unregisterListener(this); sensorManager = null; diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index 26bd42e5ab..8cb1a9db50 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -14,7 +14,6 @@ import io.sentry.Breadcrumb import io.sentry.DateUtils import io.sentry.FullyDisplayedReporter import io.sentry.Hub -import io.sentry.ISentryExecutorService import io.sentry.Scope import io.sentry.ScopeCallback import io.sentry.Sentry @@ -32,6 +31,7 @@ import io.sentry.TransactionFinishedCallback import io.sentry.TransactionOptions import io.sentry.protocol.MeasurementValue import io.sentry.protocol.TransactionNameSource +import io.sentry.test.DeferredExecutorService import io.sentry.test.getProperty import org.junit.runner.RunWith import org.mockito.ArgumentCaptor @@ -48,9 +48,7 @@ import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.Shadows.shadowOf import java.util.Date -import java.util.concurrent.Callable import java.util.concurrent.Future -import java.util.concurrent.FutureTask import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -1096,20 +1094,10 @@ class ActivityLifecycleIntegrationTest { @Test fun `When isEnableTimeToFullDisplayTracing is true and reportFullyDrawn is not called, ttfd span is finished automatically with timeout`() { val sut = fixture.getSut() - var lastScheduledRunnable: Runnable? = null - val mockExecutorService = object : ISentryExecutorService { - override fun submit(runnable: Runnable): Future<*> = mock() - override fun submit(callable: Callable): Future = mock() - override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { - lastScheduledRunnable = runnable - return FutureTask {} - } - override fun close(timeoutMillis: Long) {} - override fun isClosed() = false - } + val deferredExecutorService = DeferredExecutorService() fixture.options.tracesSampleRate = 1.0 fixture.options.isEnableTimeToFullDisplayTracing = true - fixture.options.executorService = mockExecutorService + fixture.options.executorService = deferredExecutorService sut.register(fixture.hub, fixture.options) val activity = mock() sut.onActivityCreated(activity, fixture.bundle) @@ -1118,10 +1106,10 @@ class ActivityLifecycleIntegrationTest { // Assert the ttfd span is running and a timeout autoCancel task has been scheduled assertNotNull(ttfdSpan) assertFalse(ttfdSpan.isFinished) - assertNotNull(lastScheduledRunnable) + assertTrue(deferredExecutorService.scheduledRunnables.isNotEmpty()) // Run the autoClose task and assert the ttfd span is finished with deadlineExceeded - lastScheduledRunnable!!.run() + deferredExecutorService.runAll() assertTrue(ttfdSpan.isFinished) assertEquals(SpanStatus.DEADLINE_EXCEEDED, ttfdSpan.status) @@ -1323,20 +1311,10 @@ class ActivityLifecycleIntegrationTest { val sut = fixture.getSut() val activity = mock() val view = fixture.createView() - var lastScheduledRunnable: Runnable? = null - val mockExecutorService = object : ISentryExecutorService { - override fun submit(runnable: Runnable): Future<*> = mock() - override fun submit(callable: Callable): Future = mock() - override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { - lastScheduledRunnable = runnable - return FutureTask {} - } - override fun close(timeoutMillis: Long) {} - override fun isClosed() = false - } + val deferredExecutorService = DeferredExecutorService() fixture.options.tracesSampleRate = 1.0 fixture.options.isEnableTimeToFullDisplayTracing = true - fixture.options.executorService = mockExecutorService + fixture.options.executorService = deferredExecutorService sut.register(fixture.hub, fixture.options) sut.onActivityCreated(activity, fixture.bundle) sut.onActivityResumed(activity) @@ -1355,7 +1333,7 @@ class ActivityLifecycleIntegrationTest { // Run the autoClose task 1 ms after finishing the ttid span and assert the ttfd span is finished Thread.sleep(1) - lastScheduledRunnable!!.run() + deferredExecutorService.runAll() assertTrue(ttfdSpan.isFinished) // the ttfd span should be trimmed to be equal to the ttid span, and the description should end with "-exceeded" diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt index 1149ffe7a0..cceabc9774 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AnrIntegrationTest.kt @@ -6,6 +6,7 @@ import io.sentry.IHub import io.sentry.SentryLevel import io.sentry.android.core.AnrIntegration.AnrHint import io.sentry.exception.ExceptionMechanismException +import io.sentry.test.DeferredExecutorService import io.sentry.test.ImmediateExecutorService import io.sentry.util.HintUtils import org.mockito.kotlin.any @@ -105,6 +106,18 @@ class AnrIntegrationTest { assertNull(sut.anrWatchDog) } + @Test + fun `when hub is closed right after start, integration is not registered`() { + val deferredExecutorService = DeferredExecutorService() + val sut = fixture.getSut() + fixture.options.executorService = deferredExecutorService + sut.register(fixture.hub, fixture.options) + assertNull(sut.anrWatchDog) + sut.close() + deferredExecutorService.runAll() + assertNull(sut.anrWatchDog) + } + @Test fun `When ANR watch dog is triggered, constructs exception with proper mechanism and snapshot flag`() { val sut = fixture.getSut() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt index 1f6c5090e3..2b6ca801da 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/PhoneStateBreadcrumbsIntegrationTest.kt @@ -7,6 +7,7 @@ import io.sentry.Breadcrumb import io.sentry.IHub import io.sentry.ISentryExecutorService import io.sentry.SentryLevel +import io.sentry.test.DeferredExecutorService import io.sentry.test.ImmediateExecutorService import org.mockito.kotlin.any import org.mockito.kotlin.check @@ -79,6 +80,17 @@ class PhoneStateBreadcrumbsIntegrationTest { assertNull(sut.listener) } + @Test + fun `when hub is closed right after start, integration is not registered`() { + val deferredExecutorService = DeferredExecutorService() + val sut = fixture.getSut(executorService = deferredExecutorService) + sut.register(mock(), fixture.options) + assertNull(sut.listener) + sut.close() + deferredExecutorService.runAll() + assertNull(sut.listener) + } + @Test fun `When on call state received, added breadcrumb with type and category`() { val sut = fixture.getSut() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index dd9dcd8e3d..f8293f9b87 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -6,6 +6,7 @@ import io.sentry.Breadcrumb import io.sentry.IHub import io.sentry.ISentryExecutorService import io.sentry.SentryLevel +import io.sentry.test.DeferredExecutorService import io.sentry.test.ImmediateExecutorService import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull @@ -78,6 +79,17 @@ class SystemEventsBreadcrumbsIntegrationTest { assertNull(sut.receiver) } + @Test + fun `when hub is closed right after start, integration is not registered`() { + val deferredExecutorService = DeferredExecutorService() + val sut = fixture.getSut(executorService = deferredExecutorService) + sut.register(fixture.hub, fixture.options) + assertNull(sut.receiver) + sut.close() + deferredExecutorService.runAll() + assertNull(sut.receiver) + } + @Test fun `When broadcast received, added breadcrumb with type and category`() { val sut = fixture.getSut() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt index 96b5bb912a..d443b1e345 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/TempSensorBreadcrumbsIntegrationTest.kt @@ -11,6 +11,7 @@ import io.sentry.IHub import io.sentry.ISentryExecutorService import io.sentry.SentryLevel import io.sentry.TypeCheckHint +import io.sentry.test.DeferredExecutorService import io.sentry.test.ImmediateExecutorService import io.sentry.test.getDeclaredCtor import io.sentry.test.injectForField @@ -85,6 +86,17 @@ class TempSensorBreadcrumbsIntegrationTest { assertNull(sut.sensorManager) } + @Test + fun `when hub is closed right after start, integration is not registered`() { + val deferredExecutorService = DeferredExecutorService() + val sut = fixture.getSut(executorService = deferredExecutorService) + sut.register(mock(), fixture.options) + assertNull(sut.sensorManager) + sut.close() + deferredExecutorService.runAll() + assertNull(sut.sensorManager) + } + @Test fun `When onSensorChanged received, add a breadcrumb with type and category`() { val sut = fixture.getSut() diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt index a1cea5e3d7..8d66cc7dd9 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventListenerTest.kt @@ -2,13 +2,13 @@ package io.sentry.android.okhttp import io.sentry.BaggageHeader import io.sentry.IHub -import io.sentry.ISentryExecutorService import io.sentry.SentryOptions import io.sentry.SentryTraceHeader import io.sentry.SentryTracer import io.sentry.SpanDataConvention import io.sentry.SpanStatus import io.sentry.TransactionContext +import io.sentry.test.ImmediateExecutorService import okhttp3.Call import okhttp3.EventListener import okhttp3.OkHttpClient @@ -21,14 +21,12 @@ import okhttp3.mockwebserver.MockWebServer import okhttp3.mockwebserver.SocketPolicy import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull -import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.spy import org.mockito.kotlin.verify import org.mockito.kotlin.whenever -import java.util.concurrent.Future import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotNull @@ -334,13 +332,10 @@ class SentryOkHttpEventListenerTest { @Test fun `when response is not closed, root call is trimmed to responseHeadersEnd`() { - val mockExecutor = mock() - val captor = argumentCaptor() - whenever(mockExecutor.schedule(captor.capture(), any())).then { - captor.lastValue.run() - mock>() - } - val sut = fixture.getSut(httpStatusCode = 500, configureOptions = { it.executorService = mockExecutor }) + val sut = fixture.getSut( + httpStatusCode = 500, + configureOptions = { it.executorService = ImmediateExecutorService() } + ) val request = getRequest() val call = sut.newCall(request) val response = spy(call.execute()) diff --git a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt index 9fab3b475c..51cc493d89 100644 --- a/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt +++ b/sentry-android-okhttp/src/test/java/io/sentry/android/okhttp/SentryOkHttpEventTest.kt @@ -23,6 +23,7 @@ import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_BOD import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.RESPONSE_HEADERS_EVENT import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.SECURE_CONNECT_EVENT import io.sentry.exception.SentryHttpClientException +import io.sentry.test.ImmediateExecutorService import io.sentry.test.getProperty import okhttp3.Protocol import okhttp3.Request @@ -31,7 +32,6 @@ import okhttp3.mockwebserver.MockWebServer import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.argThat -import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.check import org.mockito.kotlin.eq import org.mockito.kotlin.mock @@ -39,7 +39,6 @@ import org.mockito.kotlin.never import org.mockito.kotlin.spy import org.mockito.kotlin.verify import org.mockito.kotlin.whenever -import java.util.concurrent.Future import java.util.concurrent.RejectedExecutionException import kotlin.RuntimeException import kotlin.test.Test @@ -534,13 +533,7 @@ class SentryOkHttpEventTest { @Test fun `scheduleFinish schedules finishEvent`() { - val mockExecutor = mock() - val captor = argumentCaptor() - whenever(mockExecutor.schedule(captor.capture(), any())).then { - captor.lastValue.run() - mock>() - } - fixture.hub.options.executorService = mockExecutor + fixture.hub.options.executorService = ImmediateExecutorService() val sut = spy(fixture.getSut()) val timestamp = mock() sut.scheduleFinish(timestamp) diff --git a/sentry-test-support/api/sentry-test-support.api b/sentry-test-support/api/sentry-test-support.api index 25e171c83a..d7e83ad5ac 100644 --- a/sentry-test-support/api/sentry-test-support.api +++ b/sentry-test-support/api/sentry-test-support.api @@ -14,6 +14,7 @@ public final class io/sentry/SkipError : java/lang/Error { public final class io/sentry/test/DeferredExecutorService : io/sentry/ISentryExecutorService { public fun ()V public fun close (J)V + public final fun getScheduledRunnables ()Ljava/util/ArrayList; public fun isClosed ()Z public final fun runAll ()V public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; diff --git a/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt b/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt index 76b43c80b8..8d0466cfaf 100644 --- a/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt +++ b/sentry-test-support/src/main/kotlin/io/sentry/test/Mocks.kt @@ -13,7 +13,10 @@ class ImmediateExecutorService : ISentryExecutorService { } override fun submit(callable: Callable): Future = mock() - override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> = mock() + override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { + runnable.run() + return mock>() + } override fun close(timeoutMillis: Long) {} override fun isClosed(): Boolean = false } @@ -21,9 +24,11 @@ class ImmediateExecutorService : ISentryExecutorService { class DeferredExecutorService : ISentryExecutorService { private val runnables = ArrayList() + val scheduledRunnables = ArrayList() fun runAll() { runnables.forEach { it.run() } + scheduledRunnables.forEach { it.run() } } override fun submit(runnable: Runnable): Future<*> { @@ -32,7 +37,10 @@ class DeferredExecutorService : ISentryExecutorService { } override fun submit(callable: Callable): Future = mock() - override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> = mock() + override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { + scheduledRunnables.add(runnable) + return mock() + } override fun close(timeoutMillis: Long) {} override fun isClosed(): Boolean = false } diff --git a/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt index e132839da1..bc55f6f079 100644 --- a/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt +++ b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt @@ -1,5 +1,6 @@ package io.sentry +import io.sentry.test.DeferredExecutorService import io.sentry.test.getCtor import io.sentry.test.getProperty import io.sentry.test.injectForField @@ -13,9 +14,6 @@ import org.mockito.kotlin.spy import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import java.util.Timer -import java.util.concurrent.Callable -import java.util.concurrent.Future -import java.util.concurrent.FutureTask import java.util.concurrent.RejectedExecutionException import kotlin.test.Test import kotlin.test.assertEquals @@ -38,18 +36,7 @@ class DefaultTransactionPerformanceCollectorTest { val hub: IHub = mock() val options = SentryOptions() var mockTimer: Timer? = null - var lastScheduledRunnable: Runnable? = null - - val mockExecutorService = object : ISentryExecutorService { - override fun submit(runnable: Runnable): Future<*> = mock() - override fun submit(callable: Callable): Future = mock() - override fun schedule(runnable: Runnable, delayMillis: Long): Future<*> { - lastScheduledRunnable = runnable - return FutureTask {} - } - override fun close(timeoutMillis: Long) {} - override fun isClosed() = false - } + val deferredExecutorService = DeferredExecutorService() val mockCpuCollector: ICollector = object : ICollector { override fun setup() {} @@ -62,7 +49,7 @@ class DefaultTransactionPerformanceCollectorTest { whenever(hub.options).thenReturn(options) } - fun getSut(memoryCollector: ICollector? = JavaMemoryCollector(), cpuCollector: ICollector? = mockCpuCollector, executorService: ISentryExecutorService = mockExecutorService): TransactionPerformanceCollector { + fun getSut(memoryCollector: ICollector? = JavaMemoryCollector(), cpuCollector: ICollector? = mockCpuCollector, executorService: ISentryExecutorService = deferredExecutorService): TransactionPerformanceCollector { options.dsn = "https://key@sentry.io/proj" options.executorService = executorService if (cpuCollector != null) { @@ -173,7 +160,7 @@ class DefaultTransactionPerformanceCollectorTest { verify(fixture.mockTimer, never())!!.cancel() // Let the timeout job stop the collector - fixture.lastScheduledRunnable?.run() + fixture.deferredExecutorService.runAll() verify(fixture.mockTimer)!!.cancel() // Data is deleted after the collector times out From fdf20fcc96aee9705b3920673a371355cf92caed Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Mon, 20 Nov 2023 12:50:56 +0100 Subject: [PATCH 4/4] updated test --- .../src/test/java/io/sentry/android/core/SentryAndroidTest.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt index b441c7789a..0918b943b2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt @@ -363,7 +363,6 @@ class SentryAndroidTest { // this is necessary to delay the AnrV2Integration processing to execute the configure // scope block below (otherwise it won't be possible as hub is no-op before .init) it.executorService.submit { - Thread.sleep(2000L) Sentry.configureScope { scope -> // make sure the scope values changed to test that we're still using previously // persisted values for the old ANR events @@ -380,7 +379,7 @@ class SentryAndroidTest { .untilTrue(asserted) // assert that persisted values have changed - options.executorService.close(1000L) // finalizes all enqueued persisting tasks + options.executorService.close(5000L) // finalizes all enqueued persisting tasks assertEquals( "TestActivity", PersistingScopeObserver.read(options, TRANSACTION_FILENAME, String::class.java)