Change exception handling in the test module

Author: dkhalanskyjb

kotlinx-coroutines-test/api/kotlinx-coroutines-test.api

@@ -51,7 +51,7 @@ public final class kotlinx/coroutines/test/TestCoroutineDispatcher : kotlinx/cor
 
 public final class kotlinx/coroutines/test/TestCoroutineExceptionHandler : kotlin/coroutines/AbstractCoroutineContextElement, kotlinx/coroutines/CoroutineExceptionHandler, kotlinx/coroutines/test/UncaughtExceptionCaptor {
 	public fun <init> ()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
 }
@@ -68,9 +68,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/TestCoroutineScope$DefaultImpls {
@@ -85,6 +86,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
@@ -104,8 +106,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;
 }
 

kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt

@@ -11,13 +11,16 @@ import kotlin.coroutines.*
 /**
  * Access uncaught coroutine exceptions captured during test execution.
  */
-@ExperimentalCoroutinesApi // Since 1.2.1, tentatively till 1.3.0
+@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<Throwable>
 
@@ -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 // Since 1.2.1, tentatively till 1.3.0
+@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<Throwable>()
     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<Throwable>
+    public override val uncaughtExceptions: List<Throwable>
         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() }

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 // Since 1.2.1, tentatively till 1.3.0
-public interface TestCoroutineScope: CoroutineScope, UncaughtExceptionCaptor {
+public 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 UncompletedCoroutinesError if any pending tasks are active.
+     * @throws IllegalStateException if called more than once.
      */
     @ExperimentalCoroutinesApi
     public fun cleanupTestCoroutines()
@@ -31,23 +43,56 @@ public interface TestCoroutineScope: CoroutineScope, UncaughtExceptionCaptor {
     public val testScheduler: TestCoroutineScheduler
         get() = coroutineContext[TestCoroutineScheduler]
             ?: throw UnsupportedOperationException("This scope does not have a TestCoroutineScheduler linked to it")
+
+    /**
+     * 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 printed to the
+     * console.
+     *
+     * @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,
     val ownJob: CompletableJob?
-):
-    TestCoroutineScope,
-    UncaughtExceptionCaptor by coroutineContext.uncaughtExceptionCaptor
+): TestCoroutineScope
 {
+    private val lock = SynchronizedObject()
+    private var exceptions = mutableListOf<Throwable>()
+    private var cleanedUp = false
+
+    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)
+            exceptions.add(throwable)
+        }
+    }
+
     override val testScheduler: TestCoroutineScheduler
         get() = coroutineContext[TestCoroutineScheduler]!!
 
     /** These jobs existed before the coroutine scope was used, so it's alright if they don't get cancelled. */
     val initialJobs = coroutineContext.activeJobs()
 
     override fun cleanupTestCoroutines() {
-        coroutineContext.uncaughtExceptionCaptor.cleanupTestCoroutinesCaptor()
+        synchronized(lock) {
+            if (cleanedUp)
+                throw IllegalStateException("Attempting to clean up a test coroutine scope more than once.")
+            cleanedUp = true
+        }
+        exceptions.apply {
+            drop(1).forEach { it.printStackTrace() }
+            singleOrNull()?.let { throw it }
+        }
+        (coroutineContext[CoroutineExceptionHandler] as? UncaughtExceptionCaptor)?.cleanupTestCoroutines()
         val delayController = coroutineContext.delayController
         if (delayController != null) {
             delayController.cleanupTestCoroutines()
@@ -79,13 +124,13 @@ private fun CoroutineContext.activeJobs(): Set<Job> {
  */
 @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)
 }
 
 /**
@@ -128,14 +173,8 @@ 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"
-            }
-        }
-        this ?: TestCoroutineExceptionHandler()
-    }
+    val exceptionHandler = context[CoroutineExceptionHandler]
+        ?: TestExceptionHandler { _, throwable -> reportException(throwable) }
     val job: Job
     val ownJob: CompletableJob?
     if (context[Job] == null) {
@@ -146,17 +185,9 @@ public fun createTestCoroutineScope(context: CoroutineContext = EmptyCoroutineCo
         job = context[Job]!!
     }
     return TestCoroutineScopeImpl(context + scheduler + dispatcher + exceptionHandler + job, ownJob)
+        .also { (exceptionHandler as? TestExceptionHandlerContextElement)?.tryRegisterTestCoroutineScope(it) }
 }
 
-private 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() {
         val handler = this[ContinuationInterceptor]
@@ -248,6 +279,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<Throwable>
+    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")

kotlinx-coroutines-test/common/src/TestExceptionHandler.kt

@@ -0,0 +1,58 @@
+/*
+ * 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, and passing it to [TestCoroutineScope]
+ * will not lead to it re-linking.
+ */
+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?
+): AbstractCoroutineContextElement(CoroutineExceptionHandler), CoroutineExceptionHandler
+{
+    private val lock = SynchronizedObject()
+
+    /**
+     * Links a [TestCoroutineScope] to this, unless there's already one linked.
+     */
+    fun tryRegisterTestCoroutineScope(scope: TestCoroutineScope): Boolean =
+        synchronized(lock) {
+            if (testCoroutineScope != null) {
+                false
+            } else {
+                testCoroutineScope = scope
+                true
+            }
+        }
+
+    override fun handleException(context: CoroutineContext, exception: Throwable) {
+        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. */
+    }
+}

kotlinx-coroutines-test/common/test/TestCoroutineScopeTest.kt

@@ -124,7 +124,6 @@ class TestCoroutineScopeTest {
         internal val invalidContexts = listOf(
             Dispatchers.Default, // not a [TestDispatcher]
             StandardTestDispatcher() + TestCoroutineScheduler(), // the dispatcher is not linked to the scheduler
-            CoroutineExceptionHandler { _, _ -> }, // not an `UncaughtExceptionCaptor`
         )
     }
 }

kotlinx-coroutines-test/common/test/TestCoroutineExceptionHandlerTest.kt renamed to 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() {