Update exception handling in the test module (#2953)

Author: dkhalanskyjb

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 <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
 }
@@ -66,7 +66,7 @@ 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;
 }
@@ -79,6 +79,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
@@ -98,7 +99,7 @@ public final class kotlinx/coroutines/test/TestDispatchers {
 }
 
 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/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,15 +248,15 @@ 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) {
                 it.addSuppressed(e)
             }
             throw it
         }
-        testScope.cleanupTestCoroutines()
+        testScope.cleanup()
     }
 }
 
@@ -266,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
@@ -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<RunningInRunTest>, CoroutineContext.Element {
+private object RunningInRunTest : CoroutineContext.Key<RunningInRunTest>, 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<T>(
     private val testScope: TestCoroutineScope,
-) : AbstractCoroutine<T>(testScope.coroutineContext, initParentJob = true, active = true), TestCoroutineScope,
-    UncaughtExceptionCaptor by testScope.coroutineContext.uncaughtExceptionCaptor
-{
+) : AbstractCoroutine<T>(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."
+        )
 
+    fun cleanup() = testScope.cleanupTestCoroutines()
 }

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

@@ -11,13 +11,19 @@ import kotlin.coroutines.*
 /**
  * Access uncaught coroutine exceptions captured during test execution.
  */
-@ExperimentalCoroutinesApi
+@Deprecated(
+    "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
+)
 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 +35,40 @@ 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(
+    "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 :
-    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() }