From f7acb5e1152ec781fd38cc1e06ce469aaac13574 Mon Sep 17 00:00:00 2001 From: darken Date: Sat, 27 Aug 2022 22:59:34 +0200 Subject: [PATCH] Fix copy paste regressions --- .../capod/common/flow/DynamicStateFlow.kt | 12 +- .../capod/common/flow/DynamicStateFlowTest.kt | 115 ++++++++++-------- .../testhelper/coroutine/TestExtensions.kt | 57 ++++----- app/src/test/java/testhelper/flow/FlowTest.kt | 17 +-- build.gradle.kts | 2 +- 5 files changed, 103 insertions(+), 100 deletions(-) diff --git a/app/src/main/java/eu/darken/capod/common/flow/DynamicStateFlow.kt b/app/src/main/java/eu/darken/capod/common/flow/DynamicStateFlow.kt index 74228899..d37c6513 100644 --- a/app/src/main/java/eu/darken/capod/common/flow/DynamicStateFlow.kt +++ b/app/src/main/java/eu/darken/capod/common/flow/DynamicStateFlow.kt @@ -25,7 +25,8 @@ class DynamicStateFlow( loggingTag: String? = null, parentScope: CoroutineScope, coroutineContext: CoroutineContext = parentScope.coroutineContext, - private val startValueProvider: suspend CoroutineScope.() -> T + private val onRelease: CoroutineScope.(T) -> Unit = {}, + private val startValueProvider: suspend CoroutineScope.() -> T, ) { private val lTag = loggingTag?.let { "$it:DSFlow" } @@ -47,6 +48,12 @@ class DynamicStateFlow( } } + invokeOnClose { + lTag?.let { log(it, VERBOSE) { "invokeOnClose executing..." } } + onRelease(currentValue) + lTag?.let { log(it, VERBOSE) { "internal channelFlow finished." } } + } + updateActions.collect { update -> currentValue = valueGuard.withLock { try { @@ -68,12 +75,11 @@ class DynamicStateFlow( } } } - - lTag?.let { log(it, VERBOSE) { "internal channelFlow finished." } } } private val internalFlow = producer .onStart { lTag?.let { log(it, VERBOSE) { "Internal onStart" } } } +// .onEach { value -> lTag?.let { log(it, VERBOSE) { "New value: $value" } } } .onCompletion { err -> when { err is CancellationException -> { diff --git a/app/src/test/java/eu/darken/capod/common/flow/DynamicStateFlowTest.kt b/app/src/test/java/eu/darken/capod/common/flow/DynamicStateFlowTest.kt index f2fc3f65..2106c74e 100644 --- a/app/src/test/java/eu/darken/capod/common/flow/DynamicStateFlowTest.kt +++ b/app/src/test/java/eu/darken/capod/common/flow/DynamicStateFlowTest.kt @@ -11,16 +11,15 @@ import io.mockk.mockk import kotlinx.coroutines.* import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.firstOrNull -import kotlinx.coroutines.test.TestCoroutineDispatcher -import kotlinx.coroutines.test.TestCoroutineExceptionHandler -import kotlinx.coroutines.test.createTestCoroutineScope +import kotlinx.coroutines.test.* import org.junit.jupiter.api.Test import testhelper.BaseTest -import testhelper.coroutine.runBlockingTest2 +import testhelper.coroutine.runTest2 import testhelper.flow.test import java.io.IOException import java.lang.Thread.sleep import kotlin.concurrent.thread +import kotlin.coroutines.EmptyCoroutineContext class DynamicStateFlowTest : BaseTest() { @@ -62,7 +61,7 @@ class DynamicStateFlowTest : BaseTest() { ) testScope.apply { - runBlockingTest2(allowUncompleted = true) { + runTest2(autoCancel = true) { hotData.flow.first() shouldBe "Test" hotData.flow.first() shouldBe "Test" } @@ -83,7 +82,7 @@ class DynamicStateFlowTest : BaseTest() { startValueProvider = valueProvider, ) - val testCollector = hotData.flow.test(startOnScope = testScope) + val testCollector = hotData.flow.test(scope = testScope) testCollector.silent = true (1..16).forEach { _ -> @@ -123,7 +122,7 @@ class DynamicStateFlowTest : BaseTest() { startValueProvider = valueProvider, ) - val testCollector = hotData.flow.test(startOnScope = testScope) + val testCollector = hotData.flow.test(scope = testScope) testCollector.silent = true (1..10).forEach { _ -> @@ -159,7 +158,7 @@ class DynamicStateFlowTest : BaseTest() { startValueProvider = { "1" }, ) - val testCollector = hotData.flow.test(startOnScope = testScope) + val testCollector = hotData.flow.test(scope = testScope) testCollector.silent = true hotData.updateAsync { "1" } @@ -174,40 +173,39 @@ class DynamicStateFlowTest : BaseTest() { } @Test - fun `multiple subscribers share the flow`() { - val testScope = - createTestCoroutineScope(TestCoroutineDispatcher() + TestCoroutineExceptionHandler() + EmptyCoroutineContext) + fun `multiple subscribers share the flow`() = runTest2(autoCancel = true) { val valueProvider = mockk String>() coEvery { valueProvider.invoke(any()) } returns "Test" val hotData = DynamicStateFlow( loggingTag = "tag", - parentScope = testScope, + parentScope = this, startValueProvider = valueProvider, ) - testScope.runBlockingTest2(allowUncompleted = true) { - val sub1 = hotData.flow.test(tag = "sub1", startOnScope = this) - val sub2 = hotData.flow.test(tag = "sub2", startOnScope = this) - val sub3 = hotData.flow.test(tag = "sub3", startOnScope = this) + val sub1 = hotData.flow.test(tag = "sub1", scope = this) + val sub2 = hotData.flow.test(tag = "sub2", scope = this) + val sub3 = hotData.flow.test(tag = "sub3", scope = this) - hotData.updateAsync { "A" } - hotData.updateAsync { "B" } - hotData.updateAsync { "C" } + hotData.updateAsync { "A" } + hotData.updateAsync { "B" } + hotData.updateAsync { "C" } - listOf(sub1, sub2, sub3).forEach { - it.await { list, _ -> list.size == 4 } - it.latestValues shouldBe listOf("Test", "A", "B", "C") - it.cancel() - } + advanceUntilIdle() - hotData.flow.first() shouldBe "C" + listOf(sub1, sub2, sub3).forEach { + it.await { list, _ -> list.size == 4 } + it.latestValues shouldBe listOf("Test", "A", "B", "C") + it.cancelAndJoin() } + + hotData.flow.first() shouldBe "C" + coVerify(exactly = 1) { valueProvider.invoke(any()) } } @Test - fun `value is persisted between unsubscribes`() = runBlockingTest2(allowUncompleted = true) { + fun `value is persisted between unsubscribes`() = runTest2(autoCancel = true) { val valueProvider = mockk Long>() coEvery { valueProvider.invoke(any()) } returns 1 @@ -218,7 +216,7 @@ class DynamicStateFlowTest : BaseTest() { startValueProvider = valueProvider, ) - val testCollector1 = hotData.flow.test(tag = "collector1", startOnScope = this) + val testCollector1 = hotData.flow.test(tag = "collector1", scope = this) testCollector1.silent = false (1..10).forEach { _ -> @@ -232,16 +230,14 @@ class DynamicStateFlowTest : BaseTest() { testCollector1.await { list, _ -> list.size == 11 } testCollector1.latestValues shouldBe (1L..11L).toList() - testCollector1.cancel() - testCollector1.awaitFinal() + testCollector1.cancelAndJoin() - val testCollector2 = hotData.flow.test(tag = "collector2", startOnScope = this) + val testCollector2 = hotData.flow.test(tag = "collector2", scope = this) testCollector2.silent = false advanceUntilIdle() - testCollector2.cancel() - testCollector2.awaitFinal() + testCollector2.cancelAndJoin() testCollector2.latestValues shouldBe listOf(11L) @@ -267,7 +263,7 @@ class DynamicStateFlowTest : BaseTest() { this + 1 } - val testCollector = hotData.flow.test(startOnScope = testScope) + val testCollector = hotData.flow.test(scope = testScope) testScope.advanceUntilIdle() @@ -276,7 +272,7 @@ class DynamicStateFlowTest : BaseTest() { testCollector.await { _, i -> i == 3 } testCollector.latestValues shouldBe listOf(2, 3, 0) - testCollector.cancel() + testCollector.cancelAndJoin() } @Test @@ -293,7 +289,7 @@ class DynamicStateFlowTest : BaseTest() { }, ) - val testCollector = hotData.flow.test(startOnScope = testScope) + val testCollector = hotData.flow.test(scope = testScope) testScope.advanceUntilIdle() @@ -307,30 +303,23 @@ class DynamicStateFlowTest : BaseTest() { testScope.uncaughtExceptions.singleOrNull() shouldBe null - testCollector.cancel() + testCollector.cancelAndJoin() } @Test - fun `async updates error handler`() { - val testScope = - createTestCoroutineScope(TestCoroutineDispatcher() + TestCoroutineExceptionHandler() + EmptyCoroutineContext) - + fun `async updates error handler`() = runTest2(expectedError = IOException::class) { val hotData = DynamicStateFlow( loggingTag = "tag", - parentScope = testScope, + parentScope = this, startValueProvider = { 1 }, ) - val testCollector = hotData.flow.test(startOnScope = testScope) - testScope.advanceUntilIdle() + val testCollector = hotData.flow.test(scope = this) + advanceUntilIdle() hotData.updateAsync { throw IOException("Surprise") } - testScope.advanceUntilIdle() - - testScope.uncaughtExceptions.single() shouldBe instanceOf(IOException::class) - - testCollector.cancel() + advanceUntilIdle() } @Test @@ -344,7 +333,7 @@ class DynamicStateFlowTest : BaseTest() { startValueProvider = { 1 }, ) - val testCollector = hotData.flow.test(startOnScope = testScope) + val testCollector = hotData.flow.test(scope = testScope) testScope.advanceUntilIdle() var thrownError: Exception? = null @@ -358,6 +347,30 @@ class DynamicStateFlowTest : BaseTest() { thrownError!!.shouldBeInstanceOf() testScope.uncaughtExceptions.singleOrNull() shouldBe null - testCollector.cancel() + testCollector.cancelAndJoin() } -} + + @Test + fun `clean up function is called when parent scope is cancelled`() = runTest { + val testScope = + createTestCoroutineScope(TestCoroutineDispatcher() + TestCoroutineExceptionHandler() + EmptyCoroutineContext) + + var onReleaseValue: String? = null + + val hotData = DynamicStateFlow( + loggingTag = "tag", + parentScope = testScope, + coroutineContext = Dispatchers.Unconfined, + startValueProvider = { "Test" }, + onRelease = { + onReleaseValue = it + } + ) + + hotData.flow.first() shouldBe "Test" + + testScope.cancel() + + onReleaseValue shouldBe "Test" + } +} \ No newline at end of file diff --git a/app/src/test/java/testhelper/coroutine/TestExtensions.kt b/app/src/test/java/testhelper/coroutine/TestExtensions.kt index 72a00a8e..0ad92a04 100644 --- a/app/src/test/java/testhelper/coroutine/TestExtensions.kt +++ b/app/src/test/java/testhelper/coroutine/TestExtensions.kt @@ -1,49 +1,38 @@ package testhelper.coroutine -import eu.darken.capod.common.debug.logging.log -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.test.TestCoroutineScope -import kotlinx.coroutines.test.UncompletedCoroutinesError -import kotlinx.coroutines.test.runBlockingTest +import eu.darken.capod.common.debug.logging.asLog +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.cancel +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.runTest import kotlin.coroutines.CoroutineContext import kotlin.coroutines.EmptyCoroutineContext +import kotlin.reflect.KClass -/** - * If you have a test that uses a coroutine that never stops, you may use this. - */ - -@ExperimentalCoroutinesApi // Since 1.2.1, tentatively till 1.3.0 -fun TestCoroutineScope.runBlockingTest2( - allowUncompleted: Boolean = false, - block: suspend TestCoroutineScope.() -> Unit -): Unit = runBlockingTest2( - allowUncompleted = allowUncompleted, - context = coroutineContext, - testBody = block -) - -fun runBlockingTest2( - allowUncompleted: Boolean = false, +fun runTest2( + autoCancel: Boolean = false, context: CoroutineContext = EmptyCoroutineContext, - testBody: suspend TestCoroutineScope.() -> Unit + expectedError: KClass? = null, + testBody: suspend TestScope.() -> Unit ) { try { - runBlocking { - try { - runBlockingTest( - context = context, - testBody = testBody - ) - } catch (e: UncompletedCoroutinesError) { - if (!allowUncompleted) throw e - else log { "Ignoring active job." } + val scope = TestScope(context = context) + try { + scope.runTest { + testBody() + if (autoCancel) scope.cancel("autoCancel") } + } catch (e: Throwable) { + val isExpected = expectedError?.isInstance(e) ?: false + if (!isExpected) throw e } - } catch (e: Exception) { - if (!allowUncompleted || (e.message != "This job has not completed yet")) { + } catch (e: CancellationException) { + if (e.message == "autoCancel" && autoCancel) { + io.kotest.mpp.log { "Test was auto-cancelled ${e.asLog()}" } + } else { throw e } } } + diff --git a/app/src/test/java/testhelper/flow/FlowTest.kt b/app/src/test/java/testhelper/flow/FlowTest.kt index 4730ae04..39e7cfc7 100644 --- a/app/src/test/java/testhelper/flow/FlowTest.kt +++ b/app/src/test/java/testhelper/flow/FlowTest.kt @@ -8,14 +8,11 @@ import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.flow.* import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock -import kotlinx.coroutines.test.TestCoroutineDispatcher -import kotlinx.coroutines.test.TestCoroutineExceptionHandler -import kotlinx.coroutines.test.createTestCoroutineScope fun Flow.test( tag: String? = null, - startOnScope: CoroutineScope = createTestCoroutineScope(TestCoroutineDispatcher() + TestCoroutineExceptionHandler() + EmptyCoroutineContext) -): TestCollector = createTest(tag ?: "FlowTest").start(scope = startOnScope) + scope: CoroutineScope +): TestCollector = createTest(tag ?: "FlowTest").start(scope = scope) fun Flow.createTest( tag: String? = null @@ -80,7 +77,7 @@ class TestCollector( } suspend fun awaitFinal(cancel: Boolean = false) = apply { - if (cancel) cancel() + if (cancel) job.cancel() try { job.join() } catch (e: Exception) { @@ -93,11 +90,9 @@ class TestCollector( require(error == null) { "Error was not null: $error" } } - fun cancel() { + suspend fun cancelAndJoin() { if (job.isCompleted) throw IllegalStateException("Flow is already canceled.") - runBlocking { - job.cancelAndJoin() - } + job.cancelAndJoin() } -} +} \ No newline at end of file diff --git a/build.gradle.kts b/build.gradle.kts index dfe7f0f2..e41d23bc 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -4,7 +4,7 @@ buildscript { mavenCentral() } dependencies { - classpath("com.android.tools.build:gradle:${Versions.Gradle.buildTools}") + classpath("com.android.tools.build:gradle:7.2.2") classpath("org.jetbrains.kotlin:kotlin-gradle-plugin:${Versions.Kotlin.core}") classpath("com.google.dagger:hilt-android-gradle-plugin:${Versions.Dagger.core}") classpath("androidx.navigation:navigation-safe-args-gradle-plugin:${Versions.AndroidX.Navigation.core}")