Skip to content

Commit

Permalink
Migrate to Firebase common executors (#6128)
Browse files Browse the repository at this point in the history 
Migrate to Firebase common executors. Use the new `CrashlyticsWorker`
for common and disk write workers.
  • Loading branch information
@mrober
mrober committed Jul 29, 2024
1 parent 705d7a3 commit 96b702f
Show file tree
Hide file tree
Showing 19 changed files with 348 additions and 229 deletions.
1 change: 1 addition & 0 deletions firebase-crashlytics/firebase-crashlytics.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ dependencies {
testImplementation(libs.mockito.core)
testImplementation(libs.robolectric)
testImplementation(libs.truth)
testImplementation(project(":integ-testing"))

androidTestImplementation(libs.androidx.test.core)
androidTestImplementation(libs.androidx.test.runner)
Expand Down
50 changes: 34 additions & 16 deletions ...dTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,10 @@
import com.google.android.gms.tasks.TaskCompletionSource;
import com.google.android.gms.tasks.Tasks;
import com.google.firebase.FirebaseApp;
import com.google.firebase.concurrent.TestOnlyExecutors;
import com.google.firebase.crashlytics.internal.CrashlyticsNativeComponent;
import com.google.firebase.crashlytics.internal.CrashlyticsTestCase;
import com.google.firebase.crashlytics.internal.CrashlyticsWorker;
import com.google.firebase.crashlytics.internal.DevelopmentPlatformProvider;
import com.google.firebase.crashlytics.internal.NativeSessionFileProvider;
import com.google.firebase.crashlytics.internal.analytics.AnalyticsEventLogger;
Expand All @@ -55,13 +57,15 @@
import java.util.TreeSet;
import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;
import org.junit.Test;
import org.mockito.ArgumentCaptor;

public class CrashlyticsControllerTest extends CrashlyticsTestCase {
private static final String GOOGLE_APP_ID = "google:app:id";
private static final String SESSION_ID = "session_id";

private final CrashlyticsWorker commonWorker =
new CrashlyticsWorker(TestOnlyExecutors.background());

private Context testContext;
private IdManager idManager;
private SettingsProvider testSettingsProvider;
Expand Down Expand Up @@ -99,14 +103,19 @@ protected void setUp() throws Exception {
when(testSettingsProvider.getSettingsAsync()).thenReturn(Tasks.forResult(testSettings));
}

@Override
protected void tearDown() throws Exception {
super.tearDown();
commonWorker.await();
}

/** A convenience class for building CrashlyticsController instances for testing. */
private class ControllerBuilder {
private DataCollectionArbiter dataCollectionArbiter;
private CrashlyticsNativeComponent nativeComponent = null;
private AnalyticsEventLogger analyticsEventLogger;
private SessionReportingCoordinator sessionReportingCoordinator;

private CrashlyticsBackgroundWorker backgroundWorker;
private LogFileManager logFileManager = null;

private UserMetadata userMetadata = null;
Expand All @@ -118,8 +127,6 @@ private class ControllerBuilder {
analyticsEventLogger = mock(AnalyticsEventLogger.class);

sessionReportingCoordinator = mockSessionReportingCoordinator;

backgroundWorker = new CrashlyticsBackgroundWorker(new SameThreadExecutorService());
}

ControllerBuilder setDataCollectionArbiter(DataCollectionArbiter arbiter) {
Expand Down Expand Up @@ -168,7 +175,7 @@ public CrashlyticsController build() {
final CrashlyticsController controller =
new CrashlyticsController(
testContext.getApplicationContext(),
backgroundWorker,
commonWorker,
idManager,
dataCollectionArbiter,
testFileStore,
Expand Down Expand Up @@ -208,6 +215,8 @@ public void testWriteNonFatal_callsSessionReportingCoordinatorPersistNonFatal()
controller.writeNonFatalException(thread, nonFatal);
controller.doCloseSessions(testSettingsProvider);

commonWorker.await();

verify(mockSessionReportingCoordinator)
.persistNonFatalEvent(eq(nonFatal), eq(thread), eq(sessionId), anyLong());
}
Expand All @@ -228,9 +237,8 @@ public void testFatalException_callsSessionReportingCoordinatorPersistFatal() th
.persistFatalEvent(eq(fatal), eq(thread), eq(sessionId), anyLong());
}

@Test
@SdkSuppress(minSdkVersion = 30) // ApplicationExitInfo
public void testOnDemandFatal_callLogFatalException() {
public void testOnDemandFatal_callLogFatalException() throws Exception {
Thread thread = Thread.currentThread();
Exception fatal = new RuntimeException("Fatal");
Thread.UncaughtExceptionHandler exceptionHandler = mock(Thread.UncaughtExceptionHandler.class);
Expand All @@ -246,6 +254,8 @@ public void testOnDemandFatal_callLogFatalException() {
controller.enableExceptionHandling(SESSION_ID, exceptionHandler, testSettingsProvider);
controller.logFatalException(thread, fatal);

commonWorker.await();

verify(mockUserMetadata).setNewSession(not(eq(SESSION_ID)));
}

Expand Down Expand Up @@ -323,17 +333,20 @@ public File getOsFile() {
final CrashlyticsController controller =
builder().setNativeComponent(mockNativeComponent).setLogFileManager(logFileManager).build();

controller.finalizeSessions(testSettingsProvider);
commonWorker.submit(() -> controller.finalizeSessions(testSettingsProvider));
commonWorker.await();

verify(mockSessionReportingCoordinator)
.finalizeSessionWithNativeEvent(eq(previousSessionId), any(), any());
verify(mockSessionReportingCoordinator, never())
.finalizeSessionWithNativeEvent(eq(sessionId), any(), any());
}

@SdkSuppress(minSdkVersion = 30) // ApplicationExitInfo
public void testMissingNativeComponentCausesNoReports() {
public void testMissingNativeComponentCausesNoReports() throws Exception {
final CrashlyticsController controller = createController();
controller.finalizeSessions(testSettingsProvider);
commonWorker.submit(() -> controller.finalizeSessions(testSettingsProvider));
commonWorker.await();

List<String> sessions = testFileStore.getAllOpenSessionIds();
for (String sessionId : sessions) {
Expand Down Expand Up @@ -384,7 +397,8 @@ public void testFinalizeSessionAfterCrashOk() throws Exception {
testSettingsProvider, Thread.currentThread(), new RuntimeException());

// This should not throw.
controller.finalizeSessions(testSettingsProvider);
commonWorker.submit(() -> controller.finalizeSessions(testSettingsProvider));
commonWorker.await();
}

@SdkSuppress(minSdkVersion = 30) // ApplicationExitInfo
Expand Down Expand Up @@ -535,7 +549,7 @@ public void testUploadDisabledThenEnabled() throws Exception {
}

@SdkSuppress(minSdkVersion = 30) // ApplicationExitInfo
public void testFatalEvent_sendsAppExceptionEvent() {
public void testFatalEvent_sendsAppExceptionEvent() throws Exception {
final String sessionId = "sessionId";
final LogFileManager logFileManager = new LogFileManager(testFileStore);
final AnalyticsEventLogger mockFirebaseAnalyticsLogger = mock(AnalyticsEventLogger.class);
Expand All @@ -548,10 +562,14 @@ public void testFatalEvent_sendsAppExceptionEvent() {
when(mockSessionReportingCoordinator.listSortedOpenSessionIds())
.thenReturn(new TreeSet<>(Collections.singleton(sessionId)));

controller.openSession(SESSION_ID);
controller.handleUncaughtException(
testSettingsProvider, Thread.currentThread(), new RuntimeException("Fatal"));
controller.finalizeSessions(testSettingsProvider);
commonWorker.submit(
() -> {
controller.openSession(SESSION_ID);
controller.handleUncaughtException(
testSettingsProvider, Thread.currentThread(), new RuntimeException("Fatal"));
controller.finalizeSessions(testSettingsProvider);
});
commonWorker.await();

assertFirebaseAnalyticsCrashEvent(mockFirebaseAnalyticsLogger);
}
Expand Down
14 changes: 9 additions & 5 deletions ...va/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreInitializationTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@
import com.google.android.gms.tasks.Tasks;
import com.google.firebase.FirebaseApp;
import com.google.firebase.FirebaseOptions;
import com.google.firebase.concurrent.TestOnlyExecutors;
import com.google.firebase.crashlytics.internal.CrashlyticsNativeComponent;
import com.google.firebase.crashlytics.internal.CrashlyticsNativeComponentDeferredProxy;
import com.google.firebase.crashlytics.internal.CrashlyticsTestCase;
import com.google.firebase.crashlytics.internal.CrashlyticsWorker;
import com.google.firebase.crashlytics.internal.DevelopmentPlatformProvider;
import com.google.firebase.crashlytics.internal.RemoteConfigDeferredProxy;
import com.google.firebase.crashlytics.internal.analytics.UnavailableAnalyticsEventLogger;
Expand All @@ -44,7 +46,6 @@
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ExecutorService;

public class CrashlyticsCoreInitializationTest extends CrashlyticsTestCase {

Expand Down Expand Up @@ -89,8 +90,9 @@ private static final class CoreBuilder {
private IdManager idManager;
private CrashlyticsNativeComponent nativeComponent;
private DataCollectionArbiter arbiter;
private ExecutorService crashHandlerExecutor;
private FileStore fileStore;
private CrashlyticsWorker commonWorker;
private CrashlyticsWorker diskWriteWorker;

public CoreBuilder(Context context, FirebaseOptions firebaseOptions) {
app = mock(FirebaseApp.class);
Expand Down Expand Up @@ -119,7 +121,8 @@ public void whenAvailable(
arbiter = mock(DataCollectionArbiter.class);
when(arbiter.isAutomaticDataCollectionEnabled()).thenReturn(true);

crashHandlerExecutor = new SameThreadExecutorService();
commonWorker = new CrashlyticsWorker(TestOnlyExecutors.background());
diskWriteWorker = new CrashlyticsWorker(TestOnlyExecutors.background());
fileStore = new FileStore(context);
}

Expand All @@ -142,9 +145,10 @@ public CrashlyticsCore build() {
new DisabledBreadcrumbSource(),
new UnavailableAnalyticsEventLogger(),
fileStore,
crashHandlerExecutor,
mock(CrashlyticsAppQualitySessionsSubscriber.class),
mock(RemoteConfigDeferredProxy.class));
mock(RemoteConfigDeferredProxy.class),
commonWorker,
diskWriteWorker);
}
}

Expand Down
7 changes: 5 additions & 2 deletions ...androidTest/java/com/google/firebase/crashlytics/internal/common/CrashlyticsCoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@
import com.google.android.gms.tasks.Tasks;
import com.google.firebase.FirebaseApp;
import com.google.firebase.FirebaseOptions;
import com.google.firebase.concurrent.TestOnlyExecutors;
import com.google.firebase.crashlytics.BuildConfig;
import com.google.firebase.crashlytics.internal.CrashlyticsNativeComponent;
import com.google.firebase.crashlytics.internal.CrashlyticsNativeComponentDeferredProxy;
import com.google.firebase.crashlytics.internal.CrashlyticsTestCase;
import com.google.firebase.crashlytics.internal.CrashlyticsWorker;
import com.google.firebase.crashlytics.internal.DevelopmentPlatformProvider;
import com.google.firebase.crashlytics.internal.RemoteConfigDeferredProxy;
import com.google.firebase.crashlytics.internal.analytics.UnavailableAnalyticsEventLogger;
Expand Down Expand Up @@ -427,9 +429,10 @@ CrashlyticsCore build(Context context) {
breadcrumbSource,
new UnavailableAnalyticsEventLogger(),
new FileStore(context),
new SameThreadExecutorService(),
mock(CrashlyticsAppQualitySessionsSubscriber.class),
mock(RemoteConfigDeferredProxy.class));
mock(RemoteConfigDeferredProxy.class),
new CrashlyticsWorker(TestOnlyExecutors.background()),
new CrashlyticsWorker(TestOnlyExecutors.background()));
return crashlyticsCore;
}
}
Expand Down
Loading

0 comments on commit 96b702f

Please sign in to comment.