Skip to content

Update exception handling in the test module #2953

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions kotlinx-coroutines-test/api/kotlinx-coroutines-test.api
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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;
}
Expand All @@ -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
Expand All @@ -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;
}

32 changes: 22 additions & 10 deletions kotlinx-coroutines-test/common/src/TestBuilders.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
}
Expand All @@ -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()
}
}

Expand All @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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()
}
35 changes: 24 additions & 11 deletions kotlinx-coroutines-test/common/src/TestCoroutineExceptionHandler.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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>

Expand All @@ -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() }
Expand Down
Loading