From 64383e8508da41454cf46b5bce45eaf2a57ecbcc Mon Sep 17 00:00:00 2001
From: Matthias Urhahn <matthias.urhahn@sap.com>
Date: Mon, 5 Oct 2020 11:10:51 +0200
Subject: [PATCH] Add coroutine dispatcher injection to fix tests (DEV) (#1299)
* Add coroutine dispatcher injection to deal with flaky tests.
We need to be able to replace Dispatchers.IO in tests, and that requires providing the dispatcher via injection.
* Linting
* Replace remaining static access of Dispatchers.IO.
---
.../download/KeyFileDownloader.kt | 11 ++--
.../util/coroutine/CoroutineModule.kt | 11 ++++
.../coroutine/DefaultDispatcherProvider.kt | 7 +++
.../util/coroutine/DispatcherProvider.kt | 21 +++++++
.../util/di/ApplicationComponent.kt | 2 +
.../download/KeyFileDownloaderTest.kt | 57 ++++++++-----------
.../testhelpers/TestDispatcherProvider.kt | 18 ++++++
7 files changed, 89 insertions(+), 38 deletions(-)
create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/CoroutineModule.kt
create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/DefaultDispatcherProvider.kt
create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/DispatcherProvider.kt
create mode 100644 Corona-Warn-App/src/test/java/testhelpers/TestDispatcherProvider.kt
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloader.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloader.kt
index 0d18d7a27..aa3b39451 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloader.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloader.kt
@@ -10,7 +10,7 @@ import de.rki.coronawarnapp.diagnosiskeys.storage.legacy.LegacyKeyCacheMigration
import de.rki.coronawarnapp.risk.TimeVariables
import de.rki.coronawarnapp.storage.AppSettings
import de.rki.coronawarnapp.storage.DeviceStorage
-import kotlinx.coroutines.Dispatchers
+import de.rki.coronawarnapp.util.coroutine.DispatcherProvider
import kotlinx.coroutines.async
import kotlinx.coroutines.awaitAll
import kotlinx.coroutines.withContext
@@ -28,7 +28,8 @@ class KeyFileDownloader @Inject constructor(
private val keyServer: DiagnosisKeyServer,
private val keyCache: KeyCacheRepository,
private val legacyKeyCache: LegacyKeyCacheMigration,
- private val settings: AppSettings
+ private val settings: AppSettings,
+ private val dispatcherProvider: DispatcherProvider
) {
private suspend fun requireStorageSpace(data: List<CountryData>): DeviceStorage.CheckResult {
@@ -68,7 +69,7 @@ class KeyFileDownloader @Inject constructor(
* @return list of all files from both the cache and the diff query
*/
suspend fun asyncFetchKeyFiles(wantedCountries: List<LocationCode>): List<File> =
- withContext(Dispatchers.IO) {
+ withContext(dispatcherProvider.IO) {
val availableCountries = keyServer.getCountryIndex()
val filteredCountries = availableCountries.filter { wantedCountries.contains(it) }
Timber.tag(TAG).v(
@@ -125,7 +126,7 @@ class KeyFileDownloader @Inject constructor(
*/
private suspend fun syncMissingDays(
availableCountries: List<LocationCode>
- ) = withContext(Dispatchers.IO) {
+ ) = withContext(dispatcherProvider.IO) {
val countriesWithMissingDays = determineMissingDays(availableCountries)
requireStorageSpace(countriesWithMissingDays)
@@ -256,7 +257,7 @@ class KeyFileDownloader @Inject constructor(
private suspend fun syncMissing3Hours(
availableCountries: List<LocationCode>,
hourItemLimit: Int
- ) = withContext(Dispatchers.IO) {
+ ) = withContext(dispatcherProvider.IO) {
Timber.tag(TAG).v(
"asyncHandleLast3HoursFilesFetch(availableCountries=%s, hourLimit=%d)",
availableCountries, hourItemLimit
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/CoroutineModule.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/CoroutineModule.kt
new file mode 100644
index 000000000..a5f94c251
--- /dev/null
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/CoroutineModule.kt
@@ -0,0 +1,11 @@
+package de.rki.coronawarnapp.util.coroutine
+
+import dagger.Binds
+import dagger.Module
+
+@Module
+abstract class CoroutineModule {
+
+ @Binds
+ abstract fun dispatcherProvider(defaultProvider: DefaultDispatcherProvider): DispatcherProvider
+}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/DefaultDispatcherProvider.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/DefaultDispatcherProvider.kt
new file mode 100644
index 000000000..74271f559
--- /dev/null
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/DefaultDispatcherProvider.kt
@@ -0,0 +1,7 @@
+package de.rki.coronawarnapp.util.coroutine
+
+import javax.inject.Inject
+import javax.inject.Singleton
+
+@Singleton
+class DefaultDispatcherProvider @Inject constructor() : DispatcherProvider
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/DispatcherProvider.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/DispatcherProvider.kt
new file mode 100644
index 000000000..1e4abd60c
--- /dev/null
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/coroutine/DispatcherProvider.kt
@@ -0,0 +1,21 @@
+package de.rki.coronawarnapp.util.coroutine
+
+import kotlinx.coroutines.CoroutineDispatcher
+import kotlinx.coroutines.Dispatchers
+
+// Need this to improve testing
+// Can currently only replace the main-thread dispatcher.
+// https://github.com/Kotlin/kotlinx.coroutines/issues/1365
+@Suppress("PropertyName", "VariableNaming")
+interface DispatcherProvider {
+ val Default: CoroutineDispatcher
+ get() = Dispatchers.Default
+ val Main: CoroutineDispatcher
+ get() = Dispatchers.Main
+ val MainImmediate: CoroutineDispatcher
+ get() = Dispatchers.Main.immediate
+ val Unconfined: CoroutineDispatcher
+ get() = Dispatchers.Unconfined
+ val IO: CoroutineDispatcher
+ get() = Dispatchers.IO
+}
diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/ApplicationComponent.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/ApplicationComponent.kt
index ff0449833..c9de5b246 100644
--- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/ApplicationComponent.kt
+++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/ApplicationComponent.kt
@@ -28,6 +28,7 @@ import de.rki.coronawarnapp.transaction.SubmitDiagnosisInjectionHelper
import de.rki.coronawarnapp.ui.ActivityBinder
import de.rki.coronawarnapp.util.ConnectivityHelperInjection
import de.rki.coronawarnapp.util.UtilModule
+import de.rki.coronawarnapp.util.coroutine.CoroutineModule
import de.rki.coronawarnapp.util.device.DeviceModule
import de.rki.coronawarnapp.util.security.EncryptedPreferencesFactory
import de.rki.coronawarnapp.util.security.EncryptionErrorResetTool
@@ -39,6 +40,7 @@ import javax.inject.Singleton
modules = [
AndroidSupportInjectionModule::class,
AssistedInjectModule::class,
+ CoroutineModule::class,
AndroidModule::class,
ReceiverBinder::class,
ServiceBinder::class,
diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloaderTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloaderTest.kt
index a39102dbe..3b4644059 100644
--- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloaderTest.kt
+++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/diagnosiskeys/download/KeyFileDownloaderTest.kt
@@ -27,11 +27,8 @@ import org.joda.time.LocalTime
import org.junit.jupiter.api.AfterEach
import org.junit.jupiter.api.BeforeEach
import org.junit.jupiter.api.Test
-import org.junit.jupiter.api.extension.ExtendWith
import testhelpers.BaseIOTest
-import testhelpers.extensions.CoroutinesTestExtension
-import testhelpers.extensions.InstantExecutorExtension
-import testhelpers.flakyTest
+import testhelpers.TestDispatcherProvider
import timber.log.Timber
import java.io.File
import java.io.IOException
@@ -42,7 +39,6 @@ import kotlin.time.ExperimentalTime
*/
@ExperimentalTime
@ExperimentalCoroutinesApi
-@ExtendWith(InstantExecutorExtension::class, CoroutinesTestExtension::class)
class KeyFileDownloaderTest : BaseIOTest() {
@MockK
@@ -113,8 +109,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
locationCode = arg(0),
day = arg(1),
hour = arg(2),
- saveTo = arg(3),
- precondition = arg(4)
+ saveTo = arg(3)
)
}
@@ -172,7 +167,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
checksum: String
) {
keyRepoData[keyInfo.id] = keyInfo.copy(
- isDownloadComplete = checksum != null, checksumMD5 = checksum
+ isDownloadComplete = true, checksumMD5 = checksum
)
}
@@ -181,7 +176,6 @@ class KeyFileDownloaderTest : BaseIOTest() {
day: LocalDate,
hour: LocalTime? = null,
saveTo: File,
- precondition: suspend (DownloadInfo) -> Boolean = { true },
checksumServerMD5: String? = "serverMD5",
checksumLocalMD5: String? = "localMD5"
): DownloadInfo {
@@ -218,15 +212,15 @@ class KeyFileDownloaderTest : BaseIOTest() {
keyServer = diagnosisKeyServer,
keyCache = keyCache,
legacyKeyCache = legacyMigration,
- settings = settings
+ settings = settings,
+ dispatcherProvider = TestDispatcherProvider
)
Timber.i("createDownloader(): %s", downloader)
return downloader
}
@Test
- fun `wanted country list is empty, day mode`() = flakyTest {
-
+ fun `wanted country list is empty, day mode`() {
val downloader = createDownloader()
runBlocking {
downloader.asyncFetchKeyFiles(emptyList()) shouldBe emptyList()
@@ -234,7 +228,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `wanted country list is empty, hour mode`() = flakyTest {
+ fun `wanted country list is empty, hour mode`() {
every { settings.isLast3HourModeEnabled } returns true
val downloader = createDownloader()
@@ -244,7 +238,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `fetching is aborted in day if not enough free storage`() = flakyTest {
+ fun `fetching is aborted in day if not enough free storage`() {
coEvery { deviceStorage.requireSpacePrivateStorage(1048576L) } throws InsufficientStorageException(
mockk(relaxed = true)
)
@@ -259,7 +253,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `fetching is aborted in hour if not enough free storage`() = flakyTest {
+ fun `fetching is aborted in hour if not enough free storage`() {
every { settings.isLast3HourModeEnabled } returns true
coEvery { deviceStorage.requireSpacePrivateStorage(67584L) } throws InsufficientStorageException(
@@ -276,7 +270,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `error during country index fetch`() = flakyTest {
+ fun `error during country index fetch`() {
coEvery { diagnosisKeyServer.getCountryIndex() } throws IOException()
val downloader = createDownloader()
@@ -289,7 +283,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `day fetch without prior data`() = flakyTest {
+ fun `day fetch without prior data`() {
val downloader = createDownloader()
runBlocking {
@@ -330,7 +324,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `day fetch with existing data`() = flakyTest {
+ fun `day fetch with existing data`() {
mockAddData(
type = CachedKeyInfo.Type.COUNTRY_DAY,
location = LocationCode("DE"),
@@ -377,7 +371,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `day fetch deletes stale data`() = flakyTest {
+ fun `day fetch deletes stale data`() {
coEvery { diagnosisKeyServer.getDayIndex(LocationCode("DE")) } returns listOf(
LocalDate.parse("2020-09-02")
)
@@ -425,7 +419,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `day fetch skips single download failures`() = flakyTest {
+ fun `day fetch skips single download failures`() {
var dlCounter = 0
coEvery { diagnosisKeyServer.downloadKeyFile(any(), any(), any(), any(), any()) } answers {
dlCounter++
@@ -434,8 +428,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
locationCode = arg(0),
day = arg(1),
hour = arg(2),
- saveTo = arg(3),
- precondition = arg(4)
+ saveTo = arg(3)
)
}
@@ -452,7 +445,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `last3Hours fetch without prior data`() = flakyTest {
+ fun `last3Hours fetch without prior data`() {
every { settings.isLast3HourModeEnabled } returns true
val downloader = createDownloader()
@@ -511,7 +504,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `last3Hours fetch with prior data`() = flakyTest {
+ fun `last3Hours fetch with prior data`() {
every { settings.isLast3HourModeEnabled } returns true
mockAddData(
@@ -577,7 +570,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `last3Hours fetch deletes stale data`() = flakyTest {
+ fun `last3Hours fetch deletes stale data`() {
every { settings.isLast3HourModeEnabled } returns true
val (staleKey1, _) = mockAddData(
@@ -659,7 +652,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `last3Hours fetch skips single download failures`() = flakyTest {
+ fun `last3Hours fetch skips single download failures`() {
every { settings.isLast3HourModeEnabled } returns true
var dlCounter = 0
@@ -670,8 +663,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
locationCode = arg(0),
day = arg(1),
hour = arg(2),
- saveTo = arg(3),
- precondition = arg(4)
+ saveTo = arg(3)
)
}
@@ -688,7 +680,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `not completed cache entries are overwritten`() = flakyTest {
+ fun `not completed cache entries are overwritten`() {
mockAddData(
type = CachedKeyInfo.Type.COUNTRY_DAY,
location = LocationCode("DE"),
@@ -716,7 +708,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `database errors do not abort the whole process`() = flakyTest {
+ fun `database errors do not abort the whole process`() {
var completionCounter = 0
coEvery { keyCache.markKeyComplete(any(), any()) } answers {
completionCounter++
@@ -744,7 +736,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `store server md5`() = flakyTest {
+ fun `store server md5`() {
coEvery { diagnosisKeyServer.getCountryIndex() } returns listOf(LocationCode("DE"))
coEvery { diagnosisKeyServer.getDayIndex(LocationCode("DE")) } returns listOf(
LocalDate.parse("2020-09-01")
@@ -774,7 +766,7 @@ class KeyFileDownloaderTest : BaseIOTest() {
}
@Test
- fun `use local MD5 as fallback if there is none available from the server`() = flakyTest {
+ fun `use local MD5 as fallback if there is none available from the server`() {
coEvery { diagnosisKeyServer.getCountryIndex() } returns listOf(LocationCode("DE"))
coEvery { diagnosisKeyServer.getDayIndex(LocationCode("DE")) } returns listOf(
LocalDate.parse("2020-09-01")
@@ -785,7 +777,6 @@ class KeyFileDownloaderTest : BaseIOTest() {
day = arg(1),
hour = arg(2),
saveTo = arg(3),
- precondition = arg(4),
checksumServerMD5 = null
)
}
diff --git a/Corona-Warn-App/src/test/java/testhelpers/TestDispatcherProvider.kt b/Corona-Warn-App/src/test/java/testhelpers/TestDispatcherProvider.kt
new file mode 100644
index 000000000..9b338acc7
--- /dev/null
+++ b/Corona-Warn-App/src/test/java/testhelpers/TestDispatcherProvider.kt
@@ -0,0 +1,18 @@
+package testhelpers
+
+import de.rki.coronawarnapp.util.coroutine.DispatcherProvider
+import kotlinx.coroutines.CoroutineDispatcher
+import kotlinx.coroutines.Dispatchers
+
+object TestDispatcherProvider : DispatcherProvider {
+ override val Default: CoroutineDispatcher
+ get() = Dispatchers.Unconfined
+ override val Main: CoroutineDispatcher
+ get() = Dispatchers.Unconfined
+ override val MainImmediate: CoroutineDispatcher
+ get() = Dispatchers.Unconfined
+ override val Unconfined: CoroutineDispatcher
+ get() = Dispatchers.Unconfined
+ override val IO: CoroutineDispatcher
+ get() = Dispatchers.Unconfined
+}
--
GitLab