Skip to content

Commit 4f1d7df

Browse files
dkhalanskyjbdkhalanskyjb
committed
WIP: fix exception handling in the test module
1 parent 955ef56 commit 4f1d7df

File tree

6 files changed

+78
-82
lines changed

6 files changed

+78
-82
lines changed

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

+5-4
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,16 @@ public final class kotlinx/coroutines/test/TestCoroutineDispatcher : kotlinx/cor
3434
public fun toString ()Ljava/lang/String;
3535
}
3636

37-
public final class kotlinx/coroutines/test/TestCoroutineExceptionHandler : kotlin/coroutines/AbstractCoroutineContextElement, kotlinx/coroutines/CoroutineExceptionHandler, kotlinx/coroutines/test/UncaughtExceptionCaptor {
37+
public final class kotlinx/coroutines/test/TestCoroutineExceptionHandler : kotlin/coroutines/AbstractCoroutineContextElement, kotlinx/coroutines/CoroutineExceptionHandler {
3838
public fun <init> ()V
39-
public fun cleanupTestCoroutinesCaptor ()V
40-
public fun getUncaughtExceptions ()Ljava/util/List;
39+
public final fun cleanupTestCoroutinesCaptor ()V
40+
public final fun getUncaughtExceptions ()Ljava/util/List;
4141
public fun handleException (Lkotlin/coroutines/CoroutineContext;Ljava/lang/Throwable;)V
4242
}
4343

44-
public abstract interface class kotlinx/coroutines/test/TestCoroutineScope : kotlinx/coroutines/CoroutineScope, kotlinx/coroutines/test/DelayController, kotlinx/coroutines/test/UncaughtExceptionCaptor {
44+
public abstract interface class kotlinx/coroutines/test/TestCoroutineScope : kotlinx/coroutines/CoroutineScope, kotlinx/coroutines/test/DelayController {
4545
public abstract fun cleanupTestCoroutines ()V
46+
public abstract fun getUncaughtExceptions ()Ljava/util/List;
4647
}
4748

4849
public final class kotlinx/coroutines/test/TestCoroutineScopeKt {

‎kotlinx-coroutines-test/common/src/TestBuilders.kt

+20-19
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,20 @@ import kotlin.coroutines.*
4343
*/
4444
@ExperimentalCoroutinesApi // Since 1.2.1, tentatively till 1.3.0
4545
public fun runBlockingTest(context: CoroutineContext = EmptyCoroutineContext, testBody: suspend TestCoroutineScope.() -> Unit) {
46-
val (safeContext, dispatcher) = context.checkArguments()
46+
val (safeContext, dispatcher) = context.checkTestScopeArguments()
4747
val startingJobs = safeContext.activeJobs()
4848
val scope = TestCoroutineScope(safeContext)
49-
val deferred = scope.async {
50-
scope.testBody()
51-
}
52-
dispatcher.advanceUntilIdle()
53-
deferred.getCompletionExceptionOrNull()?.let {
54-
throw it
49+
try {
50+
val deferred = scope.async {
51+
scope.testBody()
52+
}
53+
dispatcher.advanceUntilIdle()
54+
deferred.getCompletionExceptionOrNull()?.let {
55+
throw it
56+
}
57+
} catch (e: CancellationException) {
58+
scope.cleanupTestCoroutines()
59+
throw e
5560
}
5661
scope.cleanupTestCoroutines()
5762
val endingJobs = safeContext.activeJobs()
@@ -79,17 +84,13 @@ public fun TestCoroutineScope.runBlockingTest(block: suspend TestCoroutineScope.
7984
public fun TestCoroutineDispatcher.runBlockingTest(block: suspend TestCoroutineScope.() -> Unit): Unit =
8085
runBlockingTest(this, block)
8186

82-
private fun CoroutineContext.checkArguments(): Pair<CoroutineContext, DelayController> {
83-
val dispatcher = when (val dispatcher = get(ContinuationInterceptor)) {
84-
is DelayController -> dispatcher
85-
null -> TestCoroutineDispatcher()
86-
else -> throw IllegalArgumentException("Dispatcher must implement DelayController: $dispatcher")
87-
}
88-
val exceptionHandler = when (val handler = get(CoroutineExceptionHandler)) {
89-
is UncaughtExceptionCaptor -> handler
90-
null -> TestCoroutineExceptionHandler()
91-
else -> throw IllegalArgumentException("coroutineExceptionHandler must implement UncaughtExceptionCaptor: $handler")
87+
internal fun CoroutineContext.checkTestScopeArguments(): Pair<CoroutineContext, DelayController> {
88+
// TODO optimize it
89+
val dispatcher = get(ContinuationInterceptor).run {
90+
this?.let { require(this is DelayController) { "Dispatcher must implement DelayController: $this" } }
91+
this ?: TestCoroutineDispatcher()
9292
}
93-
val job = get(Job) ?: SupervisorJob()
94-
return Pair(this + dispatcher + exceptionHandler + job, dispatcher)
93+
val exceptionHandler = get(CoroutineExceptionHandler) ?: TestCoroutineExceptionHandler()
94+
val job = get(Job) ?: Job()
95+
return Pair(this + dispatcher + exceptionHandler + job, dispatcher as DelayController)
9596
}

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

+14-8
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import kotlin.coroutines.*
1111
/**
1212
* Access uncaught coroutine exceptions captured during test execution.
1313
*/
14-
@ExperimentalCoroutinesApi // Since 1.2.1, tentatively till 1.3.0
14+
@Deprecated("Consider whether a plain `CoroutineExceptionHandler` would suffice. If not, please report your use case at https://github.com/Kotlin/kotlinx.coroutines/issues.", level = DeprecationLevel.ERROR)
1515
public interface UncaughtExceptionCaptor {
1616
/**
1717
* List of uncaught coroutine exceptions.
@@ -34,28 +34,34 @@ public interface UncaughtExceptionCaptor {
3434

3535
/**
3636
* An exception handler that captures uncaught exceptions in tests.
37+
*
38+
* In order to work as intended, this exception handler requires that the [Job] of the test coroutine scope is a
39+
* [SupervisorJob].
3740
*/
38-
@ExperimentalCoroutinesApi // Since 1.2.1, tentatively till 1.3.0
3941
public class TestCoroutineExceptionHandler :
40-
AbstractCoroutineContextElement(CoroutineExceptionHandler), UncaughtExceptionCaptor, CoroutineExceptionHandler
42+
AbstractCoroutineContextElement(CoroutineExceptionHandler), CoroutineExceptionHandler
4143
{
4244
private val _exceptions = mutableListOf<Throwable>()
4345
private val _lock = SynchronizedObject()
46+
private var _coroutinesCleanedUp = false
4447

45-
/** @suppress **/
48+
@Suppress("INVISIBLE_MEMBER")
4649
override fun handleException(context: CoroutineContext, exception: Throwable) {
4750
synchronized(_lock) {
51+
if (_coroutinesCleanedUp) {
52+
handleCoroutineExceptionImpl(context, exception)
53+
return
54+
}
4855
_exceptions += exception
4956
}
5057
}
5158

52-
/** @suppress **/
53-
override val uncaughtExceptions: List<Throwable>
59+
public val uncaughtExceptions: List<Throwable>
5460
get() = synchronized(_lock) { _exceptions.toList() }
5561

56-
/** @suppress **/
57-
override fun cleanupTestCoroutinesCaptor() {
62+
public fun cleanupTestCoroutinesCaptor() {
5863
synchronized(_lock) {
64+
_coroutinesCleanedUp = true
5965
val exception = _exceptions.firstOrNull() ?: return
6066
// log the rest
6167
_exceptions.drop(1).forEach { it.printStackTrace() }

‎kotlinx-coroutines-test/common/src/TestCoroutineScope.kt

+34-20
Original file line numberDiff line numberDiff line change
@@ -11,29 +11,56 @@ import kotlin.coroutines.*
1111
* A scope which provides detailed control over the execution of coroutines for tests.
1212
*/
1313
@ExperimentalCoroutinesApi // Since 1.2.1, tentatively till 1.3.0
14-
public interface TestCoroutineScope: CoroutineScope, UncaughtExceptionCaptor, DelayController {
14+
public interface TestCoroutineScope: CoroutineScope, DelayController {
1515
/**
16-
* Call after the test completes.
17-
* Calls [UncaughtExceptionCaptor.cleanupTestCoroutinesCaptor] and [DelayController.cleanupTestCoroutines].
16+
* Is being called after a test completes.
17+
*
18+
* Calls [DelayController.cleanupTestCoroutines].
19+
*
20+
* If the [CoroutineExceptionHandler] is a [TestCoroutineExceptionHandler], its
21+
* [TestCoroutineExceptionHandler.cleanupTestCoroutines] behavior is performed:
22+
* the first exception in [uncaughtExceptions] is rethrown, and all the other exceptions are
23+
* printed using [Throwable.printStackTrace].
1824
*
1925
* @throws Throwable the first uncaught exception, if there are any uncaught exceptions.
2026
* @throws UncompletedCoroutinesError if any pending tasks are active, however it will not throw for suspended
2127
* coroutines.
2228
*/
2329
public override fun cleanupTestCoroutines()
30+
31+
/**
32+
* List of uncaught coroutine exceptions.
33+
*
34+
* Exceptions are only collected in this list if [TestCoroutineExceptionHandler] is in the test context and this
35+
* scope's job is a [SupervisorJob].
36+
* Otherwise, the failure of one child cancels all the others and the scope itself.
37+
*
38+
* The returned list is a copy of the exceptions caught during execution.
39+
* During [cleanupTestCoroutines] the first element of this list is rethrown if it is not empty.
40+
*/
41+
@Deprecated(
42+
"This list is only populated if `TestCoroutineExceptionHandler` in the test context, and so can be " +
43+
"easily misused. It is only present for backward compatibility and will be removed in the subsequent " +
44+
"releases. If you need to check the list of exceptions, please consider your own" +
45+
"`CoroutineExceptionHandler`.",
46+
level = DeprecationLevel.WARNING)
47+
public val uncaughtExceptions: List<Throwable>
2448
}
2549

2650
private class TestCoroutineScopeImpl (
2751
override val coroutineContext: CoroutineContext
2852
):
2953
TestCoroutineScope,
30-
UncaughtExceptionCaptor by coroutineContext.uncaughtExceptionCaptor,
3154
DelayController by coroutineContext.delayController
3255
{
3356
override fun cleanupTestCoroutines() {
34-
coroutineContext.uncaughtExceptionCaptor.cleanupTestCoroutinesCaptor()
57+
(coroutineContext[CoroutineExceptionHandler] as? TestCoroutineExceptionHandler)?.cleanupTestCoroutinesCaptor()
3558
coroutineContext.delayController.cleanupTestCoroutines()
3659
}
60+
61+
override val uncaughtExceptions: List<Throwable>
62+
get() = (coroutineContext[CoroutineExceptionHandler] as? TestCoroutineExceptionHandler)?.uncaughtExceptions
63+
?: emptyList()
3764
}
3865

3966
/**
@@ -46,21 +73,8 @@ private class TestCoroutineScopeImpl (
4673
*/
4774
@Suppress("FunctionName")
4875
@ExperimentalCoroutinesApi // Since 1.2.1, tentatively till 1.3.0
49-
public fun TestCoroutineScope(context: CoroutineContext = EmptyCoroutineContext): TestCoroutineScope {
50-
var safeContext = context
51-
if (context[ContinuationInterceptor] == null) safeContext += TestCoroutineDispatcher()
52-
if (context[CoroutineExceptionHandler] == null) safeContext += TestCoroutineExceptionHandler()
53-
return TestCoroutineScopeImpl(safeContext)
54-
}
55-
56-
private inline val CoroutineContext.uncaughtExceptionCaptor: UncaughtExceptionCaptor
57-
get() {
58-
val handler = this[CoroutineExceptionHandler]
59-
return handler as? UncaughtExceptionCaptor ?: throw IllegalArgumentException(
60-
"TestCoroutineScope requires a UncaughtExceptionCaptor such as " +
61-
"TestCoroutineExceptionHandler as the CoroutineExceptionHandler"
62-
)
63-
}
76+
public fun TestCoroutineScope(context: CoroutineContext = EmptyCoroutineContext): TestCoroutineScope =
77+
TestCoroutineScopeImpl(context.checkTestScopeArguments().first)
6478

6579
private inline val CoroutineContext.delayController: DelayController
6680
get() {

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

-8
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,6 @@ import kotlinx.coroutines.*
88
import kotlin.test.*
99

1010
class TestCoroutineScopeTest {
11-
@Test
12-
fun whenGivenInvalidExceptionHandler_throwsException() {
13-
val handler = CoroutineExceptionHandler { _, _ -> }
14-
assertFails {
15-
TestCoroutineScope(handler)
16-
}
17-
}
18-
1911
@Test
2012
fun whenGivenInvalidDispatcher_throwsException() {
2113
assertFails {

‎kotlinx-coroutines-test/common/test/TestRunBlockingTest.kt

+5-23
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,6 @@ class TestRunBlockingTest {
128128

129129
@Test
130130
fun whenUsingTimeout_inAsync_doesNotTriggerWhenNotDelayed() = runBlockingTest {
131-
val testScope = this
132131
val deferred = async {
133132
withTimeout(SLOW) {
134133
delay(0)
@@ -195,7 +194,7 @@ class TestRunBlockingTest {
195194

196195
assertRunsFast {
197196
job.join()
198-
throw job.getCancellationException().cause ?: assertFails { "expected exception" }
197+
throw job.getCancellationException().cause ?: TestException("expected exception")
199198
}
200199
}
201200
}
@@ -235,12 +234,13 @@ class TestRunBlockingTest {
235234
fun callingAsyncFunction_executesAsyncBlockImmediately() = runBlockingTest {
236235
assertRunsFast {
237236
var executed = false
238-
async {
237+
val deferred = async {
239238
delay(SLOW)
240239
executed = true
241240
}
242241
advanceTimeBy(SLOW)
243242

243+
assertTrue(deferred.isCompleted)
244244
assertTrue(executed)
245245
}
246246
}
@@ -308,15 +308,6 @@ class TestRunBlockingTest {
308308
}
309309
}
310310

311-
@Test
312-
fun runBlockingTestBuilder_throwsOnBadHandler() {
313-
assertFailsWith<IllegalArgumentException> {
314-
runBlockingTest(CoroutineExceptionHandler { _, _ -> }) {
315-
316-
}
317-
}
318-
}
319-
320311
@Test
321312
fun pauseDispatcher_disablesAutoAdvance_forCurrent() = runBlockingTest {
322313
var mutable = 0
@@ -366,7 +357,7 @@ class TestRunBlockingTest {
366357
runBlockingTest {
367358
val expectedError = TestException("hello")
368359

369-
val job = launch {
360+
launch {
370361
throw expectedError
371362
}
372363

@@ -427,15 +418,6 @@ class TestRunBlockingTest {
427418
fun testOverrideExceptionHandler() = runBlockingTest(exceptionHandler) {
428419
assertSame(coroutineContext[CoroutineExceptionHandler], exceptionHandler)
429420
}
430-
431-
@Test
432-
fun testOverrideExceptionHandlerError() {
433-
assertFailsWith<IllegalArgumentException> {
434-
runBlockingTest(CoroutineExceptionHandler { _, _ -> }) {
435-
fail("Unreached")
436-
}
437-
}
438-
}
439421
}
440422

441-
private class TestException(message: String? = null): Exception(message)
423+
private class TestException(message: String? = null): Exception(message)

0 commit comments

Comments
 (0)