From 88b53821bd04cb272cad6bbb00ca039479bbc34b Mon Sep 17 00:00:00 2001 From: austinried <4966622+austinried@users.noreply.github.com> Date: Fri, 2 Oct 2026 11:36:35 +0000 Subject: [PATCH] let tests own the worker coroutine scopes The repositories and the playback controller each created a hidden SupervisorJob scope and exposed close() only so tests could join it before closing the DB; production never called close(). Inject the scope instead and have each test cancel and join the scope it passes, so no test-only teardown lives in the app. Koin now provides the app/io/playback scopes. Also read upcomingItem under the controller lock, and give the wall-clock test waits one shared 30s budget instead of 5s per file: a loaded runner could starve the controller's single thread past the old deadline. --- .../data/download/DownloadNotifier.kt | 11 +--- .../subtracks/data/repo/DownloadRepository.kt | 10 +-- .../subtracks/data/repo/LibraryRepository.kt | 6 +- .../subtracks/data/repo/SourceRepository.kt | 12 +--- .../com/subtracks/data/sync/SyncManager.kt | 11 +--- .../main/kotlin/com/subtracks/di/AppModule.kt | 30 +++++++-- .../subtracks/playback/PlaybackController.kt | 24 +++---- .../test/kotlin/com/subtracks/TestSupport.kt | 15 +++++ .../data/repo/DownloadRepositoryTest.kt | 17 +++-- .../data/repo/LibraryRepositoryTest.kt | 21 ++++++- .../data/repo/SourceRepositoryTest.kt | 27 +++++--- .../subtracks/data/sync/SyncManagerTest.kt | 32 ++++++---- .../kotlin/com/subtracks/di/AppModuleTest.kt | 29 ++++++--- .../playback/PlaybackControllerTest.kt | 62 ++++++++++++------- .../ui/downloads/DownloadsViewModelTest.kt | 22 ++++--- .../ui/playback/QueueViewModelTest.kt | 27 ++++---- .../ui/settings/AddSourceViewModelTest.kt | 19 +++--- 17 files changed, 220 insertions(+), 155 deletions(-) create mode 100644 app/src/test/kotlin/com/subtracks/TestSupport.kt diff --git a/app/src/main/kotlin/com/subtracks/data/download/DownloadNotifier.kt b/app/src/main/kotlin/com/subtracks/data/download/DownloadNotifier.kt index c30f26f8..1b83a451 100644 --- a/app/src/main/kotlin/com/subtracks/data/download/DownloadNotifier.kt +++ b/app/src/main/kotlin/com/subtracks/data/download/DownloadNotifier.kt @@ -13,11 +13,8 @@ import com.subtracks.R import com.subtracks.data.repo.DownloadRepository import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.cancel import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking /** * Our own aggregated progress notification for the downloads the platform engine runs. The engine's @@ -27,8 +24,8 @@ import kotlinx.coroutines.runBlocking class DownloadNotifier( private val context: Context, private val repository: DownloadRepository, + private val scope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Default), ) { - private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) private val manager = NotificationManagerCompat.from(context) fun start() { @@ -45,12 +42,6 @@ class DownloadNotifier( } } - internal fun close() { - val job = scope.coroutineContext[Job] - scope.cancel() - runBlocking { job?.join() } - } - private fun build(content: DownloadNotification): Notification = NotificationCompat .Builder(context, CHANNEL_ID) diff --git a/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt index 96a6a671..b508e7e3 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/DownloadRepository.kt @@ -24,7 +24,6 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.cancel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow @@ -35,7 +34,6 @@ import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -51,8 +49,8 @@ class DownloadRepository( private val artworkFetcher: ArtworkFetcher, private val showMessage: (String) -> Unit = {}, private val dispatcher: CoroutineDispatcher = Dispatchers.IO, + private val scope: CoroutineScope = CoroutineScope(SupervisorJob() + dispatcher), ) { - private val scope = CoroutineScope(SupervisorJob() + dispatcher) private val mutex = Mutex() private val loaded = CompletableDeferred() private var pollJob: Job? = null @@ -83,12 +81,6 @@ class DownloadRepository( } } - fun close() { - val job = scope.coroutineContext[Job] - scope.cancel() - runBlocking { job?.join() } - } - fun states(): StateFlow> = statesFlow @OptIn(ExperimentalCoroutinesApi::class) diff --git a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt index 5e245792..bd8a1fde 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/LibraryRepository.kt @@ -34,6 +34,7 @@ class LibraryRepository( private val sourceRepository: SourceRepository, private val serverActions: ServerActionSink, private val showMessage: (String) -> Unit = {}, + private val scope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.IO), ) { val activeSourceId: Flow = sourceRepository.activeSourceId() @@ -280,21 +281,20 @@ class LibraryRepository( } private val starLock = Mutex() - private val starScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) fun star( type: StarType, id: String, starred: Boolean, ) { - starScope.launch { setStar(type, id, starred) } + scope.launch { setStar(type, id, starred) } } fun toggleStar( type: StarType, id: String, ) { - starScope.launch { + scope.launch { starLock.withLock { val sourceId = sourceRepository.activeSourceIdOnce() ?: return@withLock setStarLocked(type, id, starredValue(sourceId, type, id) == null) diff --git a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt index 9b8786f4..0bfff69d 100644 --- a/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt +++ b/app/src/main/kotlin/com/subtracks/data/repo/SourceRepository.kt @@ -16,9 +16,7 @@ import com.subtracks.data.source.streamLengthSuffix import com.subtracks.data.source.subsonic.SubsonicClient import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -26,7 +24,6 @@ import kotlinx.coroutines.flow.combine import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withContext import okhttp3.HttpUrl.Companion.toHttpUrl import okhttp3.OkHttpClient @@ -39,9 +36,8 @@ class SourceRepository( private val artworkStore: ArtworkStore, networkMode: Flow = flowOf(NetworkMode.Wifi), private val showMessage: (String) -> Unit = {}, + private val scope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Default), ) { - private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) - private data class ActiveSource( val id: Long, val source: SubsonicMusicSource, @@ -101,12 +97,6 @@ class SourceRepository( } } - fun close() { - val job = scope.coroutineContext[Job] - scope.cancel() - runBlocking { job?.join() } - } - fun sources(): Flow> = db.sourcesDao().sources() fun activeSourceId(): Flow = db.sourcesDao().activeSourceId() diff --git a/app/src/main/kotlin/com/subtracks/data/sync/SyncManager.kt b/app/src/main/kotlin/com/subtracks/data/sync/SyncManager.kt index 91ddf68d..206bbdab 100644 --- a/app/src/main/kotlin/com/subtracks/data/sync/SyncManager.kt +++ b/app/src/main/kotlin/com/subtracks/data/sync/SyncManager.kt @@ -7,14 +7,11 @@ import com.subtracks.data.repo.SourceRepository import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.cancel import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.launch -import kotlinx.coroutines.runBlocking sealed interface SyncStatus { data object Idle : SyncStatus @@ -33,8 +30,8 @@ class SyncManager( private val sourceRepository: SourceRepository, private val queueRepository: QueueRepository, private val showMessage: (String) -> Unit = {}, + private val scope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Default), ) { - private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) private val requests = Channel(Channel.CONFLATED) private val _status = MutableStateFlow(SyncStatus.Idle) val status: StateFlow = _status @@ -49,12 +46,6 @@ class SyncManager( requests.trySend(Unit) } - fun close() { - val job = scope.coroutineContext[Job] - scope.cancel() - runBlocking { job?.join() } - } - private suspend fun runSync() { if (sourceRepository.offline.value) { showMessage("Offline mode is on") diff --git a/app/src/main/kotlin/com/subtracks/di/AppModule.kt b/app/src/main/kotlin/com/subtracks/di/AppModule.kt index bbbd906a..a7c0c0fa 100644 --- a/app/src/main/kotlin/com/subtracks/di/AppModule.kt +++ b/app/src/main/kotlin/com/subtracks/di/AppModule.kt @@ -37,8 +37,12 @@ import com.subtracks.ui.library.PlaylistDetailViewModel import com.subtracks.ui.playback.QueueViewModel import com.subtracks.ui.settings.AddSourceViewModel import com.subtracks.ui.settings.SettingsViewModel +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob import okhttp3.OkHttpClient import org.koin.core.module.dsl.viewModel +import org.koin.core.qualifier.named import org.koin.dsl.module import java.io.File @@ -53,11 +57,24 @@ fun appModule( single { createAndroidDatabase(context) } single { http } single { createUserPreferences(context) } - single { SourceRepository(get(), get(), get(), get(), networkMode = networkMode(context.applicationContext), showMessage = toast) } + single { CoroutineScope(SupervisorJob() + Dispatchers.Default) } + single(named("io")) { CoroutineScope(SupervisorJob() + Dispatchers.IO) } + single(named("playback")) { CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate) } + single { + SourceRepository( + get(), + get(), + get(), + get(), + networkMode = networkMode(context.applicationContext), + showMessage = toast, + scope = get(), + ) + } single { NetworkServerActionSink(get()) } - single { LibraryRepository(get(), get(), get(), toast) } + single { LibraryRepository(get(), get(), get(), toast, scope = get(named("io"))) } single { ArtworkSeedRepository(get()) } - single { SyncManager(get(), get(), get(), showMessage = toast) } + single { SyncManager(get(), get(), get(), showMessage = toast, scope = get()) } single { QueueRepository(get()) } single { ArtworkStore(downloadsRoot(context)) } single { OkHttpArtworkFetcher(get()) } @@ -71,16 +88,19 @@ fun appModule( artworkStore = get(), artworkFetcher = get(), showMessage = toast, + dispatcher = Dispatchers.IO, + scope = get(named("io")), ).also { it.start() } } - single(createdAtStart = true) { DownloadNotifier(context.applicationContext, get()).also { it.start() } } + single(createdAtStart = true) { DownloadNotifier(context.applicationContext, get(), scope = get()).also { it.start() } } single { MediaSessionConnection(context.applicationContext, get(), get()) } - single { PlaybackController(get(), get(), get(), get(), showMessage = toast) } + single { PlaybackController(get(), get(), get(), get(), showMessage = toast, scope = get(named("playback"))) } single(createdAtStart = true) { val playback = get() Scrobbler( sink = get(), enabled = get().scrobbling(), + scope = get(), recordPlay = get()::recordPlay, ).also { it.attach(playback.state, playback.positionMs) diff --git a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt index 4678a99a..65bd1f8c 100644 --- a/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt +++ b/app/src/main/kotlin/com/subtracks/playback/PlaybackController.kt @@ -11,12 +11,10 @@ import com.subtracks.data.repo.QueueWindowItem import com.subtracks.data.repo.Shuffle import com.subtracks.data.repo.SourceRepository import kotlinx.coroutines.CancellationException -import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.Job import kotlinx.coroutines.SupervisorJob -import kotlinx.coroutines.cancel import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.StateFlow @@ -111,9 +109,8 @@ class PlaybackController( private val connection: PlayerConnection, private val downloads: DownloadRepository, private val showMessage: (String) -> Unit = {}, - dispatcher: CoroutineDispatcher = Dispatchers.Main.immediate, + private val scope: CoroutineScope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate), ) { - private val scope = CoroutineScope(SupervisorJob() + dispatcher) private val _state = MutableStateFlow(PlaybackState()) val state: StateFlow = _state @@ -169,14 +166,6 @@ class PlaybackController( } } - fun close() { - scope.cancel() - } - - internal suspend fun awaitStopped() { - scope.coroutineContext[Job]?.join() - } - private val _ready = MutableStateFlow(false) val ready: StateFlow = _ready @@ -879,11 +868,12 @@ class PlaybackController( thumbnail: Boolean = false, ): CoverArtRef? = sourceRepository.coverArt(item?.coverArtId, thumbnail) - suspend fun upcomingItem(): QueueItem? { - val snap = snapshot ?: return null - val position = currentPosition() ?: return null - return queueRepository.itemAt(snap, position + 1)?.toQueueItem() - } + suspend fun upcomingItem(): QueueItem? = + startLock.withLock { + val snap = snapshot ?: return@withLock null + val position = currentPosition() ?: return@withLock null + queueRepository.itemAt(snap, position + 1)?.toQueueItem() + } private fun play( entries: List, diff --git a/app/src/test/kotlin/com/subtracks/TestSupport.kt b/app/src/test/kotlin/com/subtracks/TestSupport.kt new file mode 100644 index 00000000..6db81ee8 --- /dev/null +++ b/app/src/test/kotlin/com/subtracks/TestSupport.kt @@ -0,0 +1,15 @@ +package com.subtracks + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.runBlocking + +// Wall-clock budget for the tests that poll asynchronous playback/repository work on a real +// dispatcher. A loaded CI runner can starve those threads for seconds; this only needs to catch a +// real hang, not a slow machine. +const val TEST_TIMEOUT_MS = 30_000L + +fun cancelAndJoinBlocking(vararg scopes: CoroutineScope) { + runBlocking { scopes.forEach { it.coroutineContext[Job]?.cancelAndJoin() } } +} diff --git a/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt index 0c97abfc..4fee5c9d 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/DownloadRepositoryTest.kt @@ -6,6 +6,7 @@ import androidx.room3.Room import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.TEST_TIMEOUT_MS import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkFetcher import com.subtracks.data.download.ArtworkStore @@ -24,7 +25,11 @@ import com.subtracks.data.model.Source import com.subtracks.data.model.SubsonicSource import com.subtracks.data.model.coverArtKey import com.subtracks.data.prefs.fakeUserPreferences +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking @@ -52,6 +57,7 @@ class DownloadRepositoryTest { private lateinit var repository: DownloadRepository private lateinit var artwork: ArtworkStore private lateinit var dir: File + private lateinit var scope: CoroutineScope private val prefs = fakeUserPreferences() private val messages = CopyOnWriteArrayList() private val requestedArt = CopyOnWriteArrayList() @@ -74,7 +80,8 @@ class DownloadRepositoryTest { .setDriver(BundledSQLiteDriver()) .build() artwork = ArtworkStore(File(context.cacheDir, "art-${System.nanoTime()}")) - sources = SourceRepository(db, OkHttpClient(), prefs, artwork) + scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + sources = SourceRepository(db, OkHttpClient(), prefs, artwork, scope = scope) engine = FakeDownloadEngine() dir = File(context.cacheDir, "downloads-${System.nanoTime()}") repository = @@ -86,13 +93,13 @@ class DownloadRepositoryTest { artworkStore = artwork, artworkFetcher = fetcher, showMessage = { messages += it }, + scope = scope, ) } @After fun tearDown() { - repository.close() - sources.close() + runBlocking { scope.coroutineContext[Job]?.cancelAndJoin() } db.close() dir.deleteRecursively() } @@ -1153,9 +1160,9 @@ class DownloadRepositoryTest { } private fun await(predicate: () -> Boolean) { - val deadline = System.nanoTime() + 5_000_000_000L + val deadline = System.nanoTime() + TEST_TIMEOUT_MS * 1_000_000 while (!predicate() && System.nanoTime() < deadline) Thread.sleep(10) - assertTrue("Timed out waiting for the expected download state", predicate()) + assertTrue("Timed out after ${TEST_TIMEOUT_MS}ms waiting for the expected download state", predicate()) } private companion object { diff --git a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt index a007b322..3d907c71 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/LibraryRepositoryTest.kt @@ -10,6 +10,11 @@ import com.subtracks.data.download.ArtworkStore import com.subtracks.data.model.Song import com.subtracks.data.prefs.fakeUserPreferences import com.subtracks.data.source.StarType +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first import kotlinx.coroutines.launch @@ -34,6 +39,7 @@ class LibraryRepositoryTest { private lateinit var db: SubtracksDatabase private lateinit var sourceRepository: SourceRepository private lateinit var repository: LibraryRepository + private lateinit var scope: CoroutineScope private val messages = ArrayList() @Before @@ -44,13 +50,22 @@ class LibraryRepositoryTest { .inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java) .setDriver(BundledSQLiteDriver()) .build() - sourceRepository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), ArtworkStore(File(context.cacheDir, "art"))) - repository = LibraryRepository(db, sourceRepository, NetworkServerActionSink(sourceRepository), showMessage = { messages += it }) + scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + sourceRepository = + SourceRepository(db, OkHttpClient(), fakeUserPreferences(), ArtworkStore(File(context.cacheDir, "art")), scope = scope) + repository = + LibraryRepository( + db, + sourceRepository, + NetworkServerActionSink(sourceRepository), + showMessage = { messages += it }, + scope = scope, + ) } @After fun tearDown() { - sourceRepository.close() + runBlocking { scope.coroutineContext[Job]?.cancelAndJoin() } db.close() } diff --git a/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt b/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt index ca52d943..4ef84fb5 100644 --- a/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/repo/SourceRepositoryTest.kt @@ -5,6 +5,7 @@ import androidx.room3.Room import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.TEST_TIMEOUT_MS import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore import com.subtracks.data.model.CoverArtRef @@ -12,8 +13,12 @@ import com.subtracks.data.model.coverArtKey import com.subtracks.data.prefs.StreamQuality import com.subtracks.data.prefs.UserPreferences import com.subtracks.data.prefs.fakeUserPreferences +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.async +import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.delay import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.toList @@ -43,6 +48,7 @@ class SourceRepositoryTest { private lateinit var prefs: UserPreferences private lateinit var repository: SourceRepository private lateinit var artwork: ArtworkStore + private lateinit var scope: CoroutineScope private val messages = ArrayList() @Before @@ -55,12 +61,13 @@ class SourceRepositoryTest { .build() artwork = ArtworkStore(File(context.cacheDir, "art-${System.nanoTime()}")) prefs = fakeUserPreferences() - repository = SourceRepository(db, OkHttpClient(), prefs, artwork, showMessage = { messages += it }) + scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + repository = SourceRepository(db, OkHttpClient(), prefs, artwork, showMessage = { messages += it }, scope = scope) } @After fun tearDown() { - repository.close() + runBlocking { scope.coroutineContext[Job]?.cancelAndJoin() } db.close() } @@ -69,14 +76,14 @@ class SourceRepositoryTest { runBlocking { repository.addSource("nav", "http://a.example/", "u", "p", false) val sourceId = repository.activeSourceId().first()!! - withTimeout(5_000) { while (repository.downloadUri(sourceId, "s1") == null) delay(10) } + withTimeout(TEST_TIMEOUT_MS) { while (repository.downloadUri(sourceId, "s1") == null) delay(10) } assertTrue(repository.downloadUri(sourceId, "s1")!!.contains("/rest/download")) prefs.setDownloadQuality(StreamQuality(maxBitrate = 128, format = "opus")) var transcoded: String? = null - withTimeout(5_000) { + withTimeout(TEST_TIMEOUT_MS) { while (transcoded == null) { delay(10) transcoded = repository.downloadUri(sourceId, "s1")?.takeIf { it.contains("/rest/stream") } @@ -155,9 +162,9 @@ class SourceRepositoryTest { repository.addSource("nav", server.url("/").toString(), "u", "s3cret", true) - val config = withTimeout(5_000) { repository.activeConfig().first { it != null && !it.useTokenAuth } } + val config = withTimeout(TEST_TIMEOUT_MS) { repository.activeConfig().first { it != null && !it.useTokenAuth } } assertFalse(config!!.useTokenAuth) - withTimeout(5_000) { while (messages.isEmpty()) delay(10) } + withTimeout(TEST_TIMEOUT_MS) { while (messages.isEmpty()) delay(10) } assertEquals(1, messages.size) } } @@ -250,7 +257,7 @@ class SourceRepositoryTest { repository.addSource("server", "http://a.example/", "u", "p", true) val sourceId = awaitActiveSourceId() repository.setOfflineMode(true) - withTimeout(5_000) { repository.offline.first { it } } + withTimeout(TEST_TIMEOUT_MS) { repository.offline.first { it } } assertEquals(null, repository.streamUri("s1", 1_000L, StreamQuality())) assertEquals(null, repository.networkCoverArt(sourceId, "art-1", false)) @@ -266,17 +273,17 @@ class SourceRepositoryTest { val sourceId = awaitActiveSourceId() artwork.write(sourceId, coverArtKey(sourceId, "art-1", false), byteArrayOf(1, 2, 3)) repository.setOfflineMode(true) - withTimeout(5_000) { repository.offline.first { it } } + withTimeout(TEST_TIMEOUT_MS) { repository.offline.first { it } } val ref = awaitCoverArt("art-1") assertTrue("expected the stored file, was ${ref.url}", ref.url.startsWith("file:")) } - private suspend fun awaitActiveSourceId(): Long = withTimeout(5_000) { repository.activeSourceId().first { it != null }!! } + private suspend fun awaitActiveSourceId(): Long = withTimeout(TEST_TIMEOUT_MS) { repository.activeSourceId().first { it != null }!! } private suspend fun awaitCoverArt(coverArt: String): CoverArtRef = - withTimeout(5_000) { + withTimeout(TEST_TIMEOUT_MS) { var ref = repository.coverArt(coverArt) while (ref == null) { delay(10) diff --git a/app/src/test/kotlin/com/subtracks/data/sync/SyncManagerTest.kt b/app/src/test/kotlin/com/subtracks/data/sync/SyncManagerTest.kt index 599eff16..b8357dfc 100644 --- a/app/src/test/kotlin/com/subtracks/data/sync/SyncManagerTest.kt +++ b/app/src/test/kotlin/com/subtracks/data/sync/SyncManagerTest.kt @@ -5,11 +5,17 @@ import androidx.room3.Room import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.TEST_TIMEOUT_MS import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore import com.subtracks.data.prefs.fakeUserPreferences import com.subtracks.data.repo.QueueRepository import com.subtracks.data.repo.SourceRepository +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.flow.first import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout @@ -37,6 +43,7 @@ class SyncManagerTest { private lateinit var sourceRepository: SourceRepository private lateinit var queueRepository: QueueRepository private lateinit var manager: SyncManager + private lateinit var scope: CoroutineScope private val messages = CopyOnWriteArrayList() private lateinit var server: MockWebServer @@ -48,17 +55,18 @@ class SyncManagerTest { .inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java) .setDriver(BundledSQLiteDriver()) .build() - sourceRepository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), ArtworkStore(File(context.cacheDir, "art"))) + scope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + sourceRepository = + SourceRepository(db, OkHttpClient(), fakeUserPreferences(), ArtworkStore(File(context.cacheDir, "art")), scope = scope) queueRepository = QueueRepository(db) - manager = SyncManager(db, sourceRepository, queueRepository, showMessage = { messages += it }) + manager = SyncManager(db, sourceRepository, queueRepository, showMessage = { messages += it }, scope = scope) server = MockWebServer() server.start() } @After fun tearDown() { - manager.close() - sourceRepository.close() + runBlocking { scope.coroutineContext[Job]?.cancelAndJoin() } db.close() server.shutdown() } @@ -67,7 +75,7 @@ class SyncManagerTest { fun aFailedSyncTellsTheUser() = runBlocking { manager.requestSync() - withTimeout(10_000) { manager.status.first { it is SyncStatus.Failed } } + withTimeout(TEST_TIMEOUT_MS) { manager.status.first { it is SyncStatus.Failed } } assertEquals(listOf("Sync failed: No server configured"), messages) } @@ -78,7 +86,7 @@ class SyncManagerTest { val before = queueRepository.snapshot().version manager.requestSync() - val status = withTimeout(10_000) { manager.status.first { it is SyncStatus.Failed } } + val status = withTimeout(TEST_TIMEOUT_MS) { manager.status.first { it is SyncStatus.Failed } } assertTrue(status is SyncStatus.Failed) assertEquals(before + 1, queueRepository.snapshot().version) @@ -91,7 +99,7 @@ class SyncManagerTest { sourceRepository.addSource("Local", server.url("/").toString(), "u", "p", useTokenAuth = false) manager.requestSync() - withTimeout(10_000) { manager.status.first { it is SyncStatus.Success } } + withTimeout(TEST_TIMEOUT_MS) { manager.status.first { it is SyncStatus.Success } } assertEquals(listOf("ar1"), db.libraryDao().artistIds(1)) assertEquals(listOf("al1"), db.libraryDao().albumIds(1)) @@ -128,13 +136,13 @@ class SyncManagerTest { sourceRepository.addSource("Local", server.url("/").toString(), "u", "p", useTokenAuth = false) manager.requestSync() - withTimeout(10_000) { manager.status.first { it is SyncStatus.Failed } } + withTimeout(TEST_TIMEOUT_MS) { manager.status.first { it is SyncStatus.Failed } } assertEquals(listOf("ar1"), db.libraryDao().artistIds(1)) server.dispatcher = healthyDispatcher() manager.requestSync() - withTimeout(10_000) { manager.status.first { it is SyncStatus.Success } } + withTimeout(TEST_TIMEOUT_MS) { manager.status.first { it is SyncStatus.Success } } assertEquals(listOf("ar1"), db.libraryDao().artistIds(1)) assertEquals(listOf("al1"), db.libraryDao().albumIds(1)) @@ -173,7 +181,7 @@ class SyncManagerTest { manager.requestSync() releaseFirst.countDown() - withTimeout(10_000) { manager.status.first { it is SyncStatus.Success } } + withTimeout(TEST_TIMEOUT_MS) { manager.status.first { it is SyncStatus.Success } } assertTrue("the coalesced sync should start", secondArtistStarted.await(10, TimeUnit.SECONDS)) assertEquals(2, artistRequests.get()) } @@ -230,10 +238,10 @@ class SyncManagerTest { fun aSyncRequestWhileOfflineIsRefusedWithoutTouchingTheNetwork() = runBlocking { sourceRepository.setOfflineMode(true) - withTimeout(5_000) { sourceRepository.offline.first { it } } + withTimeout(TEST_TIMEOUT_MS) { sourceRepository.offline.first { it } } manager.requestSync() - val status = withTimeout(10_000) { manager.status.first { it is SyncStatus.Failed } } + val status = withTimeout(TEST_TIMEOUT_MS) { manager.status.first { it is SyncStatus.Failed } } assertEquals("Offline mode is on", (status as SyncStatus.Failed).message) } diff --git a/app/src/test/kotlin/com/subtracks/di/AppModuleTest.kt b/app/src/test/kotlin/com/subtracks/di/AppModuleTest.kt index 5c25969a..8fdb3cfc 100644 --- a/app/src/test/kotlin/com/subtracks/di/AppModuleTest.kt +++ b/app/src/test/kotlin/com/subtracks/di/AppModuleTest.kt @@ -3,10 +3,17 @@ package com.subtracks.di import android.content.Context import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.subtracks.data.download.DownloadNotifier import com.subtracks.data.repo.DownloadRepository -import com.subtracks.data.repo.SourceRepository import com.subtracks.playback.PlaybackController +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.Job +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import kotlinx.coroutines.test.resetMain +import kotlinx.coroutines.test.setMain import okhttp3.OkHttpClient import org.junit.After import org.junit.Assert.assertSame @@ -16,25 +23,31 @@ import org.junit.runner.RunWith import org.koin.core.Koin import org.koin.core.context.startKoin import org.koin.core.context.stopKoin +import org.koin.core.qualifier.named +@OptIn(ExperimentalCoroutinesApi::class) @RunWith(AndroidJUnit4::class) class AppModuleTest { private lateinit var koin: Koin + private val mainDispatcher = UnconfinedTestDispatcher() @Before - fun setUp() = stopKoin() + fun setUp() { + stopKoin() + Dispatchers.setMain(mainDispatcher) + } @After fun tearDown() { if (::koin.isInitialized) { - runCatching { - koin.get().close() - koin.get().close() - koin.get().close() - koin.get().close() + runBlocking { + koin.get().coroutineContext[Job]?.cancelAndJoin() + koin.get(named("io")).coroutineContext[Job]?.cancelAndJoin() + koin.get(named("playback")).coroutineContext[Job]?.cancelAndJoin() } } stopKoin() + Dispatchers.resetMain() } @Test diff --git a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt index 1c8d3006..c7394227 100644 --- a/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt +++ b/app/src/test/kotlin/com/subtracks/playback/PlaybackControllerTest.kt @@ -6,6 +6,8 @@ import androidx.room3.Room import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.TEST_TIMEOUT_MS +import com.subtracks.cancelAndJoinBlocking import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.FakeDownloadEngine @@ -26,7 +28,10 @@ import com.subtracks.data.prefs.fakeUserPreferences import com.subtracks.data.repo.DownloadRepository import com.subtracks.data.repo.QueueRepository import com.subtracks.data.repo.SourceRepository +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow @@ -52,6 +57,9 @@ import java.util.concurrent.Executors @RunWith(AndroidJUnit4::class) class PlaybackControllerTest { private val dispatcher = Executors.newSingleThreadExecutor().asCoroutineDispatcher() + private val repoScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + private val controllerScope = CoroutineScope(SupervisorJob() + dispatcher) + private val extraScopes = mutableListOf() private lateinit var db: SubtracksDatabase private lateinit var prefs: UserPreferences @@ -74,7 +82,15 @@ class PlaybackControllerTest { .build() prefs = fakeUserPreferences() networkMode = MutableStateFlow(NetworkMode.Wifi) - sources = SourceRepository(db, OkHttpClient(), prefs, ArtworkStore(File(context.cacheDir, "art")), networkMode = networkMode) + sources = + SourceRepository( + db, + OkHttpClient(), + prefs, + ArtworkStore(File(context.cacheDir, "art")), + networkMode = networkMode, + scope = repoScope, + ) queues = QueueRepository(db) downloadsDir = File(context.cacheDir, "downloads-${System.nanoTime()}") downloads = @@ -85,6 +101,7 @@ class PlaybackControllerTest { downloadsDir, artworkStore = ArtworkStore(File(context.cacheDir, "art")), artworkFetcher = { ByteArray(0) }, + scope = repoScope, ).also { it.start() } handle = FakePlayerHandle() controller = @@ -94,16 +111,13 @@ class PlaybackControllerTest { FakePlayerConnection(handle), downloads, showMessage = { messages += it }, - dispatcher = dispatcher, + scope = controllerScope, ) } @After fun tearDown() { - controller.close() - runBlocking { controller.awaitStopped() } - sources.close() - downloads.close() + cancelAndJoinBlocking(controllerScope, repoScope, *extraScopes.toTypedArray()) db.close() dispatcher.close() } @@ -649,7 +663,7 @@ class PlaybackControllerTest { runBlocking { queues.setPosition(12_000L) } val restoredHandle = FakePlayerHandle() - val restored = PlaybackController(sources, queues, FakePlayerConnection(restoredHandle), downloads, dispatcher = dispatcher) + val restored = controllerOn(dispatcher, restoredHandle) restored.connect() await { restored.state.value.item @@ -657,7 +671,6 @@ class PlaybackControllerTest { } assertEquals(12_000L, restored.positionMs.value) - restored.close() } @Test @@ -757,7 +770,7 @@ class PlaybackControllerTest { block: Runnable, ) = block.run() } - val eager = PlaybackController(sources, queues, FakePlayerConnection(eagerHandle), downloads, dispatcher = inline) + val eager = controllerOn(inline, eagerHandle) eager.playAlbum(1, "al1", 0) await { @@ -787,7 +800,6 @@ class PlaybackControllerTest { ?.id, ) assertEquals("q2", runBlocking { eager.upcomingItem()?.id }) - eager.close() } @Test @@ -804,7 +816,7 @@ class PlaybackControllerTest { block: Runnable, ) = block.run() } - val eager = PlaybackController(sources, queues, FakePlayerConnection(eagerHandle), downloads, dispatcher = inline) + val eager = controllerOn(inline, eagerHandle) eager.playAlbum(1, "al1", 0) await { @@ -830,7 +842,6 @@ class PlaybackControllerTest { ?.id, ) assertEquals(3, runBlocking { queues.snapshot().upNext.size }) - eager.close() } @Test @@ -847,7 +858,7 @@ class PlaybackControllerTest { block: Runnable, ) = block.run() } - val eager = PlaybackController(sources, queues, FakePlayerConnection(eagerHandle), downloads, dispatcher = inline) + val eager = controllerOn(inline, eagerHandle) eager.playAlbum(1, "al1", 0) await { @@ -873,7 +884,6 @@ class PlaybackControllerTest { ?.id, ) assertEquals(3L, runBlocking { queues.snapshot().upNextSize }) - eager.close() } @Test @@ -890,7 +900,7 @@ class PlaybackControllerTest { block: Runnable, ) = block.run() } - val eager = PlaybackController(sources, queues, FakePlayerConnection(eagerHandle), downloads, dispatcher = inline) + val eager = controllerOn(inline, eagerHandle) eager.playAlbum(1, "al1", 0) await { @@ -920,7 +930,6 @@ class PlaybackControllerTest { val queued = runBlocking { queues.snapshot().upNext.map { it.entry.refId } } assertTrue("c1" in queued) - eager.close() } @Test @@ -2308,7 +2317,7 @@ class PlaybackControllerTest { runBlocking { queues.setPosition(12_000L) } val restoredHandle = FakePlayerHandle() - val restored = PlaybackController(sources, queues, FakePlayerConnection(restoredHandle), downloads, dispatcher = dispatcher) + val restored = controllerOn(dispatcher, restoredHandle) restored.connect() await { restored.state.value.item @@ -2321,7 +2330,6 @@ class PlaybackControllerTest { "setWindow(size=3, start=0, position=12000)", restoredHandle.operations.first { it.startsWith("setWindow") }, ) - restored.close() } @Test @@ -2890,7 +2898,7 @@ class PlaybackControllerTest { songId: String, ) { val file = File(downloadsDir, "$sourceId/$songId") - val deadline = System.nanoTime() + 15_000_000_000L + val deadline = System.nanoTime() + TEST_TIMEOUT_MS * 1_000_000 while (System.nanoTime() < deadline) { runBlocking { db.downloadDao().upsert(SongDownload(sourceId, songId, DownloadStatus.Completed)) @@ -2906,12 +2914,12 @@ class PlaybackControllerTest { private fun setOffline() { sources.setOfflineMode(true) - runBlocking { withTimeout(5_000) { sources.offline.first { it } } } + runBlocking { withTimeout(TEST_TIMEOUT_MS) { sources.offline.first { it } } } } private fun setOnline() { sources.setOfflineMode(false) - runBlocking { withTimeout(5_000) { sources.offline.first { !it } } } + runBlocking { withTimeout(TEST_TIMEOUT_MS) { sources.offline.first { !it } } } } private fun seedAlbum( @@ -3063,9 +3071,17 @@ class PlaybackControllerTest { (0 until snapshot.size).mapNotNull { queues.itemAt(snapshot, it)?.song?.id } } + private fun controllerOn( + dispatcher: CoroutineDispatcher, + handle: FakePlayerHandle, + ): PlaybackController { + val scope = CoroutineScope(SupervisorJob() + dispatcher).also { extraScopes += it } + return PlaybackController(sources, queues, FakePlayerConnection(handle), downloads, scope = scope) + } + private fun await(predicate: () -> Boolean) { - val deadline = System.nanoTime() + 5_000_000_000L + val deadline = System.nanoTime() + TEST_TIMEOUT_MS * 1_000_000 while (!predicate() && System.nanoTime() < deadline) Thread.sleep(10) - assertTrue("Timed out waiting for the expected playback state", predicate()) + assertTrue("Timed out after ${TEST_TIMEOUT_MS}ms waiting for the expected playback state", predicate()) } } diff --git a/app/src/test/kotlin/com/subtracks/ui/downloads/DownloadsViewModelTest.kt b/app/src/test/kotlin/com/subtracks/ui/downloads/DownloadsViewModelTest.kt index e6bb8221..040c981f 100644 --- a/app/src/test/kotlin/com/subtracks/ui/downloads/DownloadsViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/downloads/DownloadsViewModelTest.kt @@ -6,6 +6,8 @@ import androidx.room3.Room import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.TEST_TIMEOUT_MS +import com.subtracks.cancelAndJoinBlocking import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.FakeDownloadEngine @@ -22,9 +24,11 @@ import com.subtracks.data.repo.SourceRepository import com.subtracks.playback.FakePlayerConnection import com.subtracks.playback.FakePlayerHandle import com.subtracks.playback.PlaybackController +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.runBlocking @@ -51,6 +55,8 @@ class DownloadsViewModelTest { private lateinit var downloadRepository: DownloadRepository private lateinit var controller: PlaybackController private lateinit var viewModel: DownloadsViewModel + private val repoScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + private val controllerScope = CoroutineScope(SupervisorJob() + dispatcher) private var sourceId: Long = 0 @Before @@ -63,7 +69,7 @@ class DownloadsViewModelTest { .setDriver(BundledSQLiteDriver()) .build() val artwork = ArtworkStore(File(context.cacheDir, "art-${System.nanoTime()}")) - sourceRepository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), artwork) + sourceRepository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), artwork, scope = repoScope) downloadRepository = DownloadRepository( db, @@ -72,6 +78,7 @@ class DownloadsViewModelTest { File(context.cacheDir, "downloads-${System.nanoTime()}"), artworkStore = artwork, artworkFetcher = { ByteArray(0) }, + scope = repoScope, ) controller = PlaybackController( @@ -79,11 +86,11 @@ class DownloadsViewModelTest { QueueRepository(db), FakePlayerConnection(FakePlayerHandle()), downloadRepository, - dispatcher = dispatcher, + scope = controllerScope, ) viewModel = DownloadsViewModel( - LibraryRepository(db, sourceRepository, NetworkServerActionSink(sourceRepository)), + LibraryRepository(db, sourceRepository, NetworkServerActionSink(sourceRepository), scope = repoScope), downloadRepository, controller, ) @@ -92,10 +99,7 @@ class DownloadsViewModelTest { @After fun tearDown() { runBlocking { viewModel.viewModelScope.coroutineContext[Job]?.cancelAndJoin() } - controller.close() - runBlocking { controller.awaitStopped() } - downloadRepository.close() - sourceRepository.close() + cancelAndJoinBlocking(controllerScope, repoScope) db.close() Dispatchers.resetMain() dispatcher.close() @@ -174,8 +178,8 @@ class DownloadsViewModelTest { private fun allRows(): List = runBlocking { db.downloadDao().all() } private fun await(predicate: () -> Boolean) { - val deadline = System.nanoTime() + 5_000_000_000L + val deadline = System.nanoTime() + TEST_TIMEOUT_MS * 1_000_000 while (!predicate() && System.nanoTime() < deadline) Thread.sleep(10) - assertTrue("Timed out waiting for the expected state", predicate()) + assertTrue("Timed out after ${TEST_TIMEOUT_MS}ms waiting for the expected state", predicate()) } } diff --git a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt index 2a8a4acd..5b4d18ec 100644 --- a/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/playback/QueueViewModelTest.kt @@ -6,6 +6,8 @@ import androidx.room3.Room import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.TEST_TIMEOUT_MS +import com.subtracks.cancelAndJoinBlocking import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.FakeDownloadEngine @@ -22,9 +24,11 @@ import com.subtracks.data.repo.SourceRepository import com.subtracks.playback.FakePlayerConnection import com.subtracks.playback.FakePlayerHandle import com.subtracks.playback.PlaybackController +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.flow.first @@ -56,6 +60,8 @@ class QueueViewModelTest { private lateinit var downloadsDir: File private lateinit var controller: PlaybackController private lateinit var viewModel: QueueViewModel + private val repoScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) + private val controllerScope = CoroutineScope(SupervisorJob() + dispatcher) @Before fun setUp() { @@ -66,7 +72,8 @@ class QueueViewModelTest { .inMemoryDatabaseBuilder(context, SubtracksDatabase::class.java) .setDriver(BundledSQLiteDriver()) .build() - sources = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), ArtworkStore(File(context.cacheDir, "art"))) + sources = + SourceRepository(db, OkHttpClient(), fakeUserPreferences(), ArtworkStore(File(context.cacheDir, "art")), scope = repoScope) queues = QueueRepository(db) val artwork = ArtworkStore(File(context.cacheDir, "art")) downloadsDir = File(context.cacheDir, "downloads-${System.nanoTime()}") @@ -78,18 +85,16 @@ class QueueViewModelTest { downloadsDir, artworkStore = artwork, artworkFetcher = { ByteArray(0) }, + scope = repoScope, ).also { it.start() } - controller = PlaybackController(sources, queues, FakePlayerConnection(FakePlayerHandle()), downloads, dispatcher = dispatcher) + controller = PlaybackController(sources, queues, FakePlayerConnection(FakePlayerHandle()), downloads, scope = controllerScope) viewModel = QueueViewModel(queues, controller, sources, downloads) } @After fun tearDown() { runBlocking { viewModel.viewModelScope.coroutineContext[Job]?.cancelAndJoin() } - controller.close() - runBlocking { controller.awaitStopped() } - downloads.close() - sources.close() + cancelAndJoinBlocking(controllerScope, repoScope) db.close() Dispatchers.resetMain() dispatcher.close() @@ -455,7 +460,7 @@ class QueueViewModelTest { private fun setOffline() { runBlocking { sources.setOfflineMode(true) - withTimeout(5_000) { sources.offline.first { it } } + withTimeout(TEST_TIMEOUT_MS) { sources.offline.first { it } } } } @@ -514,10 +519,8 @@ class QueueViewModelTest { private fun runOnMain(block: () -> Unit) = runBlocking { withContext(dispatcher) { block() } } private fun await(predicate: () -> Boolean) { - repeat(500) { - if (predicate()) return - Thread.sleep(10) - } - assertTrue("Timed out waiting for the expected state", predicate()) + val deadline = System.nanoTime() + TEST_TIMEOUT_MS * 1_000_000 + while (!predicate() && System.nanoTime() < deadline) Thread.sleep(10) + assertTrue("Timed out after ${TEST_TIMEOUT_MS}ms waiting for the expected state", predicate()) } } diff --git a/app/src/test/kotlin/com/subtracks/ui/settings/AddSourceViewModelTest.kt b/app/src/test/kotlin/com/subtracks/ui/settings/AddSourceViewModelTest.kt index c5c3b889..44fe0833 100644 --- a/app/src/test/kotlin/com/subtracks/ui/settings/AddSourceViewModelTest.kt +++ b/app/src/test/kotlin/com/subtracks/ui/settings/AddSourceViewModelTest.kt @@ -6,6 +6,8 @@ import androidx.room3.Room import androidx.sqlite.driver.bundled.BundledSQLiteDriver import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.subtracks.TEST_TIMEOUT_MS +import com.subtracks.cancelAndJoinBlocking import com.subtracks.data.db.SubtracksDatabase import com.subtracks.data.download.ArtworkStore import com.subtracks.data.download.FakeDownloadEngine @@ -14,9 +16,11 @@ import com.subtracks.data.repo.DownloadRepository import com.subtracks.data.repo.QueueRepository import com.subtracks.data.repo.SourceRepository import com.subtracks.data.sync.SyncManager +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.Job +import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.cancelAndJoin import kotlinx.coroutines.runBlocking @@ -45,8 +49,8 @@ class AddSourceViewModelTest { private lateinit var sourceRepository: SourceRepository private lateinit var downloadRepository: DownloadRepository private lateinit var server: MockWebServer + private val repoScope = CoroutineScope(SupervisorJob() + Dispatchers.Default) private val createdViewModels = mutableListOf() - private val syncManagers = mutableListOf() @Before fun setUp() { @@ -58,7 +62,7 @@ class AddSourceViewModelTest { .setDriver(BundledSQLiteDriver()) .build() val artwork = ArtworkStore(File(context.cacheDir, "art-${System.nanoTime()}")) - sourceRepository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), artwork) + sourceRepository = SourceRepository(db, OkHttpClient(), fakeUserPreferences(), artwork, scope = repoScope) downloadRepository = DownloadRepository( db, @@ -67,6 +71,7 @@ class AddSourceViewModelTest { File(context.cacheDir, "downloads-${System.nanoTime()}"), artworkStore = artwork, artworkFetcher = { ByteArray(0) }, + scope = repoScope, ) server = MockWebServer() server.start() @@ -75,10 +80,8 @@ class AddSourceViewModelTest { @After fun tearDown() { createdViewModels.forEach { runBlocking { it.viewModelScope.coroutineContext[Job]?.cancelAndJoin() } } - syncManagers.forEach { it.close() } + cancelAndJoinBlocking(repoScope) server.shutdown() - downloadRepository.close() - sourceRepository.close() db.close() Dispatchers.resetMain() dispatcher.close() @@ -137,7 +140,7 @@ class AddSourceViewModelTest { } private fun viewModel(sourceId: Long? = null): AddSourceViewModel { - val syncManager = SyncManager(db, sourceRepository, QueueRepository(db)).also { syncManagers += it } + val syncManager = SyncManager(db, sourceRepository, QueueRepository(db), scope = repoScope) return AddSourceViewModel(sourceRepository, syncManager, downloadRepository, sourceId).also { createdViewModels += it } } @@ -149,8 +152,8 @@ class AddSourceViewModelTest { ) private fun await(predicate: () -> Boolean) { - val deadline = System.nanoTime() + 5_000_000_000L + val deadline = System.nanoTime() + TEST_TIMEOUT_MS * 1_000_000 while (!predicate() && System.nanoTime() < deadline) Thread.sleep(10) - assertTrue("Timed out waiting for the expected state", predicate()) + assertTrue("Timed out after ${TEST_TIMEOUT_MS}ms waiting for the expected state", predicate()) } }