From f06b87d9a6ed8c27c69dfb051dd1cb91bbc0595d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9rgio=20Santos?= Date: Fri, 2 Oct 2020 14:44:49 +0100 Subject: [PATCH] fix: Flaky tests (#212) Fixes #207 --- .../data/preference/StoragePreferencesTest.kt | 28 +++++++++++++------ .../courier/domain/PrivateSyncTest.kt | 1 + .../ui/settings/SettingsActivityTest.kt | 6 ++-- .../relaycorp/cogrpc/server/Networking.kt | 4 ++- .../relaycorp/courier/ui/BaseViewModel.kt | 8 +++++- .../courier/ui/main/MainViewModel.kt | 8 +++--- .../courier/ui/settings/SettingsViewModel.kt | 12 ++++---- .../ui/sync/internet/InternetSyncViewModel.kt | 6 ++-- .../ui/sync/people/PeopleSyncViewModel.kt | 12 ++++---- .../ui/settings/SettingsViewModelTest.kt | 28 +++++++++---------- build.gradle | 2 +- 11 files changed, 66 insertions(+), 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 index 7e5a43f7..8d12520e 100644 --- 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 index 9570368e..0045b020 100644 --- 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 index 7c6cce6c..2687e777 100644 --- 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(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 index 2851fddd..518e5406 100644 --- 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 index 2217bc52..2a9ac240 100644 --- 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 index e2e7e7a0..7cc6a948 100644 --- 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,7 +72,7 @@ class MainViewModel } } .onEach(syncInternetState::send) - .launchIn(ioScope) + .launchIn(scope) getStorageUsage .observe() @@ -80,9 +80,9 @@ class MainViewModel 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 index c47905d8..86d0c9cc 100644 --- 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 index 546165f9..be778f38 100644 --- 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,7 +32,7 @@ class InternetSyncViewModel val finish get() = finishChannel.asFlow() init { - val syncJob = ioScope.launch { + val syncJob = scope.launch { publicSync.sync() } @@ -40,7 +40,7 @@ class InternetSyncViewModel 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 index 6e6381c1..91fb37bb 100644 --- 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 index 83c92227..19cd166a 100644 --- 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 index 2685f12b..ba69a164 100644 --- 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'