Clean up tests

Author: mrober

firebase-sessions/src/test/kotlin/com/google/firebase/sessions/settings/RemoteSettingsTest.kt

@@ -19,10 +19,7 @@ package com.google.firebase.sessions.settings
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.google.common.truth.Truth.assertThat
 import com.google.firebase.FirebaseApp
-import com.google.firebase.installations.FirebaseInstallationsApi
-import com.google.firebase.sessions.ApplicationInfo
 import com.google.firebase.sessions.SessionEvents
-import com.google.firebase.sessions.TimeProvider
 import com.google.firebase.sessions.testing.FakeFirebaseApp
 import com.google.firebase.sessions.testing.FakeFirebaseInstallations
 import com.google.firebase.sessions.testing.FakeRemoteConfigFetcher
@@ -31,55 +28,46 @@ import com.google.firebase.sessions.testing.FakeTimeProvider
 import kotlin.time.Duration.Companion.minutes
 import kotlin.time.Duration.Companion.seconds
 import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.test.UnconfinedTestDispatcher
-import kotlinx.coroutines.test.runCurrent
 import kotlinx.coroutines.test.runTest
 import kotlinx.coroutines.withTimeout
 import org.json.JSONObject
 import org.junit.After
 import org.junit.Test
 import org.junit.runner.RunWith
 
