diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt index a1e52d68fd..d9bd3c83e7 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt @@ -69,9 +69,7 @@ private class TestCoroutineScopeImpl ( override fun reportException(throwable: Throwable) { synchronized(lock) { if (cleanedUp) - throw IllegalStateException( - "Attempting to report an uncaught exception after the test coroutine scope was already cleaned up", - throwable) + throw ExceptionReportAfterCleanup(throwable) exceptions.add(throwable) } } @@ -83,6 +81,17 @@ private class TestCoroutineScopeImpl ( val initialJobs = coroutineContext.activeJobs() override fun cleanupTestCoroutines() { + (coroutineContext[CoroutineExceptionHandler] as? UncaughtExceptionCaptor)?.cleanupTestCoroutines() + val delayController = coroutineContext.delayController + var hasUncompletedJobs = false + if (delayController != null) { + delayController.cleanupTestCoroutines() + } else { + testScheduler.runCurrent() + if (!testScheduler.isIdle()) { + hasUncompletedJobs = true + } + } synchronized(lock) { if (cleanedUp) throw IllegalStateException("Attempting to clean up a test coroutine scope more than once.") @@ -92,18 +101,11 @@ private class TestCoroutineScopeImpl ( drop(1).forEach { it.printStackTrace() } singleOrNull()?.let { throw it } } - (coroutineContext[CoroutineExceptionHandler] as? UncaughtExceptionCaptor)?.cleanupTestCoroutines() - val delayController = coroutineContext.delayController - if (delayController != null) { - delayController.cleanupTestCoroutines() - } else { - testScheduler.runCurrent() - if (!testScheduler.isIdle()) { - throw UncompletedCoroutinesError( - "Unfinished coroutines during teardown. Ensure all coroutines are" + - " completed or cancelled by your test." - ) - } + if (hasUncompletedJobs) { + throw UncompletedCoroutinesError( + "Unfinished coroutines during teardown. Ensure all coroutines are" + + " completed or cancelled by your test." + ) } val jobs = coroutineContext.activeJobs() if ((jobs - initialJobs).isNotEmpty()) { @@ -113,6 +115,11 @@ private class TestCoroutineScopeImpl ( } } +internal class ExceptionReportAfterCleanup(cause: Throwable): IllegalStateException( + "Attempting to report an uncaught exception after the test coroutine scope was already cleaned up", + cause +) + private fun CoroutineContext.activeJobs(): Set { return checkNotNull(this[Job]).children.filter { it.isActive }.toSet() } @@ -173,10 +180,23 @@ public fun createTestCoroutineScope(context: CoroutineContext = EmptyCoroutineCo } else -> throw IllegalArgumentException("Dispatcher must implement TestDispatcher: $dispatcher") } - val exceptionHandler = context[CoroutineExceptionHandler] - ?: TestExceptionHandler { _, throwable -> reportException(throwable) } + val linkedHandler: TestExceptionHandlerContextElement? + val exceptionHandler: CoroutineExceptionHandler val handlerOwner = Any() - val linkedHandler = (exceptionHandler as? TestExceptionHandlerContextElement)?.claimOwnershipOrCopy(handlerOwner) + when (val exceptionHandlerInCtx = context[CoroutineExceptionHandler]) { + null -> { + linkedHandler = TestExceptionHandlerContextElement( + { _, throwable -> reportException(throwable) }, + null, + handlerOwner) + exceptionHandler = linkedHandler + } + else -> { + linkedHandler = (exceptionHandlerInCtx as? TestExceptionHandlerContextElement + )?.claimOwnershipOrCopy(handlerOwner) + exceptionHandler = linkedHandler ?: exceptionHandlerInCtx + } + } val job: Job val ownJob: CompletableJob? if (context[Job] == null) { diff --git a/kotlinx-coroutines-test/common/src/TestExceptionHandler.kt b/kotlinx-coroutines-test/common/src/TestExceptionHandler.kt index 827dd39206..2dc96b67c1 100644 --- a/kotlinx-coroutines-test/common/src/TestExceptionHandler.kt +++ b/kotlinx-coroutines-test/common/src/TestExceptionHandler.kt @@ -19,10 +19,14 @@ import kotlin.coroutines.* * * If [linkedScope] is `null`, the [CoroutineExceptionHandler] returned from this function has special behavior when * passed to [createTestCoroutineScope]: the newly-created scope is linked to this handler. If [linkedScope] is not - * null, then the resulting [CoroutineExceptionHandler] will be linked to it. + * null, then the resulting [CoroutineExceptionHandler] will be linked to it; however, it will *not* be part of the + * coroutine context of the [TestCoroutineScope], and this will only affect the receiver of [handler]. * * Passing an already-linked instance to [TestCoroutineScope] will lead to it making its own copy with the same * [handler]. + * + * Throwing inside [handler] is permitted. The thrown exception will we [reported][TestCoroutineScope.reportException] + * to the [TestCoroutineScope]. This is done so to simplify using assertions inside [handler]. */ public fun TestExceptionHandler( linkedScope: TestCoroutineScope? = null, @@ -60,11 +64,23 @@ internal class TestExceptionHandlerContextElement( this.owner = null } + @Suppress("INVISIBLE_MEMBER") override fun handleException(context: CoroutineContext, exception: Throwable) { - synchronized(lock) { + val scope = synchronized(lock) { testCoroutineScope ?: throw RuntimeException("Attempting to handle an exception using a `TestExceptionHandler` that is not linked to a `TestCoroutineScope`") - }.handler(context, exception) - /** it's okay if [handler] throws: [handleCoroutineException] deals with this. */ + } + try { + scope.handler(context, exception) + } catch (e: ExceptionReportAfterCleanup) { + // can only be thrown if the test coroutine scope is already closed. + handleCoroutineExceptionImpl(context, e) + } catch (e: Throwable) { + try { + scope.reportException(e) + } catch (_: ExceptionReportAfterCleanup) { + handleCoroutineExceptionImpl(context, e) + } + } } } \ No newline at end of file diff --git a/kotlinx-coroutines-test/common/test/Helpers.kt b/kotlinx-coroutines-test/common/test/Helpers.kt index c0117ccb2f..578cb4c9eb 100644 --- a/kotlinx-coroutines-test/common/test/Helpers.kt +++ b/kotlinx-coroutines-test/common/test/Helpers.kt @@ -62,4 +62,6 @@ open class OrderedExecutionTestBase { } } -internal fun T.void() { } \ No newline at end of file +internal fun T.void() { } + +internal class TestException(message: String? = null): Exception(message) diff --git a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt index c3bd165852..37ede5d2cd 100644 --- a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt +++ b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt @@ -120,6 +120,55 @@ class TestCoroutineScopeTest { assertFalse(handlerCalled) } + /** Tests that uncaught exceptions are thrown at the cleanup. */ + @Test + fun testThrowsUncaughtExceptionsOnCleanup() { + val scope = createTestCoroutineScope(SupervisorJob()) + 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(SupervisorJob()) + 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()) + } + } + companion object { internal val invalidContexts = listOf( Dispatchers.Default, // not a [TestDispatcher] diff --git a/kotlinx-coroutines-test/common/test/TestExceptionHandlerTest.kt b/kotlinx-coroutines-test/common/test/TestExceptionHandlerTest.kt new file mode 100644 index 0000000000..76e077435a --- /dev/null +++ b/kotlinx-coroutines-test/common/test/TestExceptionHandlerTest.kt @@ -0,0 +1,112 @@ +/* + * Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.coroutines.test + +import kotlinx.coroutines.* +import kotlin.coroutines.* +import kotlin.random.* +import kotlin.test.* + +class TestExceptionHandlerTest { + + /** Tests that passing a [TestExceptionHandler] to [TestCoroutineScope] overrides the exception handling there. */ + @Test + fun testPassingToCoroutineScope() { + var enteredHandler = false + var observedScope: TestCoroutineScope? = null + val handler = TestExceptionHandler { _, throwable -> + assertTrue(throwable is TestException) + observedScope = this + enteredHandler = true + } + val scope = createTestCoroutineScope(handler + SupervisorJob()) + scope.launch { + throw TestException("test") + } + scope.runCurrent() + assertTrue(enteredHandler) + assertSame(scope, observedScope) + scope.cleanupTestCoroutines() + } + + /** Tests that passing a [TestCoroutineScope] to the [TestExceptionHandler] will link, but won't affect the + * coroutine context of [TestCoroutineScope]. */ + @Test + fun testExplicitLinking() { + var observedScope: TestCoroutineScope? = null + val scope = createTestCoroutineScope(SupervisorJob()) + val handler = TestExceptionHandler(scope) { _, throwable -> + assertTrue(throwable is TestException) + observedScope = this + } + scope.launch(handler) { + throw TestException("test1") + } + scope.launch { + throw TestException("test2") + } + scope.runCurrent() + assertSame(scope, observedScope) + try { + scope.cleanupTestCoroutines() + throw AssertionError("won't reach") + } catch (e: TestException) { + assertEquals("test2", e.message) + } + } + + /** Tests that passing a [TestExceptionHandler] that's already linked to another [TestCoroutineScope] will cause it + * to be copied. */ + @Test + fun testRelinking() { + val encountered = mutableListOf() + val handler = TestExceptionHandler { _, throwable -> + assertTrue(throwable is TestException) + encountered.add(this) + } + val scopes = List(3) { createTestCoroutineScope(handler + SupervisorJob()) } + val events = List(10) { Random.nextInt(0, 3) }.map { scopes[it] } + events.forEach { + it.launch { + throw TestException() + } + it.runCurrent() + } + assertEquals(events, encountered) + } + + /** Tests that throwing inside [TestExceptionHandler] is reported. */ + @Test + fun testThrowingInsideHandler() { + val handler = TestExceptionHandler { _, throwable -> + assertEquals("y", throwable.message) + throw TestException("x") + } + val scope = createTestCoroutineScope(handler) + scope.launch { + throw TestException("y") + } + scope.runCurrent() + try { + scope.cleanupTestCoroutines() + throw AssertionError("can't be reached") + } catch (e: TestException) { + assertEquals("x", e.message) + } + } + + /** Tests that throwing inside [TestExceptionHandler] after the scope is cleaned up leads to problems. */ + @Test + @Ignore // difficult to check on JS and Native, where the error is simply logged + fun testThrowingInsideHandlerAfterCleanup() { + val handler = TestExceptionHandler { _, throwable -> + reportException(throwable) + } + val scope = createTestCoroutineScope(handler) + scope.cleanupTestCoroutines() + handler.handleException(EmptyCoroutineContext, TestException()) + } + +} \ No newline at end of file diff --git a/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt b/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt index 0bd277634f..f45bcc5b04 100644 --- a/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt +++ b/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt @@ -420,5 +420,3 @@ class TestRunBlockingTest { assertSame(coroutineContext[CoroutineExceptionHandler], exceptionHandler) } } - -private class TestException(message: String? = null): Exception(message)