fix: Flaky tests (#212)

Fixes #207
relaycorp · Oct 2, 2020 · f06b87d · f06b87d
1 parent 5a93efa
commit f06b87d
Show file tree

Hide file tree

Showing 11 changed files with 66 additions and 49 deletions.
diff --git a/app/src/androidTest/java/tech/relaycorp/courier/data/preference/StoragePreferencesTest.kt b/app/src/androidTest/java/tech/relaycorp/courier/data/preference/StoragePreferencesTest.kt
@@ -1,32 +1,42 @@
 package tech.relaycorp.courier.data.preference
 
-import kotlinx.coroutines.test.runBlockingTest
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.runBlocking
 import org.junit.After
-import org.junit.Ignore
+import org.junit.Assert.assertEquals
+import org.junit.Before
 import org.junit.Test
 import tech.relaycorp.courier.data.model.StorageSize
 import tech.relaycorp.courier.test.AppTestProvider.flowSharedPreferences
-import tech.relaycorp.courier.test.test
 import javax.inject.Provider
 
 class StoragePreferencesTest {
 
     private val preferences = StoragePreferences(Provider { flowSharedPreferences })
 
+    @Before
+    fun setUp() {
+        flowSharedPreferences.clear()
+    }
+
     @After
     fun tearDown() {
         flowSharedPreferences.clear()
     }
 
     @Test
-    @Ignore("Failing intermittently") // TODO: fix test
-    fun maxStorage() = runBlockingTest {
-        val observer = preferences.getMaxStorageSize().test(this)
+    fun maxStorage() = runBlocking {
+        assertEquals(
+            StoragePreferences.DEFAULT_MAX_STORAGE_SIZE,
+            preferences.getMaxStorageSize().first()
+        )
+
         val newSize = StorageSize(100)
         preferences.setMaxStorageSize(newSize)
 
-        observer
-            .assertValues(StoragePreferences.DEFAULT_MAX_STORAGE_SIZE, newSize)
-            .finish()
+        assertEquals(
+            newSize,
+            preferences.getMaxStorageSize().first()
+        )
     }
 }
diff --git a/app/src/androidTest/java/tech/relaycorp/courier/domain/PrivateSyncTest.kt b/app/src/androidTest/java/tech/relaycorp/courier/domain/PrivateSyncTest.kt
@@ -51,6 +51,7 @@ class PrivateSyncTest {
                 InetSocketAddress(gatewayIpAddress, 21473),
                 PrivateSubnetTrustManager.INSTANCE
             )
+            .hostnameVerifier { _, _ -> true }
             .build()
     }
 

diff --git a/app/src/androidTest/java/tech/relaycorp/courier/ui/settings/SettingsActivityTest.kt b/app/src/androidTest/java/tech/relaycorp/courier/ui/settings/SettingsActivityTest.kt
@@ -7,7 +7,6 @@ import com.schibsted.spain.barista.assertion.BaristaVisibilityAssertions.assertC
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.runBlocking
 import org.junit.Before
-import org.junit.Ignore
 import org.junit.Rule
 import org.junit.Test
 import tech.relaycorp.courier.BuildConfig
@@ -60,11 +59,12 @@ class SettingsActivityTest {
     }
 
     @Test
-    @Ignore("Failing intermittently") // TODO: fix test
     fun setMaxStorageToMinimum() {
         val activity = testRule.start()
         val slider = activity.findViewById<Slider>(R.id.storageMaxSlider)
-        slider.value = slider.valueFrom
+        slider.postDelayed({
+            slider.value = slider.valueFrom
+        }, 500)
         runBlocking {
             waitForAssertEquals(
                 SettingsViewModel.MIN_STORAGE_SIZE,

diff --git a/app/src/main/java/tech/relaycorp/cogrpc/server/Networking.kt b/app/src/main/java/tech/relaycorp/cogrpc/server/Networking.kt
@@ -33,7 +33,9 @@ internal object Networking {
         val networkInterfaces = NetworkInterface.getNetworkInterfaces().iterator().asSequence()
         val localAddresses = networkInterfaces.flatMap {
             it.inetAddresses.asSequence()
-                .filter { inetAddress -> inetAddress.isSiteLocalAddress && !inetAddress.hostAddress.contains(":") }
+                .filter { inetAddress ->
+                    inetAddress.isSiteLocalAddress && !inetAddress.hostAddress.contains(":")
+                }
                 .map { inetAddress -> inetAddress.hostAddress }
         }
         return localAddresses.toList()

diff --git a/app/src/main/java/tech/relaycorp/courier/ui/BaseViewModel.kt b/app/src/main/java/tech/relaycorp/courier/ui/BaseViewModel.kt
@@ -1,10 +1,16 @@
 package tech.relaycorp.courier.ui
 
+import androidx.annotation.VisibleForTesting
 import androidx.lifecycle.ViewModel
 import androidx.lifecycle.viewModelScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.plus
 
 abstract class BaseViewModel : ViewModel() {
-    val ioScope = viewModelScope + Dispatchers.IO
+    protected val scope = viewModelScope + VIEW_MODEL_DISPATCHER
+
+    companion object {
+        @VisibleForTesting
+        var VIEW_MODEL_DISPATCHER = Dispatchers.IO
+    }
 }
diff --git a/app/src/main/java/tech/relaycorp/courier/ui/main/MainViewModel.kt b/app/src/main/java/tech/relaycorp/courier/ui/main/MainViewModel.kt
@@ -54,7 +54,7 @@ class MainViewModel
             }
         }
             .onEach(syncPeopleState::send)
-            .launchIn(ioScope)
+            .launchIn(scope)
 
         combine(
             internetConnectionObserver.observe(),
@@ -72,17 +72,17 @@ class MainViewModel
             }
         }
             .onEach(syncInternetState::send)
-            .launchIn(ioScope)
+            .launchIn(scope)
 
         getStorageUsage
             .observe()
             .onEach {
                 storageUsage.send(it)
                 lowStorageMessageIsVisible.send(it.isLowOnSpace)
             }
-            .launchIn(ioScope)
+            .launchIn(scope)
 
-        ioScope.launch {
+        scope.launch {
             val messagesDeleted = deleteExpiredMessages.delete()
             if (messagesDeleted.any()) {
                 expiredMessagesDeleted.send(messagesDeleted.sumMessageSize())

diff --git a/app/src/main/java/tech/relaycorp/courier/ui/settings/SettingsViewModel.kt b/app/src/main/java/tech/relaycorp/courier/ui/settings/SettingsViewModel.kt
@@ -56,7 +56,7 @@ class SettingsViewModel
             .onEach {
                 deleteDataEnabled.send((!it.usedByApp.isZero).toEnableState())
             }
-            .launchIn(ioScope)
+            .launchIn(scope)
 
         combine(
             getStorageUsage.observe(),
@@ -70,24 +70,24 @@ class SettingsViewModel
             )
         }
             .onEach(storageStats::send)
-            .launchIn(ioScope)
+            .launchIn(scope)
 
         deleteDataClicks
             .asFlow()
             .onEach { deleteAllStorage.delete() }
-            .launchIn(ioScope)
+            .launchIn(scope)
 
         storagePreferences
             .getMaxStorageSize()
             .onEach(maxStorage::send)
-            .launchIn(ioScope)
+            .launchIn(scope)
 
         maxStorageChanged
             .asFlow()
             .onEach { storagePreferences.setMaxStorageSize(it) }
-            .launchIn(ioScope)
+            .launchIn(scope)
 
-        ioScope.launch {
+        scope.launch {
             val totalStorageValue = diskStats.getTotalStorage()
             maxStorageBoundary.send(
                 SizeBoundary(

diff --git a/app/src/main/java/tech/relaycorp/courier/ui/sync/internet/InternetSyncViewModel.kt b/app/src/main/java/tech/relaycorp/courier/ui/sync/internet/InternetSyncViewModel.kt
@@ -32,15 +32,15 @@ class InternetSyncViewModel
     val finish get() = finishChannel.asFlow()
 
     init {
-        val syncJob = ioScope.launch {
+        val syncJob = scope.launch {
             publicSync.sync()
         }
 
         val syncStateJob =
             publicSync
                 .state()
                 .onEach { stateChannel.send(it) }
-                .launchIn(ioScope)
+                .launchIn(scope)
 
         stopClicks
             .asFlow()
@@ -49,6 +49,6 @@ class InternetSyncViewModel
                 syncJob.cancel()
                 finishChannel.send(Finish)
             }
-            .launchIn(ioScope)
+            .launchIn(scope)
     }
 }
diff --git a/app/src/main/java/tech/relaycorp/courier/ui/sync/people/PeopleSyncViewModel.kt b/app/src/main/java/tech/relaycorp/courier/ui/sync/people/PeopleSyncViewModel.kt
@@ -61,7 +61,7 @@ class PeopleSyncViewModel
                     }
                 }
             }
-            .launchIn(ioScope)
+            .launchIn(scope)
 
         wifiHotspotStateReceiver
             .state()
@@ -72,14 +72,14 @@ class PeopleSyncViewModel
                 openHotspotInstructions.send(Unit)
                 finish.send(Finish)
             }
-            .launchIn(ioScope)
+            .launchIn(scope)
 
         privateSync
             .clientsConnected()
             .filter { it > 0 }
             .take(1)
             .onEach { hadFirstClient = true }
-            .launchIn(ioScope)
+            .launchIn(scope)
 
         combine(
             privateSync.state(),
@@ -98,7 +98,7 @@ class PeopleSyncViewModel
                 PrivateSync.State.Error -> state.send(State.Error)
             }
         }
-            .launchIn(ioScope)
+            .launchIn(scope)
 
         stopClicks
             .asFlow()
@@ -113,12 +113,12 @@ class PeopleSyncViewModel
                     finish.send(Finish)
                 }
             }
