From d72d7800dc2c646e03326065b08f76d71c0bc2e1 Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 24 Sep 2021 17:52:39 +0300 Subject: [PATCH 1/5] Change exception handling in the test module --- .../api/kotlinx-coroutines-test.api | 13 +- .../common/src/TestBuilders.kt | 4 +- .../src/TestCoroutineExceptionHandler.kt | 29 +++-- .../common/src/TestCoroutineScope.kt | 114 ++++++++++++++---- .../common/src/TestExceptionHandler.kt | 86 +++++++++++++ .../common/test/RunTestTest.kt | 24 ++++ .../common/test/TestCoroutineScopeTest.kt | 40 +++++- .../common/test/TestExceptionHandlerTest.kt | 112 +++++++++++++++++ .../TestCoroutineExceptionHandlerTest.kt | 3 +- .../test/migration/TestRunBlockingTest.kt | 21 +--- 10 files changed, 387 insertions(+), 59 deletions(-) create mode 100644 kotlinx-coroutines-test/common/src/TestExceptionHandler.kt create mode 100644 kotlinx-coroutines-test/common/test/TestExceptionHandlerTest.kt rename kotlinx-coroutines-test/common/test/{ => migration}/TestCoroutineExceptionHandlerTest.kt (83%) diff --git a/kotlinx-coroutines-test/api/kotlinx-coroutines-test.api b/kotlinx-coroutines-test/api/kotlinx-coroutines-test.api index f024f2105f..d9aeaf884b 100644 --- a/kotlinx-coroutines-test/api/kotlinx-coroutines-test.api +++ b/kotlinx-coroutines-test/api/kotlinx-coroutines-test.api @@ -49,7 +49,7 @@ public final class kotlinx/coroutines/test/TestCoroutineDispatchersKt { public final class kotlinx/coroutines/test/TestCoroutineExceptionHandler : kotlin/coroutines/AbstractCoroutineContextElement, kotlinx/coroutines/CoroutineExceptionHandler, kotlinx/coroutines/test/UncaughtExceptionCaptor { public fun ()V - public fun cleanupTestCoroutinesCaptor ()V + public fun cleanupTestCoroutines ()V public fun getUncaughtExceptions ()Ljava/util/List; public fun handleException (Lkotlin/coroutines/CoroutineContext;Ljava/lang/Throwable;)V } @@ -66,9 +66,10 @@ public final class kotlinx/coroutines/test/TestCoroutineScheduler : kotlin/corou public final class kotlinx/coroutines/test/TestCoroutineScheduler$Key : kotlin/coroutines/CoroutineContext$Key { } -public abstract interface class kotlinx/coroutines/test/TestCoroutineScope : kotlinx/coroutines/CoroutineScope, kotlinx/coroutines/test/UncaughtExceptionCaptor { +public abstract interface class kotlinx/coroutines/test/TestCoroutineScope : kotlinx/coroutines/CoroutineScope { public abstract fun cleanupTestCoroutines ()V public abstract fun getTestScheduler ()Lkotlinx/coroutines/test/TestCoroutineScheduler; + public abstract fun reportException (Ljava/lang/Throwable;)V } public final class kotlinx/coroutines/test/TestCoroutineScopeKt { @@ -79,6 +80,7 @@ public final class kotlinx/coroutines/test/TestCoroutineScopeKt { public static final fun createTestCoroutineScope (Lkotlin/coroutines/CoroutineContext;)Lkotlinx/coroutines/test/TestCoroutineScope; public static synthetic fun createTestCoroutineScope$default (Lkotlin/coroutines/CoroutineContext;ILjava/lang/Object;)Lkotlinx/coroutines/test/TestCoroutineScope; public static final fun getCurrentTime (Lkotlinx/coroutines/test/TestCoroutineScope;)J + public static final fun getUncaughtExceptions (Lkotlinx/coroutines/test/TestCoroutineScope;)Ljava/util/List; public static final fun pauseDispatcher (Lkotlinx/coroutines/test/TestCoroutineScope;)V public static final fun pauseDispatcher (Lkotlinx/coroutines/test/TestCoroutineScope;Lkotlin/jvm/functions/Function1;Lkotlin/coroutines/Continuation;)Ljava/lang/Object; public static final fun resumeDispatcher (Lkotlinx/coroutines/test/TestCoroutineScope;)V @@ -97,8 +99,13 @@ public final class kotlinx/coroutines/test/TestDispatchers { public static final fun setMain (Lkotlinx/coroutines/Dispatchers;Lkotlinx/coroutines/CoroutineDispatcher;)V } +public final class kotlinx/coroutines/test/TestExceptionHandlerKt { + public static final fun TestExceptionHandler (Lkotlinx/coroutines/test/TestCoroutineScope;Lkotlin/jvm/functions/Function3;)Lkotlinx/coroutines/CoroutineExceptionHandler; + public static synthetic fun TestExceptionHandler$default (Lkotlinx/coroutines/test/TestCoroutineScope;Lkotlin/jvm/functions/Function3;ILjava/lang/Object;)Lkotlinx/coroutines/CoroutineExceptionHandler; +} + public abstract interface class kotlinx/coroutines/test/UncaughtExceptionCaptor { - public abstract fun cleanupTestCoroutinesCaptor ()V + public abstract fun cleanupTestCoroutines ()V public abstract fun getUncaughtExceptions ()Ljava/util/List; } diff --git a/kotlinx-coroutines-test/common/src/TestBuilders.kt b/kotlinx-coroutines-test/common/src/TestBuilders.kt index 0d5013cb88..2977b14605 100644 --- a/kotlinx-coroutines-test/common/src/TestBuilders.kt +++ b/kotlinx-coroutines-test/common/src/TestBuilders.kt @@ -308,11 +308,11 @@ private const val DEFAULT_DISPATCH_TIMEOUT_MS = 60_000L private class TestBodyCoroutine( private val testScope: TestCoroutineScope, -) : AbstractCoroutine(testScope.coroutineContext, initParentJob = true, active = true), TestCoroutineScope, - UncaughtExceptionCaptor by testScope.coroutineContext.uncaughtExceptionCaptor +) : 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/TestCoroutineExceptionHandler.kt b/kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt index 9f49292dab..0e766b6275 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt @@ -11,13 +11,16 @@ import kotlin.coroutines.* /** * Access uncaught coroutine exceptions captured during test execution. */ -@ExperimentalCoroutinesApi +@Deprecated( + "Consider whether a `TestExceptionHandler` would work instead. If not, please report your use case at https://github.com/Kotlin/kotlinx.coroutines/issues.", + level = DeprecationLevel.WARNING +) public interface UncaughtExceptionCaptor { /** * List of uncaught coroutine exceptions. * * The returned list is a copy of the currently caught exceptions. - * During [cleanupTestCoroutinesCaptor] the first element of this list is rethrown if it is not empty. + * During [cleanupTestCoroutines] the first element of this list is rethrown if it is not empty. */ public val uncaughtExceptions: List @@ -29,33 +32,39 @@ public interface UncaughtExceptionCaptor { * * @throws Throwable the first uncaught exception, if there are any uncaught exceptions. */ - public fun cleanupTestCoroutinesCaptor() + public fun cleanupTestCoroutines() } /** * An exception handler that captures uncaught exceptions in tests. */ -@ExperimentalCoroutinesApi +@Deprecated("You can use `TestExceptionHandler` to define an exception handler that is linked to " + + "a `TestCoroutineScope`, or define your own `CoroutineExceptionHandler` if you just need to handle uncaught " + + "exceptions without a special `TestCoroutineScope` integration.", level = DeprecationLevel.WARNING) public class TestCoroutineExceptionHandler : - AbstractCoroutineContextElement(CoroutineExceptionHandler), UncaughtExceptionCaptor, CoroutineExceptionHandler + AbstractCoroutineContextElement(CoroutineExceptionHandler), CoroutineExceptionHandler, UncaughtExceptionCaptor { private val _exceptions = mutableListOf() private val _lock = SynchronizedObject() + private var _coroutinesCleanedUp = false - /** @suppress **/ + @Suppress("INVISIBLE_MEMBER") override fun handleException(context: CoroutineContext, exception: Throwable) { synchronized(_lock) { + if (_coroutinesCleanedUp) { + handleCoroutineExceptionImpl(context, exception) + return + } _exceptions += exception } } - /** @suppress **/ - override val uncaughtExceptions: List + public override val uncaughtExceptions: List get() = synchronized(_lock) { _exceptions.toList() } - /** @suppress **/ - override fun cleanupTestCoroutinesCaptor() { + public override fun cleanupTestCoroutines() { synchronized(_lock) { + _coroutinesCleanedUp = true val exception = _exceptions.firstOrNull() ?: return // log the rest _exceptions.drop(1).forEach { it.printStackTrace() } diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt index f60a97e088..9abcde655b 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt @@ -5,21 +5,33 @@ package kotlinx.coroutines.test import kotlinx.coroutines.* +import kotlinx.coroutines.internal.* import kotlin.coroutines.* /** * A scope which provides detailed control over the execution of coroutines for tests. */ @ExperimentalCoroutinesApi -public sealed interface TestCoroutineScope: CoroutineScope, UncaughtExceptionCaptor { +public sealed interface TestCoroutineScope: CoroutineScope { /** * Called after the test completes. * - * Calls [UncaughtExceptionCaptor.cleanupTestCoroutinesCaptor] and [DelayController.cleanupTestCoroutines]. - * If a new job was created for this scope, the job is completed. + * * It checks that there were no uncaught exceptions reported via [reportException]. If there were any, then the + * first one is thrown, whereas the rest are printed to the standard output or the standard error output + * (see [Throwable.printStackTrace]). + * * It runs the tasks pending in the scheduler at the current time. If there are any uncompleted tasks afterwards, + * it fails with [UncompletedCoroutinesError]. + * * It checks whether some new child [Job]s were created but not completed since this [TestCoroutineScope] was + * created. If so, it fails with [UncompletedCoroutinesError]. + * + * For backwards compatibility, if the [CoroutineExceptionHandler] is an [UncaughtExceptionCaptor], its + * [TestCoroutineExceptionHandler.cleanupTestCoroutines] behavior is performed. + * Likewise, if the [ContinuationInterceptor] is a [DelayController], its [DelayController.cleanupTestCoroutines] + * is called. * * @throws Throwable the first uncaught exception, if there are any uncaught exceptions. * @throws AssertionError if any pending tasks are active. + * @throws IllegalStateException if called more than once. */ @ExperimentalCoroutinesApi public fun cleanupTestCoroutines() @@ -29,14 +41,36 @@ public sealed interface TestCoroutineScope: CoroutineScope, UncaughtExceptionCap */ @ExperimentalCoroutinesApi public val testScheduler: TestCoroutineScheduler + + /** + * Reports an exception so that it is thrown on [cleanupTestCoroutines]. + * + * If several exceptions are reported, only the first one will be thrown, and the other ones will be suppressed by + * it. + * + * @throws IllegalStateException with the [Throwable.cause] set to [throwable] if [cleanupTestCoroutines] was + * already called. + */ + @ExperimentalCoroutinesApi + public fun reportException(throwable: Throwable) } private class TestCoroutineScopeImpl( override val coroutineContext: CoroutineContext -): - TestCoroutineScope, - UncaughtExceptionCaptor by coroutineContext.uncaughtExceptionCaptor +): TestCoroutineScope { + private val lock = SynchronizedObject() + private var exceptions = mutableListOf() + private var cleanedUp = false + + override fun reportException(throwable: Throwable) { + synchronized(lock) { + if (cleanedUp) + throw ExceptionReportAfterCleanup(throwable) + exceptions.add(throwable) + } + } + override val testScheduler: TestCoroutineScheduler get() = coroutineContext[TestCoroutineScheduler]!! @@ -56,7 +90,16 @@ private class TestCoroutineScopeImpl( testScheduler.runCurrent() !testScheduler.isIdle() } - coroutineContext.uncaughtExceptionCaptor.cleanupTestCoroutinesCaptor() + (coroutineContext[CoroutineExceptionHandler] as? UncaughtExceptionCaptor)?.cleanupTestCoroutines() + synchronized(lock) { + if (cleanedUp) + throw IllegalStateException("Attempting to clean up a test coroutine scope more than once.") + cleanedUp = true + } + exceptions.firstOrNull()?.let { toThrow -> + exceptions.drop(1).forEach { toThrow.addSuppressed(it) } + throw toThrow + } if (hasUnfinishedJobs) throw UncompletedCoroutinesError( "Unfinished coroutines during teardown. Ensure all coroutines are" + @@ -68,6 +111,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() } @@ -79,13 +127,13 @@ private fun CoroutineContext.activeJobs(): Set { */ @Deprecated("This constructs a `TestCoroutineScope` with a deprecated `CoroutineDispatcher` by default. " + "Please use `createTestCoroutineScope` instead.", - ReplaceWith("createTestCoroutineScope(TestCoroutineDispatcher() + context)", + ReplaceWith("createTestCoroutineScope(TestCoroutineDispatcher() + TestCoroutineExceptionHandler() + context)", "kotlin.coroutines.EmptyCoroutineContext"), level = DeprecationLevel.WARNING ) public fun TestCoroutineScope(context: CoroutineContext = EmptyCoroutineContext): TestCoroutineScope { val scheduler = context[TestCoroutineScheduler] ?: TestCoroutineScheduler() - return createTestCoroutineScope(TestCoroutineDispatcher(scheduler) + context) + return createTestCoroutineScope(TestCoroutineDispatcher(scheduler) + TestCoroutineExceptionHandler() + context) } /** @@ -126,26 +174,28 @@ public fun createTestCoroutineScope(context: CoroutineContext = EmptyCoroutineCo } else -> throw IllegalArgumentException("Dispatcher must implement TestDispatcher: $dispatcher") } - val exceptionHandler = context[CoroutineExceptionHandler].run { - this?.let { - require(this is UncaughtExceptionCaptor) { - "coroutineExceptionHandler must implement UncaughtExceptionCaptor: $context" - } + val linkedHandler: TestExceptionHandlerContextElement? + val exceptionHandler: CoroutineExceptionHandler + val handlerOwner = Any() + 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 } - this ?: TestCoroutineExceptionHandler() } val job: Job = context[Job] ?: Job() return TestCoroutineScopeImpl(context + scheduler + dispatcher + exceptionHandler + job) + .also { linkedHandler?.registerTestCoroutineScope(handlerOwner, it) } } -internal inline val CoroutineContext.uncaughtExceptionCaptor: UncaughtExceptionCaptor - get() { - val handler = this[CoroutineExceptionHandler] - return handler as? UncaughtExceptionCaptor ?: throw IllegalArgumentException( - "TestCoroutineScope requires a UncaughtExceptionCaptor such as " + - "TestCoroutineExceptionHandler as the CoroutineExceptionHandler" - ) - } private inline val CoroutineContext.delayController: DelayController? get() { @@ -238,6 +288,24 @@ public fun TestCoroutineScope.resumeDispatcher() { delayControllerForPausing.resumeDispatcher() } +/** + * List of uncaught coroutine exceptions, for backward compatibility. + * + * The returned list is a copy of the exceptions caught during execution. + * During [TestCoroutineScope.cleanupTestCoroutines] the first element of this list is rethrown if it is not empty. + * + * Exceptions are only collected in this list if the [UncaughtExceptionCaptor] is in the test context. + */ +@Deprecated( + "This list is only populated if `UncaughtExceptionCaptor` is in the test context, and so can be " + + "easily misused. It is only present for backward compatibility and will be removed in the subsequent " + + "releases. If you need to check the list of exceptions, please consider creating your own " + + "`CoroutineExceptionHandler`.", + level = DeprecationLevel.WARNING) +public val TestCoroutineScope.uncaughtExceptions: List + get() = (coroutineContext[CoroutineExceptionHandler] as? UncaughtExceptionCaptor)?.uncaughtExceptions + ?: emptyList() + private val TestCoroutineScope.delayControllerForPausing: DelayController get() = coroutineContext.delayController ?: throw IllegalStateException("This scope isn't able to pause its dispatchers") diff --git a/kotlinx-coroutines-test/common/src/TestExceptionHandler.kt b/kotlinx-coroutines-test/common/src/TestExceptionHandler.kt new file mode 100644 index 0000000000..2dc96b67c1 --- /dev/null +++ b/kotlinx-coroutines-test/common/src/TestExceptionHandler.kt @@ -0,0 +1,86 @@ +/* + * 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 kotlinx.coroutines.internal.* +import kotlin.coroutines.* + +/** + * A [CoroutineExceptionHandler] connected to a [TestCoroutineScope]. + * + * This function accepts a [handler] that describes how to handle uncaught exceptions during tests; see + * [CoroutineExceptionHandler] for details. As opposed to [CoroutineExceptionHandler], however, this has access to the + * [TestCoroutineScope], which allows [cancelling][CoroutineScope.cancel] it or + * [reporting][TestCoroutineScope.reportException] the error so that it is thrown on the call to + * [TestCoroutineScope.cleanupTestCoroutines]. + * + * 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; 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, + handler: TestCoroutineScope.(CoroutineContext, Throwable) -> Unit +): CoroutineExceptionHandler = TestExceptionHandlerContextElement(handler, linkedScope) + +/** The [CoroutineExceptionHandler] corresponding to the given [handler]. */ +internal class TestExceptionHandlerContextElement( + private val handler: TestCoroutineScope.(CoroutineContext, Throwable) -> Unit, + private var testCoroutineScope: TestCoroutineScope?, + private var owner: Any? = null +): AbstractCoroutineContextElement(CoroutineExceptionHandler), CoroutineExceptionHandler +{ + private val lock = SynchronizedObject() + + /** + * Claims ownership of this [TestExceptionHandler], or returns its copy, owned and not linked to anything. + */ + fun claimOwnershipOrCopy(owner: Any): TestExceptionHandlerContextElement = synchronized(lock) { + if (this.owner == null && testCoroutineScope == null) { + this.owner = owner + this + } else { + TestExceptionHandlerContextElement(handler, null, owner) + } + } + + /** + * Links a [TestCoroutineScope] to this, unless there's already one linked. + */ + fun registerTestCoroutineScope(owner: Any, scope: TestCoroutineScope) = + synchronized(lock) { + check(this.owner === owner && testCoroutineScope == null) + testCoroutineScope = scope + this.owner = null + } + + @Suppress("INVISIBLE_MEMBER") + override fun handleException(context: CoroutineContext, exception: Throwable) { + val scope = synchronized(lock) { + testCoroutineScope + ?: throw RuntimeException("Attempting to handle an exception using a `TestExceptionHandler` that is not linked to a `TestCoroutineScope`") + } + 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/RunTestTest.kt b/kotlinx-coroutines-test/common/test/RunTestTest.kt index 623b5bf758..03f933a0ec 100644 --- a/kotlinx-coroutines-test/common/test/RunTestTest.kt +++ b/kotlinx-coroutines-test/common/test/RunTestTest.kt @@ -251,4 +251,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") + } + }) } diff --git a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt index 9002d2d8c3..15ac46059e 100644 --- a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt +++ b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt @@ -124,11 +124,49 @@ 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] StandardTestDispatcher() + TestCoroutineScheduler(), // the dispatcher is not linked to the scheduler - CoroutineExceptionHandler { _, _ -> }, // not an `UncaughtExceptionCaptor` ) } } 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/TestCoroutineExceptionHandlerTest.kt b/kotlinx-coroutines-test/common/test/migration/TestCoroutineExceptionHandlerTest.kt similarity index 83% rename from kotlinx-coroutines-test/common/test/TestCoroutineExceptionHandlerTest.kt rename to kotlinx-coroutines-test/common/test/migration/TestCoroutineExceptionHandlerTest.kt index 674fd288dd..20da130725 100644 --- a/kotlinx-coroutines-test/common/test/TestCoroutineExceptionHandlerTest.kt +++ b/kotlinx-coroutines-test/common/test/migration/TestCoroutineExceptionHandlerTest.kt @@ -1,11 +1,12 @@ /* - * Copyright 2016-2019 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + * 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 kotlin.test.* +@Suppress("DEPRECATION") class TestCoroutineExceptionHandlerTest { @Test fun whenExceptionsCaught_availableViaProperty() { diff --git a/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt b/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt index 66c06cf49f..dcae80e0a4 100644 --- a/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt +++ b/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt @@ -235,12 +235,13 @@ class TestRunBlockingTest { fun callingAsyncFunction_executesAsyncBlockImmediately() = runBlockingTest { assertRunsFast { var executed = false - async { + val deferred = async { delay(SLOW) executed = true } advanceTimeBy(SLOW) + assertTrue(deferred.isCompleted) assertTrue(executed) } } @@ -308,15 +309,6 @@ class TestRunBlockingTest { } } - @Test - fun runBlockingTestBuilder_throwsOnBadHandler() { - assertFailsWith { - runBlockingTest(CoroutineExceptionHandler { _, _ -> }) { - - } - } - } - @Test fun pauseDispatcher_disablesAutoAdvance_forCurrent() = runBlockingTest { var mutable = 0 @@ -427,13 +419,4 @@ class TestRunBlockingTest { fun testOverrideExceptionHandler() = runBlockingTest(exceptionHandler) { assertSame(coroutineContext[CoroutineExceptionHandler], exceptionHandler) } - - @Test - fun testOverrideExceptionHandlerError() { - assertFailsWith { - runBlockingTest(CoroutineExceptionHandler { _, _ -> }) { - fail("Unreached") - } - } - } } \ No newline at end of file From 0c197e7079374cf6d4ea286599a9a9ec3bfaebfb Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 29 Oct 2021 12:01:09 +0300 Subject: [PATCH 2/5] Remove configurable exception handling --- kotlinx-coroutines-core/common/README.md | 1 - .../api/kotlinx-coroutines-test.api | 6 - .../common/src/TestBuilders.kt | 30 +++- .../src/TestCoroutineExceptionHandler.kt | 14 +- .../common/src/TestCoroutineScope.kt | 154 ++++++++++-------- .../common/src/TestExceptionHandler.kt | 86 ---------- .../common/test/Helpers.kt | 3 +- .../common/test/RunTestTest.kt | 6 +- .../common/test/TestCoroutineScopeTest.kt | 17 +- .../common/test/TestExceptionHandlerTest.kt | 112 ------------- .../jvm/test/MultithreadingTest.kt | 1 - 11 files changed, 124 insertions(+), 306 deletions(-) delete mode 100644 kotlinx-coroutines-test/common/src/TestExceptionHandler.kt delete mode 100644 kotlinx-coroutines-test/common/test/TestExceptionHandlerTest.kt diff --git a/kotlinx-coroutines-core/common/README.md b/kotlinx-coroutines-core/common/README.md index b00921bbcd..23ca0a55ca 100644 --- a/kotlinx-coroutines-core/common/README.md +++ b/kotlinx-coroutines-core/common/README.md @@ -130,7 +130,6 @@ Low-level primitives for finer-grained control of coroutines. [kotlinx.coroutines.sync.Mutex]: https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.sync/-mutex/index.html [kotlinx.coroutines.sync.Mutex.lock]: https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.sync/-mutex/lock.html -[kotlinx.coroutines.sync.Mutex.tryLock]: https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.sync/-mutex/try-lock.html diff --git a/kotlinx-coroutines-test/api/kotlinx-coroutines-test.api b/kotlinx-coroutines-test/api/kotlinx-coroutines-test.api index d9aeaf884b..f3a69b9f1c 100644 --- a/kotlinx-coroutines-test/api/kotlinx-coroutines-test.api +++ b/kotlinx-coroutines-test/api/kotlinx-coroutines-test.api @@ -69,7 +69,6 @@ public final class kotlinx/coroutines/test/TestCoroutineScheduler$Key : kotlin/c public abstract interface class kotlinx/coroutines/test/TestCoroutineScope : kotlinx/coroutines/CoroutineScope { public abstract fun cleanupTestCoroutines ()V public abstract fun getTestScheduler ()Lkotlinx/coroutines/test/TestCoroutineScheduler; - public abstract fun reportException (Ljava/lang/Throwable;)V } public final class kotlinx/coroutines/test/TestCoroutineScopeKt { @@ -99,11 +98,6 @@ public final class kotlinx/coroutines/test/TestDispatchers { public static final fun setMain (Lkotlinx/coroutines/Dispatchers;Lkotlinx/coroutines/CoroutineDispatcher;)V } -public final class kotlinx/coroutines/test/TestExceptionHandlerKt { - public static final fun TestExceptionHandler (Lkotlinx/coroutines/test/TestCoroutineScope;Lkotlin/jvm/functions/Function3;)Lkotlinx/coroutines/CoroutineExceptionHandler; - public static synthetic fun TestExceptionHandler$default (Lkotlinx/coroutines/test/TestCoroutineScope;Lkotlin/jvm/functions/Function3;ILjava/lang/Object;)Lkotlinx/coroutines/CoroutineExceptionHandler; -} - public abstract interface class kotlinx/coroutines/test/UncaughtExceptionCaptor { public abstract fun cleanupTestCoroutines ()V public abstract fun getUncaughtExceptions ()Ljava/util/List; diff --git a/kotlinx-coroutines-test/common/src/TestBuilders.kt b/kotlinx-coroutines-test/common/src/TestBuilders.kt index 2977b14605..912580248f 100644 --- a/kotlinx-coroutines-test/common/src/TestBuilders.kt +++ b/kotlinx-coroutines-test/common/src/TestBuilders.kt @@ -43,7 +43,10 @@ import kotlin.coroutines.* * @param testBody The code of the unit-test. */ @Deprecated("Use `runTest` instead to support completing from other dispatchers.", level = DeprecationLevel.WARNING) -public fun runBlockingTest(context: CoroutineContext = EmptyCoroutineContext, testBody: suspend TestCoroutineScope.() -> Unit) { +public fun runBlockingTest( + context: CoroutineContext = EmptyCoroutineContext, + testBody: suspend TestCoroutineScope.() -> Unit +) { val scope = createTestCoroutineScope(TestCoroutineDispatcher() + SupervisorJob() + context) val scheduler = scope.testScheduler val deferred = scope.async { @@ -235,7 +238,7 @@ public fun runTest( } onTimeout(dispatchTimeoutMs) { try { - testScope.cleanupTestCoroutines() + testScope.cleanup() } catch (e: UncompletedCoroutinesError) { // we expect these and will instead throw a more informative exception just below. } @@ -245,7 +248,7 @@ public fun runTest( } testScope.getCompletionExceptionOrNull()?.let { try { - testScope.cleanupTestCoroutines() + testScope.cleanup() } 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) { @@ -253,7 +256,7 @@ public fun runTest( } throw it } - testScope.cleanupTestCoroutines() + testScope.cleanup() } } @@ -295,7 +298,7 @@ public fun TestDispatcher.runTest( runTest(this, dispatchTimeoutMs, block) /** A coroutine context element indicating that the coroutine is running inside `runTest`. */ -private object RunningInRunTest: CoroutineContext.Key, CoroutineContext.Element { +private object RunningInRunTest : CoroutineContext.Key, CoroutineContext.Element { override val key: CoroutineContext.Key<*> get() = this @@ -308,11 +311,20 @@ private const val DEFAULT_DISPATCH_TIMEOUT_MS = 60_000L private class TestBodyCoroutine( private val testScope: TestCoroutineScope, -) : AbstractCoroutine(testScope.coroutineContext, initParentJob = true, active = true), TestCoroutineScope -{ +) : AbstractCoroutine(testScope.coroutineContext, initParentJob = true, active = true), TestCoroutineScope { + override val testScheduler get() = testScope.testScheduler - override fun cleanupTestCoroutines() = testScope.cleanupTestCoroutines() + @Deprecated( + "This deprecation is to prevent accidentally calling `cleanupTestCoroutines` in our own code.", + ReplaceWith("this.cleanup()"), + DeprecationLevel.ERROR + ) + override fun cleanupTestCoroutines() = + throw UnsupportedOperationException( + "Calling `cleanupTestCoroutines` inside `runTest` is prohibited: " + + "it will be called at the end of the test in any case." + ) - override fun reportException(throwable: Throwable) = testScope.reportException(throwable) + fun cleanup() = testScope.cleanupTestCoroutines() } diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt b/kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt index 0e766b6275..e40ef4ed28 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt @@ -12,7 +12,9 @@ import kotlin.coroutines.* * Access uncaught coroutine exceptions captured during test execution. */ @Deprecated( - "Consider whether a `TestExceptionHandler` would work instead. If not, please report your use case at https://github.com/Kotlin/kotlinx.coroutines/issues.", + "Consider whether the default mechanism of handling uncaught exceptions is sufficient. " + + "If not, try writing your own `CoroutineExceptionHandler` and " + + "please report your use case at https://github.com/Kotlin/kotlinx.coroutines/issues.", level = DeprecationLevel.WARNING ) public interface UncaughtExceptionCaptor { @@ -38,12 +40,12 @@ public interface UncaughtExceptionCaptor { /** * An exception handler that captures uncaught exceptions in tests. */ -@Deprecated("You can use `TestExceptionHandler` to define an exception handler that is linked to " + - "a `TestCoroutineScope`, or define your own `CoroutineExceptionHandler` if you just need to handle uncaught " + - "exceptions without a special `TestCoroutineScope` integration.", level = DeprecationLevel.WARNING) +@Deprecated( + "It's better to define one's own `CoroutineExceptionHandler` if you just need to handle '" + + "uncaught exceptions without a special `TestCoroutineScope` integration.", level = DeprecationLevel.WARNING +) public class TestCoroutineExceptionHandler : - AbstractCoroutineContextElement(CoroutineExceptionHandler), CoroutineExceptionHandler, UncaughtExceptionCaptor -{ + AbstractCoroutineContextElement(CoroutineExceptionHandler), CoroutineExceptionHandler, UncaughtExceptionCaptor { private val _exceptions = mutableListOf() private val _lock = SynchronizedObject() private var _coroutinesCleanedUp = false diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt index 9abcde655b..066bbe2657 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt @@ -12,19 +12,18 @@ import kotlin.coroutines.* * A scope which provides detailed control over the execution of coroutines for tests. */ @ExperimentalCoroutinesApi -public sealed interface TestCoroutineScope: CoroutineScope { +public sealed interface TestCoroutineScope : CoroutineScope { /** * Called after the test completes. * - * * It checks that there were no uncaught exceptions reported via [reportException]. If there were any, then the - * first one is thrown, whereas the rest are printed to the standard output or the standard error output - * (see [Throwable.printStackTrace]). + * * It checks that there were no uncaught exceptions caught by its [CoroutineExceptionHandler]. + * If there were any, then the first one is thrown, whereas the rest are suppressed by it. * * It runs the tasks pending in the scheduler at the current time. If there are any uncompleted tasks afterwards, * it fails with [UncompletedCoroutinesError]. * * It checks whether some new child [Job]s were created but not completed since this [TestCoroutineScope] was * created. If so, it fails with [UncompletedCoroutinesError]. * - * For backwards compatibility, if the [CoroutineExceptionHandler] is an [UncaughtExceptionCaptor], its + * For backward compatibility, if the [CoroutineExceptionHandler] is an [UncaughtExceptionCaptor], its * [TestCoroutineExceptionHandler.cleanupTestCoroutines] behavior is performed. * Likewise, if the [ContinuationInterceptor] is a [DelayController], its [DelayController.cleanupTestCoroutines] * is called. @@ -41,35 +40,32 @@ public sealed interface TestCoroutineScope: CoroutineScope { */ @ExperimentalCoroutinesApi public val testScheduler: TestCoroutineScheduler - - /** - * Reports an exception so that it is thrown on [cleanupTestCoroutines]. - * - * If several exceptions are reported, only the first one will be thrown, and the other ones will be suppressed by - * it. - * - * @throws IllegalStateException with the [Throwable.cause] set to [throwable] if [cleanupTestCoroutines] was - * already called. - */ - @ExperimentalCoroutinesApi - public fun reportException(throwable: Throwable) } private class TestCoroutineScopeImpl( override val coroutineContext: CoroutineContext -): TestCoroutineScope -{ +) : TestCoroutineScope { private val lock = SynchronizedObject() private var exceptions = mutableListOf() private var cleanedUp = false - override fun reportException(throwable: Throwable) { + /** + * Reports an exception so that it is thrown on [cleanupTestCoroutines]. + * + * If several exceptions are reported, only the first one will be thrown, and the other ones will be suppressed by + * it. + * + * Returns `false` if [cleanupTestCoroutines] was already called. + */ + fun reportException(throwable: Throwable): Boolean = synchronized(lock) { - if (cleanedUp) - throw ExceptionReportAfterCleanup(throwable) - exceptions.add(throwable) + if (cleanedUp) { + false + } else { + exceptions.add(throwable) + true + } } - } override val testScheduler: TestCoroutineScheduler get() = coroutineContext[TestCoroutineScheduler]!! @@ -111,7 +107,7 @@ private class TestCoroutineScopeImpl( } } -internal class ExceptionReportAfterCleanup(cause: Throwable): IllegalStateException( +internal class ExceptionReportAfterCleanup(cause: Throwable) : IllegalStateException( "Attempting to report an uncaught exception after the test coroutine scope was already cleaned up", cause ) @@ -125,10 +121,13 @@ private fun CoroutineContext.activeJobs(): Set { * * [createTestCoroutineScope] is a similar function that defaults to [StandardTestDispatcher]. */ -@Deprecated("This constructs a `TestCoroutineScope` with a deprecated `CoroutineDispatcher` by default. " + - "Please use `createTestCoroutineScope` instead.", - ReplaceWith("createTestCoroutineScope(TestCoroutineDispatcher() + TestCoroutineExceptionHandler() + context)", - "kotlin.coroutines.EmptyCoroutineContext"), +@Deprecated( + "This constructs a `TestCoroutineScope` with a deprecated `CoroutineDispatcher` by default. " + + "Please use `createTestCoroutineScope` instead.", + ReplaceWith( + "createTestCoroutineScope(TestCoroutineDispatcher() + TestCoroutineExceptionHandler() + context)", + "kotlin.coroutines.EmptyCoroutineContext" + ), level = DeprecationLevel.WARNING ) public fun TestCoroutineScope(context: CoroutineContext = EmptyCoroutineContext): TestCoroutineScope { @@ -143,8 +142,13 @@ public fun TestCoroutineScope(context: CoroutineContext = EmptyCoroutineContext) * * If [context] doesn't define a [TestCoroutineScheduler] for orchestrating the virtual time used for delay-skipping, * a new one is created, unless a [TestDispatcher] is provided, in which case [TestDispatcher.scheduler] is used. * * If [context] doesn't have a [ContinuationInterceptor], a [StandardTestDispatcher] is created. - * * If [context] does not provide a [CoroutineExceptionHandler], [TestCoroutineExceptionHandler] is created - * automatically. + * * A [CoroutineExceptionHandler] is created that makes [TestCoroutineScope.cleanupTestCoroutines] throw if there were + * any uncaught exceptions, or forwards the exceptions further in a platform-specific manner if the cleanup was + * already performed when an exception happened. Passing a [CoroutineExceptionHandler] is illegal, unless it's an + * [UncaughtExceptionCaptor], in which case the behavior is preserved for the time being for backward compatibility. + * If you need to have a specific [CoroutineExceptionHandler], please pass it to [launch] on an already-created + * [TestCoroutineScope] and share your use case at + * [our issue tracker](https://github.com/Kotlin/kotlinx.coroutines/issues). * * If [context] provides a [Job], that job is used for the new scope; otherwise, a [CompletableJob] is created. * * @throws IllegalArgumentException if [context] has both [TestCoroutineScheduler] and a [TestDispatcher] linked to a @@ -174,26 +178,25 @@ public fun createTestCoroutineScope(context: CoroutineContext = EmptyCoroutineCo } else -> throw IllegalArgumentException("Dispatcher must implement TestDispatcher: $dispatcher") } - val linkedHandler: TestExceptionHandlerContextElement? - val exceptionHandler: CoroutineExceptionHandler - val handlerOwner = Any() - when (val exceptionHandlerInCtx = context[CoroutineExceptionHandler]) { + var scope: TestCoroutineScopeImpl? = null + val exceptionHandler = when (val exceptionHandler = context[CoroutineExceptionHandler]) { + is UncaughtExceptionCaptor -> exceptionHandler null -> { - linkedHandler = TestExceptionHandlerContextElement( - { _, throwable -> reportException(throwable) }, - null, - handlerOwner) - exceptionHandler = linkedHandler - } - else -> { - linkedHandler = (exceptionHandlerInCtx as? TestExceptionHandlerContextElement - )?.claimOwnershipOrCopy(handlerOwner) - exceptionHandler = linkedHandler ?: exceptionHandlerInCtx + val lock = SynchronizedObject() + CoroutineExceptionHandler { context, throwable -> + val reported = synchronized(lock) { + scope!!.reportException(throwable) + } + if (!reported) + throw throwable // let this exception crash everything + } } + else -> throw IllegalArgumentException("A CoroutineExceptionHandler was passed to TestCoroutineScope") } val job: Job = context[Job] ?: Job() - return TestCoroutineScopeImpl(context + scheduler + dispatcher + exceptionHandler + job) - .also { linkedHandler?.registerTestCoroutineScope(handlerOwner, it) } + return TestCoroutineScopeImpl(context + scheduler + dispatcher + exceptionHandler + job).also { + scope = it + } } @@ -219,10 +222,12 @@ public val TestCoroutineScope.currentTime: Long * @see TestCoroutineScheduler.advanceTimeBy */ @ExperimentalCoroutinesApi -@Deprecated("The name of this function is misleading: it not only advances the time, but also runs the tasks " + - "scheduled *at* the ending moment.", +@Deprecated( + "The name of this function is misleading: it not only advances the time, but also runs the tasks " + + "scheduled *at* the ending moment.", ReplaceWith("this.testScheduler.apply { advanceTimeBy(delayTimeMillis); runCurrent() }"), - DeprecationLevel.WARNING) + DeprecationLevel.WARNING +) public fun TestCoroutineScope.advanceTimeBy(delayTimeMillis: Long): Unit = when (val controller = coroutineContext.delayController) { null -> { @@ -256,34 +261,46 @@ public fun TestCoroutineScope.runCurrent() { } @ExperimentalCoroutinesApi -@Deprecated("The test coroutine scope isn't able to pause its dispatchers in the general case. " + - "Only `TestCoroutineDispatcher` supports pausing; pause it directly, or use a dispatcher that is always " + - "\"paused\", like `StandardTestDispatcher`.", - ReplaceWith("(this.coroutineContext[ContinuationInterceptor]!! as DelayController).pauseDispatcher(block)", - "kotlin.coroutines.ContinuationInterceptor"), - DeprecationLevel.WARNING) +@Deprecated( + "The test coroutine scope isn't able to pause its dispatchers in the general case. " + + "Only `TestCoroutineDispatcher` supports pausing; pause it directly, or use a dispatcher that is always " + + "\"paused\", like `StandardTestDispatcher`.", + ReplaceWith( + "(this.coroutineContext[ContinuationInterceptor]!! as DelayController).pauseDispatcher(block)", + "kotlin.coroutines.ContinuationInterceptor" + ), + DeprecationLevel.WARNING +) public suspend fun TestCoroutineScope.pauseDispatcher(block: suspend () -> Unit) { delayControllerForPausing.pauseDispatcher(block) } @ExperimentalCoroutinesApi -@Deprecated("The test coroutine scope isn't able to pause its dispatchers in the general case. " + - "Only `TestCoroutineDispatcher` supports pausing; pause it directly, or use a dispatcher that is always " + +@Deprecated( + "The test coroutine scope isn't able to pause its dispatchers in the general case. " + + "Only `TestCoroutineDispatcher` supports pausing; pause it directly, or use a dispatcher that is always " + "\"paused\", like `StandardTestDispatcher`.", - ReplaceWith("(this.coroutineContext[ContinuationInterceptor]!! as DelayController).pauseDispatcher()", - "kotlin.coroutines.ContinuationInterceptor"), -level = DeprecationLevel.WARNING) + ReplaceWith( + "(this.coroutineContext[ContinuationInterceptor]!! as DelayController).pauseDispatcher()", + "kotlin.coroutines.ContinuationInterceptor" + ), + level = DeprecationLevel.WARNING +) public fun TestCoroutineScope.pauseDispatcher() { delayControllerForPausing.pauseDispatcher() } @ExperimentalCoroutinesApi -@Deprecated("The test coroutine scope isn't able to pause its dispatchers in the general case. " + - "Only `TestCoroutineDispatcher` supports pausing; pause it directly, or use a dispatcher that is always " + +@Deprecated( + "The test coroutine scope isn't able to pause its dispatchers in the general case. " + + "Only `TestCoroutineDispatcher` supports pausing; pause it directly, or use a dispatcher that is always " + "\"paused\", like `StandardTestDispatcher`.", - ReplaceWith("(this.coroutineContext[ContinuationInterceptor]!! as DelayController).resumeDispatcher()", - "kotlin.coroutines.ContinuationInterceptor"), - level = DeprecationLevel.WARNING) + ReplaceWith( + "(this.coroutineContext[ContinuationInterceptor]!! as DelayController).resumeDispatcher()", + "kotlin.coroutines.ContinuationInterceptor" + ), + level = DeprecationLevel.WARNING +) public fun TestCoroutineScope.resumeDispatcher() { delayControllerForPausing.resumeDispatcher() } @@ -301,7 +318,8 @@ public fun TestCoroutineScope.resumeDispatcher() { "easily misused. It is only present for backward compatibility and will be removed in the subsequent " + "releases. If you need to check the list of exceptions, please consider creating your own " + "`CoroutineExceptionHandler`.", - level = DeprecationLevel.WARNING) + level = DeprecationLevel.WARNING +) public val TestCoroutineScope.uncaughtExceptions: List get() = (coroutineContext[CoroutineExceptionHandler] as? UncaughtExceptionCaptor)?.uncaughtExceptions ?: emptyList() @@ -314,4 +332,4 @@ private val TestCoroutineScope.delayControllerForPausing: DelayController * Thrown when a test has completed and there are tasks that are not completed or cancelled. */ @ExperimentalCoroutinesApi -internal class UncompletedCoroutinesError(message: String): AssertionError(message) +internal class UncompletedCoroutinesError(message: String) : AssertionError(message) diff --git a/kotlinx-coroutines-test/common/src/TestExceptionHandler.kt b/kotlinx-coroutines-test/common/src/TestExceptionHandler.kt deleted file mode 100644 index 2dc96b67c1..0000000000 --- a/kotlinx-coroutines-test/common/src/TestExceptionHandler.kt +++ /dev/null @@ -1,86 +0,0 @@ -/* - * 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 kotlinx.coroutines.internal.* -import kotlin.coroutines.* - -/** - * A [CoroutineExceptionHandler] connected to a [TestCoroutineScope]. - * - * This function accepts a [handler] that describes how to handle uncaught exceptions during tests; see - * [CoroutineExceptionHandler] for details. As opposed to [CoroutineExceptionHandler], however, this has access to the - * [TestCoroutineScope], which allows [cancelling][CoroutineScope.cancel] it or - * [reporting][TestCoroutineScope.reportException] the error so that it is thrown on the call to - * [TestCoroutineScope.cleanupTestCoroutines]. - * - * 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; 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, - handler: TestCoroutineScope.(CoroutineContext, Throwable) -> Unit -): CoroutineExceptionHandler = TestExceptionHandlerContextElement(handler, linkedScope) - -/** The [CoroutineExceptionHandler] corresponding to the given [handler]. */ -internal class TestExceptionHandlerContextElement( - private val handler: TestCoroutineScope.(CoroutineContext, Throwable) -> Unit, - private var testCoroutineScope: TestCoroutineScope?, - private var owner: Any? = null -): AbstractCoroutineContextElement(CoroutineExceptionHandler), CoroutineExceptionHandler -{ - private val lock = SynchronizedObject() - - /** - * Claims ownership of this [TestExceptionHandler], or returns its copy, owned and not linked to anything. - */ - fun claimOwnershipOrCopy(owner: Any): TestExceptionHandlerContextElement = synchronized(lock) { - if (this.owner == null && testCoroutineScope == null) { - this.owner = owner - this - } else { - TestExceptionHandlerContextElement(handler, null, owner) - } - } - - /** - * Links a [TestCoroutineScope] to this, unless there's already one linked. - */ - fun registerTestCoroutineScope(owner: Any, scope: TestCoroutineScope) = - synchronized(lock) { - check(this.owner === owner && testCoroutineScope == null) - testCoroutineScope = scope - this.owner = null - } - - @Suppress("INVISIBLE_MEMBER") - override fun handleException(context: CoroutineContext, exception: Throwable) { - val scope = synchronized(lock) { - testCoroutineScope - ?: throw RuntimeException("Attempting to handle an exception using a `TestExceptionHandler` that is not linked to a `TestCoroutineScope`") - } - 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 68d9b6ee65..8d3cf6a023 100644 --- a/kotlinx-coroutines-test/common/test/Helpers.kt +++ b/kotlinx-coroutines-test/common/test/Helpers.kt @@ -7,6 +7,7 @@ package kotlinx.coroutines.test import kotlinx.atomicfu.* import kotlin.test.* import kotlin.time.* +import kotlin.time.Duration.Companion.seconds /** * The number of milliseconds that is sure not to pass [assertRunsFast]. @@ -28,7 +29,7 @@ inline fun assertRunsFast(timeout: Duration, block: () -> T): T { * Asserts that a block completed within two seconds. */ @OptIn(ExperimentalTime::class) -inline fun assertRunsFast(block: () -> T): T = assertRunsFast(Duration.seconds(2), block) +inline fun assertRunsFast(block: () -> T): T = assertRunsFast(2.seconds, block) /** * Passes [test] as an argument to [block], but as a function returning not a [TestResult] but [Unit]. diff --git a/kotlinx-coroutines-test/common/test/RunTestTest.kt b/kotlinx-coroutines-test/common/test/RunTestTest.kt index 03f933a0ec..1d16ba565f 100644 --- a/kotlinx-coroutines-test/common/test/RunTestTest.kt +++ b/kotlinx-coroutines-test/common/test/RunTestTest.kt @@ -269,9 +269,9 @@ class RunTestTest { } }, { runTest { - reportException(TestException("x")) - reportException(TestException("y")) - reportException(TestException("z")) + launch(SupervisorJob()) { throw TestException("x") } + launch(SupervisorJob()) { throw TestException("y") } + launch(SupervisorJob()) { throw TestException("z") } throw TestException("w") } }) diff --git a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt index 15ac46059e..37ce7cc3fd 100644 --- a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt +++ b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt @@ -134,23 +134,13 @@ class TestCoroutineScopeTest { } } - /** 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")) + launch(SupervisorJob()) { throw TestException("x") } + launch(SupervisorJob()) { throw TestException("y") } + launch(SupervisorJob()) { throw TestException("z") } try { cleanupTestCoroutines() fail("should not be reached") @@ -166,6 +156,7 @@ class TestCoroutineScopeTest { companion object { internal val invalidContexts = listOf( Dispatchers.Default, // not a [TestDispatcher] + CoroutineExceptionHandler { _, _ -> }, // not an [UncaughtExceptionCaptor] StandardTestDispatcher() + TestCoroutineScheduler(), // the dispatcher is not linked to the scheduler ) } diff --git a/kotlinx-coroutines-test/common/test/TestExceptionHandlerTest.kt b/kotlinx-coroutines-test/common/test/TestExceptionHandlerTest.kt deleted file mode 100644 index 76e077435a..0000000000 --- a/kotlinx-coroutines-test/common/test/TestExceptionHandlerTest.kt +++ /dev/null @@ -1,112 +0,0 @@ -/* - * 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/jvm/test/MultithreadingTest.kt b/kotlinx-coroutines-test/jvm/test/MultithreadingTest.kt index da70bf5444..90a16d0622 100644 --- a/kotlinx-coroutines-test/jvm/test/MultithreadingTest.kt +++ b/kotlinx-coroutines-test/jvm/test/MultithreadingTest.kt @@ -3,7 +3,6 @@ */ import kotlinx.coroutines.* -import kotlinx.coroutines.flow.* import kotlinx.coroutines.test.* import kotlin.concurrent.* import kotlin.coroutines.* From cb0494b67a4637b2ce4803ed15d68778f6c8b8ae Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Fri, 29 Oct 2021 15:04:09 +0300 Subject: [PATCH 3/5] Fixes --- kotlinx-coroutines-core/common/README.md | 1 + .../src/TestCoroutineExceptionHandler.kt | 6 ++++-- .../common/src/TestCoroutineScope.kt | 8 ++++++-- .../test/migration/TestRunBlockingTest.kt | 18 ++++++++++++++++++ 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/kotlinx-coroutines-core/common/README.md b/kotlinx-coroutines-core/common/README.md index 23ca0a55ca..b00921bbcd 100644 --- a/kotlinx-coroutines-core/common/README.md +++ b/kotlinx-coroutines-core/common/README.md @@ -130,6 +130,7 @@ Low-level primitives for finer-grained control of coroutines. [kotlinx.coroutines.sync.Mutex]: https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.sync/-mutex/index.html [kotlinx.coroutines.sync.Mutex.lock]: https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.sync/-mutex/lock.html +[kotlinx.coroutines.sync.Mutex.tryLock]: https://kotlin.github.io/kotlinx.coroutines/kotlinx-coroutines-core/kotlinx.coroutines.sync/-mutex/try-lock.html diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt b/kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt index e40ef4ed28..b85f21ee69 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt @@ -12,7 +12,8 @@ import kotlin.coroutines.* * Access uncaught coroutine exceptions captured during test execution. */ @Deprecated( - "Consider whether the default mechanism of handling uncaught exceptions is sufficient. " + + "Deprecated for removal without a replacement. " + + "Consider whether the default mechanism of handling uncaught exceptions is sufficient. " + "If not, try writing your own `CoroutineExceptionHandler` and " + "please report your use case at https://github.com/Kotlin/kotlinx.coroutines/issues.", level = DeprecationLevel.WARNING @@ -41,7 +42,8 @@ public interface UncaughtExceptionCaptor { * An exception handler that captures uncaught exceptions in tests. */ @Deprecated( - "It's better to define one's own `CoroutineExceptionHandler` if you just need to handle '" + + "Deprecated for removal without a replacement. " + + "It may be to define one's own `CoroutineExceptionHandler` if you just need to handle '" + "uncaught exceptions without a special `TestCoroutineScope` integration.", level = DeprecationLevel.WARNING ) public class TestCoroutineExceptionHandler : diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt index 066bbe2657..859e82694a 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt @@ -183,7 +183,7 @@ public fun createTestCoroutineScope(context: CoroutineContext = EmptyCoroutineCo is UncaughtExceptionCaptor -> exceptionHandler null -> { val lock = SynchronizedObject() - CoroutineExceptionHandler { context, throwable -> + CoroutineExceptionHandler { _, throwable -> val reported = synchronized(lock) { scope!!.reportException(throwable) } @@ -191,7 +191,11 @@ public fun createTestCoroutineScope(context: CoroutineContext = EmptyCoroutineCo throw throwable // let this exception crash everything } } - else -> throw IllegalArgumentException("A CoroutineExceptionHandler was passed to TestCoroutineScope") + else -> throw IllegalArgumentException( + "A CoroutineExceptionHandler was passed to TestCoroutineScope. " + + "Please pass it as an argument to a `launch` or `async` block on an already-created scope " + + "if uncaught exceptions require special treatment." + ) } val job: Job = context[Job] ?: Job() return TestCoroutineScopeImpl(context + scheduler + dispatcher + exceptionHandler + job).also { diff --git a/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt b/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt index dcae80e0a4..af3b24892a 100644 --- a/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt +++ b/kotlinx-coroutines-test/common/test/migration/TestRunBlockingTest.kt @@ -309,6 +309,15 @@ class TestRunBlockingTest { } } + @Test + fun runBlockingTestBuilder_throwsOnBadHandler() { + assertFailsWith { + runBlockingTest(CoroutineExceptionHandler { _, _ -> }) { + + } + } + } + @Test fun pauseDispatcher_disablesAutoAdvance_forCurrent() = runBlockingTest { var mutable = 0 @@ -419,4 +428,13 @@ class TestRunBlockingTest { fun testOverrideExceptionHandler() = runBlockingTest(exceptionHandler) { assertSame(coroutineContext[CoroutineExceptionHandler], exceptionHandler) } + + @Test + fun testOverrideExceptionHandlerError() { + assertFailsWith { + runBlockingTest(CoroutineExceptionHandler { _, _ -> }) { + fail("Unreached") + } + } + } } \ No newline at end of file From 6b7c614d34e9ebb8b7fadc148a068e5aeeca295f Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 1 Nov 2021 12:52:45 +0300 Subject: [PATCH 4/5] Fix 'TestCoroutineScope.runTest' not working --- .../common/src/TestBuilders.kt | 2 +- .../common/src/TestCoroutineScope.kt | 22 +++++++++++----- .../common/test/RunTestTest.kt | 18 +++++++++++++ .../common/test/TestCoroutineScopeTest.kt | 26 +++++++++++++++++++ 4 files changed, 60 insertions(+), 8 deletions(-) diff --git a/kotlinx-coroutines-test/common/src/TestBuilders.kt b/kotlinx-coroutines-test/common/src/TestBuilders.kt index 912580248f..de06102c5a 100644 --- a/kotlinx-coroutines-test/common/src/TestBuilders.kt +++ b/kotlinx-coroutines-test/common/src/TestBuilders.kt @@ -269,7 +269,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 + * Calls [runTest] using a coroutine context from this [TestCoroutineScope]. The [TestCoroutineScope] used to run the * [block] will be different from this one, but will use its [Job] as a parent. * * Since this function returns [TestResult], in order to work correctly on the JS, its result must be returned diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt index 859e82694a..467fc738ed 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt @@ -179,18 +179,22 @@ public fun createTestCoroutineScope(context: CoroutineContext = EmptyCoroutineCo else -> throw IllegalArgumentException("Dispatcher must implement TestDispatcher: $dispatcher") } var scope: TestCoroutineScopeImpl? = null - val exceptionHandler = when (val exceptionHandler = context[CoroutineExceptionHandler]) { - is UncaughtExceptionCaptor -> exceptionHandler - null -> { - val lock = SynchronizedObject() - CoroutineExceptionHandler { _, throwable -> + val ownExceptionHandler = run { + val lock = SynchronizedObject() + object : AbstractCoroutineContextElement(CoroutineExceptionHandler), TestCoroutineScopeExceptionHandler { + override fun handleException(context: CoroutineContext, exception: Throwable) { val reported = synchronized(lock) { - scope!!.reportException(throwable) + scope!!.reportException(exception) } if (!reported) - throw throwable // let this exception crash everything + throw exception // let this exception crash everything } } + } + val exceptionHandler = when (val exceptionHandler = context[CoroutineExceptionHandler]) { + is UncaughtExceptionCaptor -> exceptionHandler + null -> ownExceptionHandler + is TestCoroutineScopeExceptionHandler -> ownExceptionHandler else -> throw IllegalArgumentException( "A CoroutineExceptionHandler was passed to TestCoroutineScope. " + "Please pass it as an argument to a `launch` or `async` block on an already-created scope " + @@ -203,6 +207,10 @@ public fun createTestCoroutineScope(context: CoroutineContext = EmptyCoroutineCo } } +/** A marker that shows that this [CoroutineExceptionHandler] was created for [TestCoroutineScope]. With this, + * constructing a new [TestCoroutineScope] with the [CoroutineScope.coroutineContext] of an existing one will override + * the exception handler, instead of failing. */ +private interface TestCoroutineScopeExceptionHandler: CoroutineExceptionHandler private inline val CoroutineContext.delayController: DelayController? get() { diff --git a/kotlinx-coroutines-test/common/test/RunTestTest.kt b/kotlinx-coroutines-test/common/test/RunTestTest.kt index 1d16ba565f..a679a7cf6e 100644 --- a/kotlinx-coroutines-test/common/test/RunTestTest.kt +++ b/kotlinx-coroutines-test/common/test/RunTestTest.kt @@ -275,4 +275,22 @@ class RunTestTest { throw TestException("w") } }) + + /** Tests that [TestCoroutineScope.runTest] does not inherit the exception handler and works. */ + @Test + fun testScopeRunTestExceptionHandler(): TestResult { + val scope = createTestCoroutineScope() + return testResultMap({ + try { + it() + fail("should not be reached") + } catch (e: TestException) { + scope.cleanupTestCoroutines() // should not fail + } + }, { + scope.runTest { + launch(SupervisorJob()) { throw TestException("x") } + } + }) + } } diff --git a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt index 37ce7cc3fd..bdea37de49 100644 --- a/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt +++ b/kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt @@ -153,6 +153,32 @@ class TestCoroutineScopeTest { } } + /** Tests that constructing a new [TestCoroutineScope] using another one's scope works and overrides the exception + * handler. */ + @Test + fun testCopyingContexts() { + val deferred = CompletableDeferred() + val scope1 = createTestCoroutineScope() + scope1.launch { deferred.await() } // a pending job in the outer scope + val scope2 = createTestCoroutineScope(scope1.coroutineContext) + val scope3 = createTestCoroutineScope(scope1.coroutineContext) + assertEquals( + scope1.coroutineContext.minusKey(CoroutineExceptionHandler), + scope2.coroutineContext.minusKey(CoroutineExceptionHandler)) + scope2.launch(SupervisorJob()) { throw TestException("x") } // will fail the cleanup of scope2 + try { + scope2.cleanupTestCoroutines() + fail("should not be reached") + } catch (e: TestException) { } + scope3.cleanupTestCoroutines() // the pending job in the outer scope will not cause this to fail + try { + scope1.cleanupTestCoroutines() + fail("should not be reached") + } catch (e: UncompletedCoroutinesError) { + // the pending job in the outer scope + } + } + companion object { internal val invalidContexts = listOf( Dispatchers.Default, // not a [TestDispatcher] From 9f635e9efcc1d8110c78abd58144a6be9208b34a Mon Sep 17 00:00:00 2001 From: Dmitry Khalanskiy Date: Mon, 1 Nov 2021 15:34:22 +0300 Subject: [PATCH 5/5] Remove unneeded exception --- kotlinx-coroutines-test/common/src/TestCoroutineScope.kt | 5 ----- 1 file changed, 5 deletions(-) diff --git a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt index 467fc738ed..e4de60f227 100644 --- a/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt +++ b/kotlinx-coroutines-test/common/src/TestCoroutineScope.kt @@ -107,11 +107,6 @@ 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() }