mirror of
https://github.com/austinried/subtracks.git
synced 2026-11-11 15:29:15 +01:00
address review: join the controller scope, keep DB closes where nothing leaks
PlaybackController.now exposes awaitStopped() so the tests that own its DB can join its scope before db.close(); close() stays cancel-only so AppModuleTest (Main dispatcher) cannot deadlock. Restores db.close() in the tests that have no leaking scope, closes the SyncManager AddSourceViewModelTest was leaking, replaces the duplicated poll with the existing awaitCoverArt helper, and trims the testLogging block to the one line that does anything.
This commit is contained in:
@@ -103,10 +103,6 @@ tasks.withType<Test>().configureEach {
|
|||||||
}
|
}
|
||||||
testLogging {
|
testLogging {
|
||||||
exceptionFormat = TestExceptionFormat.FULL
|
exceptionFormat = TestExceptionFormat.FULL
|
||||||
events("failed")
|
|
||||||
showStackTraces = true
|
|
||||||
showExceptions = true
|
|
||||||
showCauses = true
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -173,6 +173,10 @@ class PlaybackController(
|
|||||||
scope.cancel()
|
scope.cancel()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
internal suspend fun awaitStopped() {
|
||||||
|
scope.coroutineContext[Job]?.join()
|
||||||
|
}
|
||||||
|
|
||||||
private val _ready = MutableStateFlow(false)
|
private val _ready = MutableStateFlow(false)
|
||||||
val ready: StateFlow<Boolean> = _ready
|
val ready: StateFlow<Boolean> = _ready
|
||||||
|
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import com.subtracks.data.model.ArtworkSeed
|
|||||||
import com.subtracks.data.model.Source
|
import com.subtracks.data.model.Source
|
||||||
import com.subtracks.data.repo.ArtworkSeedRepository
|
import com.subtracks.data.repo.ArtworkSeedRepository
|
||||||
import kotlinx.coroutines.test.runTest
|
import kotlinx.coroutines.test.runTest
|
||||||
|
import org.junit.After
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
import org.junit.Assert.assertNull
|
import org.junit.Assert.assertNull
|
||||||
import org.junit.Before
|
import org.junit.Before
|
||||||
@@ -29,6 +30,11 @@ class ArtworkSeedDaoTest {
|
|||||||
.build()
|
.build()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@After
|
||||||
|
fun tearDown() {
|
||||||
|
db.close()
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun deletingASourceCascadesToItsSeeds() =
|
fun deletingASourceCascadesToItsSeeds() =
|
||||||
runTest {
|
runTest {
|
||||||
|
|||||||
@@ -17,6 +17,7 @@ import com.subtracks.data.model.SongDownload
|
|||||||
import com.subtracks.data.model.Source
|
import com.subtracks.data.model.Source
|
||||||
import kotlinx.coroutines.flow.first
|
import kotlinx.coroutines.flow.first
|
||||||
import kotlinx.coroutines.test.runTest
|
import kotlinx.coroutines.test.runTest
|
||||||
|
import org.junit.After
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
import org.junit.Assert.assertFalse
|
import org.junit.Assert.assertFalse
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
@@ -38,6 +39,11 @@ class LibraryDaoTest {
|
|||||||
.build()
|
.build()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@After
|
||||||
|
fun tearDown() {
|
||||||
|
db.close()
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun albumSortsAndStarredFilter() =
|
fun albumSortsAndStarredFilter() =
|
||||||
runTest {
|
runTest {
|
||||||
|
|||||||
@@ -93,6 +93,7 @@ class DownloadRepositoryTest {
|
|||||||
fun tearDown() {
|
fun tearDown() {
|
||||||
repository.close()
|
repository.close()
|
||||||
sources.close()
|
sources.close()
|
||||||
|
db.close()
|
||||||
dir.deleteRecursively()
|
dir.deleteRecursively()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -51,6 +51,7 @@ class LibraryRepositoryTest {
|
|||||||
@After
|
@After
|
||||||
fun tearDown() {
|
fun tearDown() {
|
||||||
sourceRepository.close()
|
sourceRepository.close()
|
||||||
|
db.close()
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
|
|||||||
@@ -12,6 +12,7 @@ import com.subtracks.data.model.PlaylistSong
|
|||||||
import com.subtracks.data.model.Song
|
import com.subtracks.data.model.Song
|
||||||
import com.subtracks.data.model.Source
|
import com.subtracks.data.model.Source
|
||||||
import kotlinx.coroutines.test.runTest
|
import kotlinx.coroutines.test.runTest
|
||||||
|
import org.junit.After
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
import org.junit.Before
|
import org.junit.Before
|
||||||
@@ -34,6 +35,11 @@ class QueueKeysetTest {
|
|||||||
repository = QueueRepository(db)
|
repository = QueueRepository(db)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@After
|
||||||
|
fun tearDown() {
|
||||||
|
db.close()
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun albumKeysetFromMatchesOffsetWindowAtADeepPosition() =
|
fun albumKeysetFromMatchesOffsetWindowAtADeepPosition() =
|
||||||
runTest {
|
runTest {
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import com.subtracks.data.model.QueueKind
|
|||||||
import com.subtracks.data.model.Song
|
import com.subtracks.data.model.Song
|
||||||
import com.subtracks.data.model.Source
|
import com.subtracks.data.model.Source
|
||||||
import kotlinx.coroutines.test.runTest
|
import kotlinx.coroutines.test.runTest
|
||||||
|
import org.junit.After
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
import org.junit.Assert.assertFalse
|
import org.junit.Assert.assertFalse
|
||||||
import org.junit.Assert.assertTrue
|
import org.junit.Assert.assertTrue
|
||||||
@@ -38,6 +39,11 @@ class QueueRepositoryTest {
|
|||||||
repository = QueueRepository(db)
|
repository = QueueRepository(db)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@After
|
||||||
|
fun tearDown() {
|
||||||
|
db.close()
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun wholeAlbumEntryResolvesEveryTrackInOrder() =
|
fun wholeAlbumEntryResolvesEveryTrackInOrder() =
|
||||||
runTest {
|
runTest {
|
||||||
|
|||||||
@@ -61,6 +61,7 @@ class SourceRepositoryTest {
|
|||||||
@After
|
@After
|
||||||
fun tearDown() {
|
fun tearDown() {
|
||||||
repository.close()
|
repository.close()
|
||||||
|
db.close()
|
||||||
}
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@@ -267,15 +268,9 @@ class SourceRepositoryTest {
|
|||||||
repository.setOfflineMode(true)
|
repository.setOfflineMode(true)
|
||||||
withTimeout(5_000) { repository.offline.first { it } }
|
withTimeout(5_000) { repository.offline.first { it } }
|
||||||
|
|
||||||
var ref: CoverArtRef? = repository.coverArt("art-1")
|
val ref = awaitCoverArt("art-1")
|
||||||
withTimeout(5_000) {
|
|
||||||
while (ref?.url?.startsWith("file:") != true) {
|
|
||||||
delay(10)
|
|
||||||
ref = repository.coverArt("art-1")
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
assertTrue("expected the stored file, was ${ref?.url}", ref?.url?.startsWith("file:") == true)
|
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(5_000) { repository.activeSourceId().first { it != null }!! }
|
||||||
|
|||||||
@@ -59,6 +59,7 @@ class SyncManagerTest {
|
|||||||
fun tearDown() {
|
fun tearDown() {
|
||||||
manager.close()
|
manager.close()
|
||||||
sourceRepository.close()
|
sourceRepository.close()
|
||||||
|
db.close()
|
||||||
server.shutdown()
|
server.shutdown()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -21,6 +21,7 @@ import kotlinx.coroutines.flow.Flow
|
|||||||
import kotlinx.coroutines.flow.first
|
import kotlinx.coroutines.flow.first
|
||||||
import kotlinx.coroutines.flow.flow
|
import kotlinx.coroutines.flow.flow
|
||||||
import kotlinx.coroutines.test.runTest
|
import kotlinx.coroutines.test.runTest
|
||||||
|
import org.junit.After
|
||||||
import org.junit.Assert.assertEquals
|
import org.junit.Assert.assertEquals
|
||||||
import org.junit.Before
|
import org.junit.Before
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
@@ -40,6 +41,11 @@ class SyncServiceTest {
|
|||||||
.build()
|
.build()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@After
|
||||||
|
fun tearDown() {
|
||||||
|
db.close()
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
fun syncInsertsAndPrunesRemovedRows() =
|
fun syncInsertsAndPrunesRemovedRows() =
|
||||||
runTest {
|
runTest {
|
||||||
|
|||||||
@@ -101,8 +101,10 @@ class PlaybackControllerTest {
|
|||||||
@After
|
@After
|
||||||
fun tearDown() {
|
fun tearDown() {
|
||||||
controller.close()
|
controller.close()
|
||||||
|
runBlocking { controller.awaitStopped() }
|
||||||
sources.close()
|
sources.close()
|
||||||
downloads.close()
|
downloads.close()
|
||||||
|
db.close()
|
||||||
dispatcher.close()
|
dispatcher.close()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -93,8 +93,10 @@ class DownloadsViewModelTest {
|
|||||||
fun tearDown() {
|
fun tearDown() {
|
||||||
runBlocking { viewModel.viewModelScope.coroutineContext[Job]?.cancelAndJoin() }
|
runBlocking { viewModel.viewModelScope.coroutineContext[Job]?.cancelAndJoin() }
|
||||||
controller.close()
|
controller.close()
|
||||||
|
runBlocking { controller.awaitStopped() }
|
||||||
downloadRepository.close()
|
downloadRepository.close()
|
||||||
sourceRepository.close()
|
sourceRepository.close()
|
||||||
|
db.close()
|
||||||
Dispatchers.resetMain()
|
Dispatchers.resetMain()
|
||||||
dispatcher.close()
|
dispatcher.close()
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -87,8 +87,10 @@ class QueueViewModelTest {
|
|||||||
fun tearDown() {
|
fun tearDown() {
|
||||||
runBlocking { viewModel.viewModelScope.coroutineContext[Job]?.cancelAndJoin() }
|
runBlocking { viewModel.viewModelScope.coroutineContext[Job]?.cancelAndJoin() }
|
||||||
controller.close()
|
controller.close()
|
||||||
|
runBlocking { controller.awaitStopped() }
|
||||||
downloads.close()
|
downloads.close()
|
||||||
sources.close()
|
sources.close()
|
||||||
|
db.close()
|
||||||
Dispatchers.resetMain()
|
Dispatchers.resetMain()
|
||||||
dispatcher.close()
|
dispatcher.close()
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -46,6 +46,7 @@ class AddSourceViewModelTest {
|
|||||||
private lateinit var downloadRepository: DownloadRepository
|
private lateinit var downloadRepository: DownloadRepository
|
||||||
private lateinit var server: MockWebServer
|
private lateinit var server: MockWebServer
|
||||||
private val createdViewModels = mutableListOf<AddSourceViewModel>()
|
private val createdViewModels = mutableListOf<AddSourceViewModel>()
|
||||||
|
private val syncManagers = mutableListOf<SyncManager>()
|
||||||
|
|
||||||
@Before
|
@Before
|
||||||
fun setUp() {
|
fun setUp() {
|
||||||
@@ -74,9 +75,11 @@ class AddSourceViewModelTest {
|
|||||||
@After
|
@After
|
||||||
fun tearDown() {
|
fun tearDown() {
|
||||||
createdViewModels.forEach { runBlocking { it.viewModelScope.coroutineContext[Job]?.cancelAndJoin() } }
|
createdViewModels.forEach { runBlocking { it.viewModelScope.coroutineContext[Job]?.cancelAndJoin() } }
|
||||||
|
syncManagers.forEach { it.close() }
|
||||||
server.shutdown()
|
server.shutdown()
|
||||||
downloadRepository.close()
|
downloadRepository.close()
|
||||||
sourceRepository.close()
|
sourceRepository.close()
|
||||||
|
db.close()
|
||||||
Dispatchers.resetMain()
|
Dispatchers.resetMain()
|
||||||
dispatcher.close()
|
dispatcher.close()
|
||||||
}
|
}
|
||||||
@@ -133,9 +136,10 @@ class AddSourceViewModelTest {
|
|||||||
assertNotNull("the active source must survive the refused delete", db.sourcesDao().sourceOnce(sourceId))
|
assertNotNull("the active source must survive the refused delete", db.sourcesDao().sourceOnce(sourceId))
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun viewModel(sourceId: Long? = null): AddSourceViewModel =
|
private fun viewModel(sourceId: Long? = null): AddSourceViewModel {
|
||||||
AddSourceViewModel(sourceRepository, SyncManager(db, sourceRepository, QueueRepository(db)), downloadRepository, sourceId)
|
val syncManager = SyncManager(db, sourceRepository, QueueRepository(db)).also { syncManagers += it }
|
||||||
.also { createdViewModels += it }
|
return AddSourceViewModel(sourceRepository, syncManager, downloadRepository, sourceId).also { createdViewModels += it }
|
||||||
|
}
|
||||||
|
|
||||||
private fun ok() = MockResponse().setBody("<subsonic-response status=\"ok\" version=\"1.16.1\"/>")
|
private fun ok() = MockResponse().setBody("<subsonic-response status=\"ok\" version=\"1.16.1\"/>")
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user