-            .launchIn(ioScope)
+            .launchIn(scope)
 
         confirmStopClicks
             .asFlow()
             .onEach { finish.send(Finish) }
-            .launchIn(ioScope)
+            .launchIn(scope)
     }
 
     override fun onCleared() {

diff --git a/app/src/test/java/tech/relaycorp/courier/ui/settings/SettingsViewModelTest.kt b/app/src/test/java/tech/relaycorp/courier/ui/settings/SettingsViewModelTest.kt
@@ -4,13 +4,12 @@ import com.nhaarman.mockitokotlin2.eq
 import com.nhaarman.mockitokotlin2.mock
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.whenever
-import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.emptyFlow
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.flowOf
-import kotlinx.coroutines.runBlocking
+import kotlinx.coroutines.test.TestCoroutineDispatcher
+import kotlinx.coroutines.test.runBlockingTest
 import org.junit.jupiter.api.BeforeEach
-import org.junit.jupiter.api.Disabled
 import org.junit.jupiter.api.Test
 import tech.relaycorp.courier.data.disk.DiskStats
 import tech.relaycorp.courier.data.model.StorageSize
@@ -20,6 +19,7 @@ import tech.relaycorp.courier.domain.DeleteAllStorage
 import tech.relaycorp.courier.domain.GetStorageUsage
 import tech.relaycorp.courier.test.WaitAssertions.waitForAssertEquals
 import tech.relaycorp.courier.test.factory.StorageUsageFactory
+import tech.relaycorp.courier.ui.BaseViewModel
 import tech.relaycorp.courier.ui.common.EnableState
 
 internal class SettingsViewModelTest {
@@ -33,21 +33,21 @@ internal class SettingsViewModelTest {
 
     @BeforeEach
     internal fun setUp() {
+        BaseViewModel.VIEW_MODEL_DISPATCHER = TestCoroutineDispatcher()
         whenever(observeStorageUsage.observe()).thenReturn(emptyFlow())
         whenever(storagePreferences.getMaxStorageSize()).thenReturn(emptyFlow())
         whenever(diskStats.observeAvailableStorage()).thenReturn(emptyFlow())
     }
 
     @Test
-    @Disabled("Failing intermittently") // TODO: fix test
-    fun `deleteData clicked called right domain method`() = runBlocking(Dispatchers.IO) {
-        assert(false)
-        buildViewModel().deleteDataClicked()
+    fun `deleteData clicked called right domain method`() = runBlockingTest {
+        val vm = buildViewModel()
+        vm.deleteDataClicked()
         verify(deleteAllStorage).delete()
     }
 
     @Test
-    fun `deleteData is disabled when data is empty`() = runBlocking(Dispatchers.IO) {
+    fun `deleteData is disabled when data is empty`() = runBlockingTest {
         val emptyUsage = StorageUsageFactory.build().copy(usedByApp = StorageSize(0L))
         whenever(observeStorageUsage.observe()).thenReturn(flowOf(emptyUsage))
 
@@ -59,7 +59,7 @@ internal class SettingsViewModelTest {
     }
 
     @Test
-    fun `deleteData is enabled when there's data`() = runBlocking(Dispatchers.IO) {
+    fun `deleteData is enabled when there's data`() = runBlockingTest {
         val emptyUsage = StorageUsageFactory.build().copy(usedByApp = StorageSize(1L))
         whenever(observeStorageUsage.observe()).thenReturn(flowOf(emptyUsage))
 
@@ -71,7 +71,7 @@ internal class SettingsViewModelTest {
     }
 
     @Test
-    fun `observe storage stats`() = runBlocking(Dispatchers.IO) {
+    fun `observe storage stats`() = runBlockingTest {
         val total = StorageSize(1_000_000)
         val available = StorageSize(500_000)
         val used = StorageSize(100_000)
@@ -88,7 +88,7 @@ internal class SettingsViewModelTest {
     }
 
     @Test
-    fun `max storage boundary`() = runBlocking(Dispatchers.IO) {
+    fun `max storage boundary`() = runBlockingTest {
         val totalStorage = SettingsViewModel.MIN_STORAGE_SIZE * 10
         whenever(diskStats.getTotalStorage()).thenReturn(totalStorage)
 
@@ -104,11 +104,9 @@ internal class SettingsViewModelTest {
     }
 
     @Test
-    internal fun `max storage value changed`() = runBlocking {
+    internal fun `max storage value changed`() = runBlockingTest {
         val newValue = StorageSize(1_000_000_00)
-        runBlocking(Dispatchers.IO) {
-            buildViewModel().maxStorageChanged(newValue)
-        }
+        buildViewModel().maxStorageChanged(newValue)
         verify(storagePreferences).setMaxStorageSize(eq(newValue))
     }
 

diff --git a/build.gradle b/build.gradle
@@ -3,7 +3,7 @@
 buildscript {
     ext {
         kotlinVersion = '1.4.10'
-        kotlinCoroutinesVersion = '1.3.8'
+        kotlinCoroutinesVersion = '1.3.9'
         grpcVersion = '1.32.1'
         protobufVersion = '3.12.2'
         protobufGradleVersion = '0.8.12'