-@OptIn(ExperimentalCoroutinesApi::class)
 @RunWith(AndroidJUnit4::class)
 class RemoteSettingsTest {
 
   @Test
-  fun remoteSettings_successfulFetchCachesValues() =
-    runTest(UnconfinedTestDispatcher()) {
-      val firebaseApp = FakeFirebaseApp().firebaseApp
-      val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
-      val fakeFetcher = FakeRemoteConfigFetcher()
-
-      val remoteSettings =
-        buildRemoteSettings(
-          FakeTimeProvider(),
-          firebaseInstallations,
-          SessionEvents.getApplicationInfo(firebaseApp),
-          fakeFetcher,
-          FakeSettingsCache(),
-        )
-
-      runCurrent()
+  fun remoteSettings_successfulFetchCachesValues() = runTest {
+    val firebaseApp = FakeFirebaseApp().firebaseApp
+    val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
+    val fakeFetcher = FakeRemoteConfigFetcher()
 
-      assertThat(remoteSettings.sessionEnabled).isNull()
-      assertThat(remoteSettings.samplingRate).isNull()
-      assertThat(remoteSettings.sessionRestartTimeout).isNull()
+    val remoteSettings =
+      RemoteSettings(
+        FakeTimeProvider(),
+        firebaseInstallations,
+        SessionEvents.getApplicationInfo(firebaseApp),
+        fakeFetcher,
+        FakeSettingsCache(),
+      )
 
-      fakeFetcher.responseJSONObject = JSONObject(VALID_RESPONSE)
-      remoteSettings.updateSettings()
+    assertThat(remoteSettings.sessionEnabled).isNull()
+    assertThat(remoteSettings.samplingRate).isNull()
+    assertThat(remoteSettings.sessionRestartTimeout).isNull()
 
-      runCurrent()
+    fakeFetcher.responseJSONObject = JSONObject(VALID_RESPONSE)
+    remoteSettings.updateSettings()
 
-      assertThat(remoteSettings.sessionEnabled).isFalse()
-      assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
-      assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
+    assertThat(remoteSettings.sessionEnabled).isFalse()
+    assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
+    assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
 
-      remoteSettings.clearCachedSettings()
-    }
+    remoteSettings.clearCachedSettings()
+  }
 
   @Test
   fun remoteSettings_successfulFetchWithLessConfigsCachesOnlyReceivedValues() = runTest {
@@ -88,16 +76,14 @@ class RemoteSettingsTest {
     val fakeFetcher = FakeRemoteConfigFetcher()
 
     val remoteSettings =
-      buildRemoteSettings(
+      RemoteSettings(
         FakeTimeProvider(),
         firebaseInstallations,
         SessionEvents.getApplicationInfo(firebaseApp),
         fakeFetcher,
         FakeSettingsCache(),
       )
 
-    runCurrent()
-
     assertThat(remoteSettings.sessionEnabled).isNull()
     assertThat(remoteSettings.samplingRate).isNull()
     assertThat(remoteSettings.sessionRestartTimeout).isNull()
@@ -107,8 +93,6 @@ class RemoteSettingsTest {
     fakeFetcher.responseJSONObject = fetchedResponse
     remoteSettings.updateSettings()
 
-    runCurrent()
-
     assertThat(remoteSettings.sessionEnabled).isNull()
     assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
     assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -124,7 +108,7 @@ class RemoteSettingsTest {
     val fakeTimeProvider = FakeTimeProvider()
 
     val remoteSettings =
-      buildRemoteSettings(
+      RemoteSettings(
         fakeTimeProvider,
         firebaseInstallations,
         SessionEvents.getApplicationInfo(firebaseApp),
@@ -137,8 +121,6 @@ class RemoteSettingsTest {
     fakeFetcher.responseJSONObject = fetchedResponse
     remoteSettings.updateSettings()
 
-    runCurrent()
-
     assertThat(remoteSettings.sessionEnabled).isFalse()
     assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
     assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
@@ -152,8 +134,6 @@ class RemoteSettingsTest {
     fakeFetcher.responseJSONObject = fetchedResponse
     remoteSettings.updateSettings()
 
-    runCurrent()
-
     assertThat(remoteSettings.sessionEnabled).isTrue()
     assertThat(remoteSettings.samplingRate).isEqualTo(0.25)
     assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(20.minutes)
@@ -162,110 +142,99 @@ class RemoteSettingsTest {
   }
 
   @Test
-  fun remoteSettings_successfulFetchWithEmptyConfigRetainsOldConfigs() =
-    runTest(UnconfinedTestDispatcher()) {
-      val firebaseApp = FakeFirebaseApp().firebaseApp
-      val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
-      val fakeFetcher = FakeRemoteConfigFetcher()
-      val fakeTimeProvider = FakeTimeProvider()
-
-      val remoteSettings =
-        buildRemoteSettings(
-          fakeTimeProvider,
-          firebaseInstallations,
-          SessionEvents.getApplicationInfo(firebaseApp),
-          fakeFetcher,
-          FakeSettingsCache(),
-        )
-
-      val fetchedResponse = JSONObject(VALID_RESPONSE)
-      fetchedResponse.getJSONObject("app_quality").put("cache_duration", 1)
-      fakeFetcher.responseJSONObject = fetchedResponse
-      remoteSettings.updateSettings()
-
-      runCurrent()
-      fakeTimeProvider.addInterval(31.seconds)
-
-      assertThat(remoteSettings.sessionEnabled).isFalse()
-      assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
-      assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
-
-      fetchedResponse.remove("app_quality")
-
-      // Sleep for a second before updating configs
-      Thread.sleep(2000)
-
-      fakeFetcher.responseJSONObject = fetchedResponse
-      remoteSettings.updateSettings()
-
-      runCurrent()
-      Thread.sleep(30)
-
-      assertThat(remoteSettings.sessionEnabled).isFalse()
-      assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
-      assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
-
-      remoteSettings.clearCachedSettings()
-    }
+  fun remoteSettings_successfulFetchWithEmptyConfigRetainsOldConfigs() = runTest {
+    val firebaseApp = FakeFirebaseApp().firebaseApp
+    val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
+    val fakeFetcher = FakeRemoteConfigFetcher()
+    val fakeTimeProvider = FakeTimeProvider()
+
+    val remoteSettings =
+      RemoteSettings(
+        fakeTimeProvider,
+        firebaseInstallations,
+        SessionEvents.getApplicationInfo(firebaseApp),
+        fakeFetcher,
+        FakeSettingsCache(),
+      )
+
+    val fetchedResponse = JSONObject(VALID_RESPONSE)
+    fetchedResponse.getJSONObject("app_quality").put("cache_duration", 1)
+    fakeFetcher.responseJSONObject = fetchedResponse
+    remoteSettings.updateSettings()
+
+    fakeTimeProvider.addInterval(31.seconds)
+
+    assertThat(remoteSettings.sessionEnabled).isFalse()
+    assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
+    assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
+
+    fetchedResponse.remove("app_quality")
+
+    fakeFetcher.responseJSONObject = fetchedResponse
+    remoteSettings.updateSettings()
+
+    assertThat(remoteSettings.sessionEnabled).isFalse()
+    assertThat(remoteSettings.samplingRate).isEqualTo(0.75)
+    assertThat(remoteSettings.sessionRestartTimeout).isEqualTo(40.minutes)
+
+    remoteSettings.clearCachedSettings()
+  }
 
   @Test
-  fun remoteSettings_fetchWhileFetchInProgress() =
-    runTest(UnconfinedTestDispatcher()) {
-      // This test does:
-      // 1. Do a fetch with a fake fetcher that will block for 3 seconds.
-      // 2. While that is happening, do a second fetch.
-      //    - First fetch is still fetching, so second fetch should fall through to the mutex.
-      //    - Second fetch will be blocked until first completes.
-      //    - First fetch returns, should unblock the second fetch.
-      //    - Second fetch should go into mutex, sees cache is valid in "double check," exist early.
-      // 3. After a fetch completes, do a third fetch.
-      //    - First fetch should have have updated the cache.
-      //    - Third fetch should exit even earlier, never having gone into the mutex.
-
-      val firebaseApp = FakeFirebaseApp().firebaseApp
-      val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
-      val fakeFetcherWithDelay =
-        FakeRemoteConfigFetcher(JSONObject(VALID_RESPONSE), networkDelay = 3.seconds)
-
-      fakeFetcherWithDelay.responseJSONObject
-        .getJSONObject("app_quality")
-        .put("sampling_rate", 0.125)
-
-      val remoteSettingsWithDelay =
-        buildRemoteSettings(
-          FakeTimeProvider(),
-          firebaseInstallations,
-          SessionEvents.getApplicationInfo(firebaseApp),
-          configsFetcher = fakeFetcherWithDelay,
-          FakeSettingsCache(),
-        )
-
-      // Do the first fetch. This one should fetched the configsFetcher.
-      val firstFetch = launch(Dispatchers.Default) { remoteSettingsWithDelay.updateSettings() }
-
-      // Wait a second, and then do the second fetch while first is still running.
-      // This one should block until the first fetch completes, but then exit early.
-      launch(Dispatchers.Default) {
-        delay(1.seconds)
-        remoteSettingsWithDelay.updateSettings()
-      }
+  fun remoteSettings_fetchWhileFetchInProgress() = runTest {
+    // This test does:
+    // 1. Do a fetch with a fake fetcher that will block for 3 seconds.
+    // 2. While that is happening, do a second fetch.
+    //    - First fetch is still fetching, so second fetch should fall through to the mutex.
+    //    - Second fetch will be blocked until first completes.
+    //    - First fetch returns, should unblock the second fetch.
+    //    - Second fetch should go into mutex, sees cache is valid in "double check," exist early.
+    // 3. After a fetch completes, do a third fetch.
+    //    - First fetch should have have updated the cache.
+    //    - Third fetch should exit even earlier, never having gone into the mutex.
+
+    val firebaseApp = FakeFirebaseApp().firebaseApp
+    val firebaseInstallations = FakeFirebaseInstallations("FaKeFiD")
+    val fakeFetcherWithDelay =
+      FakeRemoteConfigFetcher(JSONObject(VALID_RESPONSE), networkDelay = 3.seconds)
+
+    fakeFetcherWithDelay.responseJSONObject.getJSONObject("app_quality").put("sampling_rate", 0.125)
+
+    val remoteSettingsWithDelay =
+      RemoteSettings(
+        FakeTimeProvider(),
+        firebaseInstallations,
+        SessionEvents.getApplicationInfo(firebaseApp),
+        configsFetcher = fakeFetcherWithDelay,
+        FakeSettingsCache(),
+      )
 
-      // Wait until the first fetch is done, then do a third fetch.
-      // This one should not even block, and exit early.
-      firstFetch.join()
-      withTimeout(1.seconds) { remoteSettingsWithDelay.updateSettings() }
+    // Do the first fetch. This one should fetched the configsFetcher.
+    val firstFetch = launch(Dispatchers.Default) { remoteSettingsWithDelay.updateSettings() }
 
-      // Assert that the configsFetcher was fetched exactly once.
-      assertThat(fakeFetcherWithDelay.timesCalled).isEqualTo(1)
-      assertThat(remoteSettingsWithDelay.samplingRate).isEqualTo(0.125)
+    // Wait a second, and then do the second fetch while first is still running.
+    // This one should block until the first fetch completes, but then exit early.
+    launch(Dispatchers.Default) {
+      delay(1.seconds)
+      remoteSettingsWithDelay.updateSettings()
     }
 
+    // Wait until the first fetch is done, then do a third fetch.
+    // This one should not even block, and exit early.
+    firstFetch.join()
+    withTimeout(1.seconds) { remoteSettingsWithDelay.updateSettings() }
+
+    // Assert that the configsFetcher was fetched exactly once.
+    assertThat(fakeFetcherWithDelay.timesCalled).isEqualTo(1)
+    assertThat(remoteSettingsWithDelay.samplingRate).isEqualTo(0.125)
+  }
+
   @After
   fun cleanUp() {
     FirebaseApp.clearInstancesForTest()
   }
 
-  internal companion object {
+  private companion object {
     const val VALID_RESPONSE =
       """
       {
@@ -284,30 +253,5 @@ class RemoteSettingsTest {
         }
       }
       """
-
-    /**
-     * Build an instance of [RemoteSettings] using the Dagger factory.
-     *
-     * This is needed because the SDK vendors Dagger to a difference namespace, but it does not for
-     * these unit tests. The [RemoteSettings.lazySettingsCache] has type [dagger.Lazy] in these
-     * tests, but type `com.google.firebase.sessions.dagger.Lazy` in the SDK. This method to build
-     * the instance is the easiest I could find that does not need any reference to [dagger.Lazy] in
-     * the test code.
-     */
-    fun buildRemoteSettings(
-      timeProvider: TimeProvider,
-      firebaseInstallationsApi: FirebaseInstallationsApi,
-      appInfo: ApplicationInfo,
-      configsFetcher: CrashlyticsSettingsFetcher,
-      settingsCache: SettingsCache,
-    ): RemoteSettings =
-      RemoteSettings_Factory.create(
-          { timeProvider },
-          { firebaseInstallationsApi },
-          { appInfo },
-          { configsFetcher },
-          { settingsCache },
-        )
-        .get()
   }
 }