From 45453522bbb4e058ca89023ed4ac1fb8bf18d27f Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Wed, 20 Oct 2021 11:02:12 +0300 Subject: [PATCH 1/7] Cancel the child coroutines if the test body has thrown Fixes #1910 --- .../common/src/TestBuilders.kt | 32 ++++++++++-- .../common/src/TestCoroutineScheduler.kt | 24 ++++++--- .../common/test/RunTestTest.kt | 50 +++++++++++++++++++ 3 files changed, 95 insertions(+), 11 deletions(-) diff --git a/kotlinx-coroutines-test/common/src/TestBuilders.kt b/kotlinx-coroutines-test/common/src/TestBuilders.kt index c60a5155f5..080ee45d2c 100644 --- a/kotlinx-coroutines-test/common/src/TestBuilders.kt +++ b/kotlinx-coroutines-test/common/src/TestBuilders.kt @@ -141,6 +141,18 @@ public expect class TestResult * * ### Failures * + * #### Test body failures + * + * If the test body finishes with an exception, then this exception will be thrown at the end of the test. Additionally, + * to prevent child coroutines getting stuck, the whole scope will be cancelled in this case. + * + * #### Reported exceptions + * + * Exceptions reported to the test coroutine scope via [TestCoroutineScope.reportException] will be thrown at the end. + * By default (without passing an explicit [TestExceptionHandler]), this includes all unhandled exceptions. + * + * #### Uncompleted coroutines + * * This method requires that all coroutines launched inside [testBody] complete, or are cancelled. Otherwise, the test * will be failed (which, on JVM and Native, means that [runTest] itself will throw [AssertionError], * whereas on JS, the `Promise` will fail with it). @@ -151,8 +163,6 @@ public expect class TestResult * idle before throwing [AssertionError]. If some dispatcher linked to [TestCoroutineScheduler] receives a * task during that time, the timer gets reset. * - * Unhandled exceptions thrown by coroutines in the test will be rethrown at the end of the test. - * * ### Configuration * * [context] can be used to affect the environment of the code under test. Beside just being passed to the coroutine @@ -178,7 +188,23 @@ public fun runTest( } var completed = false while (!completed) { - scheduler.advanceUntilIdle() + while (scheduler.tryRunNextTask()) { + if (deferred.isCompleted && deferred.getCompletionExceptionOrNull() != null && testScope.isActive) { + /** + * Here, we already know how the test will finish: it will throw + * [Deferred.getCompletionExceptionOrNull]. Therefore, we won't care if there are uncompleted jobs, + * and may as well just exit right here. However, in order to lower the surprise factor, we + * cancel the child jobs here and wait for them to finish instead of dropping them: there could be + * some cleanup procedures involved, and not having finalizers run could mean leaking resources. + * + * Another approach to take if this turns out not to be enough and some child jobs still fail is to + * only make at most a fixed number of [TestCoroutineScheduler.tryRunNextTask] once we detect the + * failure with which the test will finish. This has the downside that there is still some + * negligible risk of not running the finalizers. + */ + testScope.cancel() + } + } if (deferred.isCompleted) { /* don't even enter `withTimeout`; this allows to use a timeout of zero to check that there are no non-trivial dispatches. */ diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt b/kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt index 00c6975cc8..42d12e47e0 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt @@ -81,6 +81,21 @@ public class TestCoroutineScheduler : AbstractCoroutineContextElement(TestCorout } } + /** + * Runs the next enqueued task, advancing the virtual time to the time of its scheduled awakening. + */ + internal fun tryRunNextTask(): Boolean { + val event = synchronized(lock) { + val event = events.removeFirstOrNull() ?: return false + if (currentTime > event.time) + currentTimeAheadOfEvents() + currentTime = event.time + event + } + event.dispatcher.processEvent(event.time, event.marker) + return true + } + /** * Runs the enqueued tasks in the specified order, advancing the virtual time as needed until there are no more * tasks associated with the dispatchers linked to this scheduler. @@ -92,14 +107,7 @@ public class TestCoroutineScheduler : AbstractCoroutineContextElement(TestCorout @ExperimentalCoroutinesApi public fun advanceUntilIdle() { while (!events.isEmpty) { - val event = synchronized(lock) { - val event = events.removeFirstOrNull() ?: return - if (currentTime > event.time) - currentTimeAheadOfEvents() - currentTime = event.time - event - } - event.dispatcher.processEvent(event.time, event.marker) + tryRunNextTask() } } diff --git a/kotlinx-coroutines-test/common/test/RunTestTest.kt b/kotlinx-coroutines-test/common/test/RunTestTest.kt index 68c9ab9eb5..686a9b24a0 100644 --- a/kotlinx-coroutines-test/common/test/RunTestTest.kt +++ b/kotlinx-coroutines-test/common/test/RunTestTest.kt @@ -153,4 +153,54 @@ class RunTestTest { } } + @Test + fun reproducer2405() = runTest { + val dispatcher = StandardTestDispatcher(testScheduler) + var collectedError = false + withContext(dispatcher) { + flow { emit(1) } + .combine( + flow { throw IllegalArgumentException() } + ) { int, string -> int.toString() + string } + .catch { emit("error") } + .collect { + assertEquals("error", it) + collectedError = true + } + } + assertTrue(collectedError) + } + + /** Tests that, once the test body has thrown, the child coroutines are cancelled. */ + @Test + fun testChildrenCancellationOnTestBodyFailure() { + var job: Job? = null + testResultMap({ + assertFailsWith { it() } + assertTrue(job!!.isCancelled) + }, { + runTest { + job = launch { + while (true) { + delay(1000) + } + } + throw AssertionError() + } + }) + } + + /** Tests that [runTest] reports [TimeoutCancellationException]. */ + @Test + fun testTimeout() = testResultMap({ + assertFailsWith { it() } + }, { + runTest { + withTimeout(50) { + launch { + delay(1000) + } + } + } + }) } From 304dd7820f26c67281f87b08f7b1772285c8e61c Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 25 Oct 2021 13:56:08 +0300 Subject: [PATCH 2/7] Don't lose reported exceptions if the test body has thrown --- kotlinx-coroutines-test/common/src/TestBuilders.kt | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/kotlinx-coroutines-test/common/src/TestBuilders.kt b/kotlinx-coroutines-test/common/src/TestBuilders.kt index 080ee45d2c..fb33458b6f 100644 --- a/kotlinx-coroutines-test/common/src/TestBuilders.kt +++ b/kotlinx-coroutines-test/common/src/TestBuilders.kt @@ -232,6 +232,13 @@ public fun runTest( } } deferred.getCompletionExceptionOrNull()?.let { + try { + testScope.cleanupTestCoroutines() + } catch (e: UncompletedCoroutinesError) { + // it's normal that some jobs are not completed if the test body has failed, won't clutter the output + } catch (e: Throwable) { + e.printStackTrace() + } throw it } testScope.cleanupTestCoroutines() From 9835932bed4130fbfc90ad44e685503eda3b9ce7 Mon Sep 17 00:00:00 2001 From: dkhalanskyjb <52952525+dkhalanskyjb@users.noreply.github.com> Date: Mon, 25 Oct 2021 19:29:48 +0300 Subject: [PATCH 3/7] Structured concurrency in runTest (#3000) --- .../common/src/TestBuilders.kt | 47 ++++++------- .../common/src/TestCoroutineScope.kt | 2 +- .../common/test/RunTestTest.kt | 12 ++++ .../common/test/TestCoroutineScopeTest.kt | 68 +++++++++++++++++++ 4 files changed, 102 insertions(+), 27 deletions(-) diff --git a/kotlinx-coroutines-test/common/src/TestBuilders.kt b/kotlinx-coroutines-test/common/src/TestBuilders.kt index fb33458b6f..a773cfe1bb 100644 --- a/kotlinx-coroutines-test/common/src/TestBuilders.kt +++ b/kotlinx-coroutines-test/common/src/TestBuilders.kt @@ -44,7 +44,7 @@ import kotlin.coroutines.* */ @Deprecated("Use `runTest` instead to support completing from other dispatchers.", level = DeprecationLevel.WARNING) public fun runBlockingTest(context: CoroutineContext = EmptyCoroutineContext, testBody: suspend TestCoroutineScope.() -> Unit) { - val scope = TestCoroutineScope(context) + val scope = TestCoroutineScope(TestCoroutineDispatcher() + SupervisorJob() + context) val scheduler = scope.testScheduler val deferred = scope.async { scope.testBody() @@ -180,32 +180,16 @@ public fun runTest( ): TestResult { if (context[RunningInRunTest] != null) throw IllegalStateException("Calls to `runTest` can't be nested. Please read the docs on `TestResult` for details.") - val testScope = TestCoroutineScope(context + RunningInRunTest()) + val testScope = TestBodyCoroutine(TestCoroutineScope(context + RunningInRunTest())) val scheduler = testScope.testScheduler + testScope.start(CoroutineStart.DEFAULT, testScope) { + testBody() + } return createTestResult { - val deferred = testScope.async { - testScope.testBody() - } var completed = false while (!completed) { - while (scheduler.tryRunNextTask()) { - if (deferred.isCompleted && deferred.getCompletionExceptionOrNull() != null && testScope.isActive) { - /** - * Here, we already know how the test will finish: it will throw - * [Deferred.getCompletionExceptionOrNull]. Therefore, we won't care if there are uncompleted jobs, - * and may as well just exit right here. However, in order to lower the surprise factor, we - * cancel the child jobs here and wait for them to finish instead of dropping them: there could be - * some cleanup procedures involved, and not having finalizers run could mean leaking resources. - * - * Another approach to take if this turns out not to be enough and some child jobs still fail is to - * only make at most a fixed number of [TestCoroutineScheduler.tryRunNextTask] once we detect the - * failure with which the test will finish. This has the downside that there is still some - * negligible risk of not running the finalizers. - */ - testScope.cancel() - } - } - if (deferred.isCompleted) { + scheduler.advanceUntilIdle() + if (testScope.isCompleted) { /* don't even enter `withTimeout`; this allows to use a timeout of zero to check that there are no non-trivial dispatches. */ completed = true @@ -214,7 +198,7 @@ public fun runTest( try { withTimeout(dispatchTimeoutMs) { select { - deferred.onAwait { + testScope.onJoin { completed = true } scheduler.onDispatchEvent { @@ -231,7 +215,7 @@ public fun runTest( throw UncompletedCoroutinesError("The test coroutine was not completed after waiting for $dispatchTimeoutMs ms") } } - deferred.getCompletionExceptionOrNull()?.let { + testScope.getCompletionExceptionOrNull()?.let { try { testScope.cleanupTestCoroutines() } catch (e: UncompletedCoroutinesError) { @@ -255,7 +239,7 @@ internal expect fun createTestResult(testProcedure: suspend () -> Unit): TestRes * Runs a test in a [TestCoroutineScope] based on this one. * * Calls [runTest] using a coroutine context from this [TestCoroutineScope]. The [TestCoroutineScope] used to run - * [block] will be different from this one, but will reuse its [Job]; therefore, even if calling + * [block] will be different from this one, but will use its [Job] as a parent; therefore, even if calling * [TestCoroutineScope.cleanupTestCoroutines] on this scope were to complete its job, [runTest] won't complete it at the * end of the test. * @@ -292,3 +276,14 @@ private class RunningInRunTest: AbstractCoroutineContextElement(RunningInRunTest /** The default timeout to use when waiting for asynchronous completions of the coroutines managed by * a [TestCoroutineScheduler]. */ private const val DEFAULT_DISPATCH_TIMEOUT_MS = 10_000L + +private class TestBodyCoroutine( + private val testScope: TestCoroutineScope, +) : AbstractCoroutine(testScope.coroutineContext, initParentJob = true, active = true), TestCoroutineScope +{ + override val testScheduler get() = testScope.testScheduler + + override fun cleanupTestCoroutines() = testScope.cleanupTestCoroutines() + + override fun reportException(throwable: Throwable) = testScope.reportException(throwable) +} diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt index 69af8cd764..2ea9b04d33 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt @@ -115,7 +115,7 @@ public fun TestCoroutineScope(context: CoroutineContext = EmptyCoroutineContext) val job: Job val ownJob: CompletableJob? if (context[Job] == null) { - ownJob = SupervisorJob() + ownJob = Job() job = ownJob } else { ownJob = null diff --git a/kotlinx-coroutines-test/common/test/RunTestTest.kt b/kotlinx-coroutines-test/common/test/RunTestTest.kt index 686a9b24a0..4eb26e454c 100644 --- a/kotlinx-coroutines-test/common/test/RunTestTest.kt +++ b/kotlinx-coroutines-test/common/test/RunTestTest.kt @@ -203,4 +203,16 @@ class RunTestTest { } } }) + + /** Checks that [runTest] throws the root cause and not [JobCancellationException] when a child coroutine throws. */ + @Test + fun testRunTestThrowsRootCause() = testResultMap({ + assertFailsWith { it() } + }, { + runTest { + launch { + throw TestException() + } + } + }) } diff --git a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt index e31ed7aee7..a900cc64f9 100644 --- a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt +++ b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt @@ -120,6 +120,74 @@ class TestCoroutineScopeTest { assertFalse(handlerCalled) } + /** Tests that uncaught exceptions are thrown at the cleanup. */ + @Test + fun testThrowsUncaughtExceptionsOnCleanup() { + val scope = createTestCoroutineScope() + val exception = TestException("test") + scope.launch { + throw exception + } + assertFailsWith { + scope.cleanupTestCoroutines() + } + } + + /** Tests that uncaught exceptions take priority over uncompleted jobs when throwing on cleanup. */ + @Test + fun testUncaughtExceptionsPrioritizedOnCleanup() { + val scope = createTestCoroutineScope() + val exception = TestException("test") + scope.launch { + throw exception + } + scope.launch { + delay(1000) + } + assertFailsWith { + scope.cleanupTestCoroutines() + } + } + + /** Tests that cleaning up twice is forbidden. */ + @Test + fun testClosingTwice() { + val scope = createTestCoroutineScope() + scope.cleanupTestCoroutines() + assertFailsWith { + scope.cleanupTestCoroutines() + } + } + + /** Tests that throwing after cleaning up is forbidden. */ + @Test + fun testReportingAfterClosing() { + val scope = createTestCoroutineScope() + scope.cleanupTestCoroutines() + assertFailsWith { + scope.reportException(TestException()) + } + } + + /** Tests that, when reporting several exceptions, the first one is thrown, with the rest suppressed. */ + @Test + fun testSuppressedExceptions() { + createTestCoroutineScope().apply { + reportException(TestException("x")) + reportException(TestException("y")) + reportException(TestException("z")) + try { + cleanupTestCoroutines() + fail("should not be reached") + } catch (e: TestException) { + assertEquals("x", e.message) + assertEquals(2, e.suppressedExceptions.size) + assertEquals("y", e.suppressedExceptions[0].message) + assertEquals("z", e.suppressedExceptions[1].message) + } + } + } + companion object { internal val invalidContexts = listOf( Dispatchers.Default, // not a [TestDispatcher] From cd79aea8bb4cca1235c17eff35fc5efcbfd07262 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 25 Oct 2021 19:35:17 +0300 Subject: [PATCH 4/7] Fixes --- kotlinx-coroutines-test/common/src/TestBuilders.kt | 5 ++--- kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/kotlinx-coroutines-test/common/src/TestBuilders.kt b/kotlinx-coroutines-test/common/src/TestBuilders.kt index a773cfe1bb..88a102a20e 100644 --- a/kotlinx-coroutines-test/common/src/TestBuilders.kt +++ b/kotlinx-coroutines-test/common/src/TestBuilders.kt @@ -143,13 +143,12 @@ public expect class TestResult * * #### Test body failures * - * If the test body finishes with an exception, then this exception will be thrown at the end of the test. Additionally, - * to prevent child coroutines getting stuck, the whole scope will be cancelled in this case. + * If the test body finishes with an exception, then this exception will be thrown at the end of the test. * * #### Reported exceptions * * Exceptions reported to the test coroutine scope via [TestCoroutineScope.reportException] will be thrown at the end. - * By default (without passing an explicit [TestExceptionHandler]), this includes all unhandled exceptions. + * By default, unless an explicit [TestExceptionHandler] is passed, this includes all unhandled exceptions. * * #### Uncompleted coroutines * diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt b/kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt index 42d12e47e0..2acd8e527f 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt @@ -84,7 +84,7 @@ public class TestCoroutineScheduler : AbstractCoroutineContextElement(TestCorout /** * Runs the next enqueued task, advancing the virtual time to the time of its scheduled awakening. */ - internal fun tryRunNextTask(): Boolean { + private fun tryRunNextTask(): Boolean { val event = synchronized(lock) { val event = events.removeFirstOrNull() ?: return false if (currentTime > event.time) @@ -106,7 +106,7 @@ public class TestCoroutineScheduler : AbstractCoroutineContextElement(TestCorout */ @ExperimentalCoroutinesApi public fun advanceUntilIdle() { - while (!events.isEmpty) { + while (!synchronized(lock) { events.isEmpty }) { tryRunNextTask() } } From 22306af5e56d019f202805cf99b6d67ccb4c88a2 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Tue, 26 Oct 2021 11:46:52 +0300 Subject: [PATCH 5/7] Suppress reported exceptions in runTest --- .../common/src/TestBuilders.kt | 5 ++-- .../common/test/RunTestTest.kt | 24 +++++++++++++++++++ 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/kotlinx-coroutines-test/common/src/TestBuilders.kt b/kotlinx-coroutines-test/common/src/TestBuilders.kt index 88a102a20e..78c05b3006 100644 --- a/kotlinx-coroutines-test/common/src/TestBuilders.kt +++ b/kotlinx-coroutines-test/common/src/TestBuilders.kt @@ -148,7 +148,8 @@ public expect class TestResult * #### Reported exceptions * * Exceptions reported to the test coroutine scope via [TestCoroutineScope.reportException] will be thrown at the end. - * By default, unless an explicit [TestExceptionHandler] is passed, this includes all unhandled exceptions. + * By default, unless an explicit [TestExceptionHandler] is passed, this includes all unhandled exceptions. If the test + * body also fails, the reported exceptions are suppressed by it. * * #### Uncompleted coroutines * @@ -220,7 +221,7 @@ public fun runTest( } catch (e: UncompletedCoroutinesError) { // it's normal that some jobs are not completed if the test body has failed, won't clutter the output } catch (e: Throwable) { - e.printStackTrace() + it.addSuppressed(e) } throw it } diff --git a/kotlinx-coroutines-test/common/test/RunTestTest.kt b/kotlinx-coroutines-test/common/test/RunTestTest.kt index 4eb26e454c..47c1a1b0ea 100644 --- a/kotlinx-coroutines-test/common/test/RunTestTest.kt +++ b/kotlinx-coroutines-test/common/test/RunTestTest.kt @@ -215,4 +215,28 @@ class RunTestTest { } } }) + + /** Tests that, when the test body fails, the reported exceptions are suppressed. */ + @Test + fun testSuppressedExceptions() = testResultMap({ + try { + it() + fail("should not be reached") + } catch (e: TestException) { + assertEquals("w", e.message) + val suppressed = e.suppressedExceptions + + (e.suppressedExceptions.firstOrNull()?.suppressedExceptions ?: emptyList()) + assertEquals(3, suppressed.size) + assertEquals("x", suppressed[0].message) + assertEquals("y", suppressed[1].message) + assertEquals("z", suppressed[2].message) + } + }, { + runTest { + reportException(TestException("x")) + reportException(TestException("y")) + reportException(TestException("z")) + throw TestException("w") + } + }) } From 48bf565dc6054e1de2f9ec89b07dd36765c82ceb Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Wed, 27 Oct 2021 11:36:43 +0300 Subject: [PATCH 6/7] Fix build after rebasing --- .../common/src/TestBuilders.kt | 12 +++--- .../common/src/TestCoroutineScope.kt | 2 +- .../common/test/Helpers.kt | 4 +- .../common/test/RunTestTest.kt | 26 +---------- .../common/test/TestCoroutineScopeTest.kt | 43 +------------------ .../common/test/TestRunBlockingTest.kt | 4 +- 6 files changed, 16 insertions(+), 75 deletions(-) diff --git a/kotlinx-coroutines-test/common/src/TestBuilders.kt b/kotlinx-coroutines-test/common/src/TestBuilders.kt index 78c05b3006..70a1a5643e 100644 --- a/kotlinx-coroutines-test/common/src/TestBuilders.kt +++ b/kotlinx-coroutines-test/common/src/TestBuilders.kt @@ -182,10 +182,12 @@ public fun runTest( throw IllegalStateException("Calls to `runTest` can't be nested. Please read the docs on `TestResult` for details.") val testScope = TestBodyCoroutine(TestCoroutineScope(context + RunningInRunTest())) val scheduler = testScope.testScheduler - testScope.start(CoroutineStart.DEFAULT, testScope) { - testBody() - } return createTestResult { + /** TODO: moving this [AbstractCoroutine.start] call outside [createTestResult] fails on Native with + * [TestCoroutineDispatcher], because the event loop is not started. */ + testScope.start(CoroutineStart.DEFAULT, testScope) { + testBody() + } var completed = false while (!completed) { scheduler.advanceUntilIdle() @@ -279,11 +281,11 @@ private const val DEFAULT_DISPATCH_TIMEOUT_MS = 10_000L private class TestBodyCoroutine( private val testScope: TestCoroutineScope, -) : AbstractCoroutine(testScope.coroutineContext, initParentJob = true, active = true), TestCoroutineScope +) : AbstractCoroutine(testScope.coroutineContext, initParentJob = true, active = true), TestCoroutineScope, + UncaughtExceptionCaptor by testScope.coroutineContext.uncaughtExceptionCaptor { override val testScheduler get() = testScope.testScheduler override fun cleanupTestCoroutines() = testScope.cleanupTestCoroutines() - override fun reportException(throwable: Throwable) = testScope.reportException(throwable) } diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt index 2ea9b04d33..c10d5d982c 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt @@ -124,7 +124,7 @@ public fun TestCoroutineScope(context: CoroutineContext = EmptyCoroutineContext) return TestCoroutineScopeImpl(context + scheduler + dispatcher + exceptionHandler + job, ownJob) } -private inline val CoroutineContext.uncaughtExceptionCaptor: UncaughtExceptionCaptor +internal inline val CoroutineContext.uncaughtExceptionCaptor: UncaughtExceptionCaptor get() { val handler = this[CoroutineExceptionHandler] return handler as? UncaughtExceptionCaptor ?: throw IllegalArgumentException( diff --git a/kotlinx-coroutines-test/common/test/Helpers.kt b/kotlinx-coroutines-test/common/test/Helpers.kt index 453472ad81..8be3ea106a 100644 --- a/kotlinx-coroutines-test/common/test/Helpers.kt +++ b/kotlinx-coroutines-test/common/test/Helpers.kt @@ -32,4 +32,6 @@ inline fun assertRunsFast(block: () -> T): T = assertRunsFast(Duration.secon /** * Passes [test] as an argument to [block], but as a function returning not a [TestResult] but [Unit]. */ -expect fun testResultMap(block: (() -> Unit) -> Unit, test: () -> TestResult): TestResult \ No newline at end of file +expect fun testResultMap(block: (() -> Unit) -> Unit, test: () -> TestResult): TestResult + +class TestException(message: String? = null): Exception(message) diff --git a/kotlinx-coroutines-test/common/test/RunTestTest.kt b/kotlinx-coroutines-test/common/test/RunTestTest.kt index 47c1a1b0ea..e086fea9ea 100644 --- a/kotlinx-coroutines-test/common/test/RunTestTest.kt +++ b/kotlinx-coroutines-test/common/test/RunTestTest.kt @@ -5,6 +5,7 @@ package kotlinx.coroutines.test import kotlinx.coroutines.* +import kotlinx.coroutines.flow.* import kotlin.coroutines.* import kotlin.test.* @@ -155,7 +156,7 @@ class RunTestTest { @Test fun reproducer2405() = runTest { - val dispatcher = StandardTestDispatcher(testScheduler) + val dispatcher = TestCoroutineDispatcher(testScheduler) var collectedError = false withContext(dispatcher) { flow { emit(1) } @@ -216,27 +217,4 @@ class RunTestTest { } }) - /** Tests that, when the test body fails, the reported exceptions are suppressed. */ - @Test - fun testSuppressedExceptions() = testResultMap({ - try { - it() - fail("should not be reached") - } catch (e: TestException) { - assertEquals("w", e.message) - val suppressed = e.suppressedExceptions + - (e.suppressedExceptions.firstOrNull()?.suppressedExceptions ?: emptyList()) - assertEquals(3, suppressed.size) - assertEquals("x", suppressed[0].message) - assertEquals("y", suppressed[1].message) - assertEquals("z", suppressed[2].message) - } - }, { - runTest { - reportException(TestException("x")) - reportException(TestException("y")) - reportException(TestException("z")) - throw TestException("w") - } - }) } diff --git a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt index a900cc64f9..98cff3a7f5 100644 --- a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt +++ b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt @@ -123,7 +123,7 @@ class TestCoroutineScopeTest { /** Tests that uncaught exceptions are thrown at the cleanup. */ @Test fun testThrowsUncaughtExceptionsOnCleanup() { - val scope = createTestCoroutineScope() + val scope = TestCoroutineScope() val exception = TestException("test") scope.launch { throw exception @@ -136,7 +136,7 @@ class TestCoroutineScopeTest { /** Tests that uncaught exceptions take priority over uncompleted jobs when throwing on cleanup. */ @Test fun testUncaughtExceptionsPrioritizedOnCleanup() { - val scope = createTestCoroutineScope() + val scope = TestCoroutineScope() val exception = TestException("test") scope.launch { throw exception @@ -149,45 +149,6 @@ class TestCoroutineScopeTest { } } - /** Tests that cleaning up twice is forbidden. */ - @Test - fun testClosingTwice() { - val scope = createTestCoroutineScope() - scope.cleanupTestCoroutines() - assertFailsWith { - scope.cleanupTestCoroutines() - } - } - - /** Tests that throwing after cleaning up is forbidden. */ - @Test - fun testReportingAfterClosing() { - val scope = createTestCoroutineScope() - scope.cleanupTestCoroutines() - assertFailsWith { - scope.reportException(TestException()) - } - } - - /** Tests that, when reporting several exceptions, the first one is thrown, with the rest suppressed. */ - @Test - fun testSuppressedExceptions() { - createTestCoroutineScope().apply { - reportException(TestException("x")) - reportException(TestException("y")) - reportException(TestException("z")) - try { - cleanupTestCoroutines() - fail("should not be reached") - } catch (e: TestException) { - assertEquals("x", e.message) - assertEquals(2, e.suppressedExceptions.size) - assertEquals("y", e.suppressedExceptions[0].message) - assertEquals("z", e.suppressedExceptions[1].message) - } - } - } - companion object { internal val invalidContexts = listOf( Dispatchers.Default, // not a [TestDispatcher] diff --git a/kotlinx-coroutines-test/common/test/TestRunBlockingTest.kt b/kotlinx-coroutines-test/common/test/TestRunBlockingTest.kt index 041f58a3c5..139229e610 100644 --- a/kotlinx-coroutines-test/common/test/TestRunBlockingTest.kt +++ b/kotlinx-coroutines-test/common/test/TestRunBlockingTest.kt @@ -435,6 +435,4 @@ class TestRunBlockingTest { } } } -} - -private class TestException(message: String? = null): Exception(message) \ No newline at end of file +} \ No newline at end of file From 723e1367b86d4612372457fd4e7e5d3c4d528016 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Wed, 27 Oct 2021 11:45:39 +0300 Subject: [PATCH 7/7] Make RunningInRunTest an object --- kotlinx-coroutines-test/common/src/TestBuilders.kt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/kotlinx-coroutines-test/common/src/TestBuilders.kt b/kotlinx-coroutines-test/common/src/TestBuilders.kt index 70a1a5643e..1867999830 100644 --- a/kotlinx-coroutines-test/common/src/TestBuilders.kt +++ b/kotlinx-coroutines-test/common/src/TestBuilders.kt @@ -180,7 +180,7 @@ public fun runTest( ): TestResult { if (context[RunningInRunTest] != null) throw IllegalStateException("Calls to `runTest` can't be nested. Please read the docs on `TestResult` for details.") - val testScope = TestBodyCoroutine(TestCoroutineScope(context + RunningInRunTest())) + val testScope = TestBodyCoroutine(TestCoroutineScope(context + RunningInRunTest)) val scheduler = testScope.testScheduler return createTestResult { /** TODO: moving this [AbstractCoroutine.start] call outside [createTestResult] fails on Native with @@ -271,8 +271,11 @@ public fun TestDispatcher.runTest( runTest(this, dispatchTimeoutMs, block) /** A coroutine context element indicating that the coroutine is running inside `runTest`. */ -private class RunningInRunTest: AbstractCoroutineContextElement(RunningInRunTest), CoroutineContext.Element { - companion object Key : CoroutineContext.Key +private object RunningInRunTest: CoroutineContext.Key, CoroutineContext.Element { + override val key: CoroutineContext.Key<*> + get() = this + + override fun toString(): String = "RunningInRunTest" } /** The default timeout to use when waiting for asynchronous completions of the coroutines managed by