Skip to content

Commit d651dec

Browse files
dkhalanskyjbdkhalanskyjb
committed
Add tests and various fixes
1 parent 84af26d commit d651dec

File tree

6 files changed

+222
-25
lines changed

6 files changed

+222
-25
lines changed

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

+38-18
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,7 @@ private class TestCoroutineScopeImpl (
6969
override fun reportException(throwable: Throwable) {
7070
synchronized(lock) {
7171
if (cleanedUp)
72-
throw IllegalStateException(
73-
"Attempting to report an uncaught exception after the test coroutine scope was already cleaned up",
74-
throwable)
72+
throw ExceptionReportAfterCleanup(throwable)
7573
exceptions.add(throwable)
7674
}
7775
}
@@ -83,6 +81,17 @@ private class TestCoroutineScopeImpl (
8381
val initialJobs = coroutineContext.activeJobs()
8482

8583
override fun cleanupTestCoroutines() {
84+
(coroutineContext[CoroutineExceptionHandler] as? UncaughtExceptionCaptor)?.cleanupTestCoroutines()
85+
val delayController = coroutineContext.delayController
86+
var hasUncompletedJobs = false
87+
if (delayController != null) {
88+
delayController.cleanupTestCoroutines()
89+
} else {
90+
testScheduler.runCurrent()
91+
if (!testScheduler.isIdle()) {
92+
hasUncompletedJobs = true
93+
}
94+
}
8695
synchronized(lock) {
8796
if (cleanedUp)
8897
throw IllegalStateException("Attempting to clean up a test coroutine scope more than once.")
@@ -92,18 +101,11 @@ private class TestCoroutineScopeImpl (
92101
drop(1).forEach { it.printStackTrace() }
93102
singleOrNull()?.let { throw it }
94103
}
95-
(coroutineContext[CoroutineExceptionHandler] as? UncaughtExceptionCaptor)?.cleanupTestCoroutines()
96-
val delayController = coroutineContext.delayController
97-
if (delayController != null) {
98-
delayController.cleanupTestCoroutines()
99-
} else {
100-
testScheduler.runCurrent()
101-
if (!testScheduler.isIdle()) {
102-
throw UncompletedCoroutinesError(
103-
"Unfinished coroutines during teardown. Ensure all coroutines are" +
104-
" completed or cancelled by your test."
105-
)
106-
}
104+
if (hasUncompletedJobs) {
105+
throw UncompletedCoroutinesError(
106+
"Unfinished coroutines during teardown. Ensure all coroutines are" +
107+
" completed or cancelled by your test."
108+
)
107109
}
108110
val jobs = coroutineContext.activeJobs()
109111
if ((jobs - initialJobs).isNotEmpty()) {
@@ -113,6 +115,11 @@ private class TestCoroutineScopeImpl (
113115
}
114116
}
115117

118+
internal class ExceptionReportAfterCleanup(cause: Throwable): IllegalStateException(
119+
"Attempting to report an uncaught exception after the test coroutine scope was already cleaned up",
120+
cause
121+
)
122+
116123
private fun CoroutineContext.activeJobs(): Set<Job> {
117124
return checkNotNull(this[Job]).children.filter { it.isActive }.toSet()
118125
}
@@ -173,10 +180,23 @@ public fun createTestCoroutineScope(context: CoroutineContext = EmptyCoroutineCo
173180
}
174181
else -> throw IllegalArgumentException("Dispatcher must implement TestDispatcher: $dispatcher")
175182
}
176-
val exceptionHandler = context[CoroutineExceptionHandler]
177-
?: TestExceptionHandler { _, throwable -> reportException(throwable) }
183+
val linkedHandler: TestExceptionHandlerContextElement?
184+
val exceptionHandler: CoroutineExceptionHandler
178185
val handlerOwner = Any()
179-
val linkedHandler = (exceptionHandler as? TestExceptionHandlerContextElement)?.claimOwnershipOrCopy(handlerOwner)
186+
when (val exceptionHandlerInCtx = context[CoroutineExceptionHandler]) {
187+
null -> {
188+
linkedHandler = TestExceptionHandlerContextElement(
189+
{ _, throwable -> reportException(throwable) },
190+
null,
191+
handlerOwner)
192+
exceptionHandler = linkedHandler
193+
}
194+
else -> {
195+
linkedHandler = (exceptionHandlerInCtx as? TestExceptionHandlerContextElement
196+
)?.claimOwnershipOrCopy(handlerOwner)
197+
exceptionHandler = linkedHandler ?: exceptionHandlerInCtx
198+
}
199+
}
180200
val job: Job
181201
val ownJob: CompletableJob?
182202
if (context[Job] == null) {

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

+20-4
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,14 @@ import kotlin.coroutines.*
1919
*
2020
* If [linkedScope] is `null`, the [CoroutineExceptionHandler] returned from this function has special behavior when
2121
* passed to [createTestCoroutineScope]: the newly-created scope is linked to this handler. If [linkedScope] is not
22-
* null, then the resulting [CoroutineExceptionHandler] will be linked to it.
22+
* null, then the resulting [CoroutineExceptionHandler] will be linked to it; however, it will *not* be part of the
23+
* coroutine context of the [TestCoroutineScope], and this will only affect the receiver of [handler].
2324
*
2425
* Passing an already-linked instance to [TestCoroutineScope] will lead to it making its own copy with the same
2526
* [handler].
27+
*
28+
* Throwing inside [handler] is permitted. The thrown exception will we [reported][TestCoroutineScope.reportException]
29+
* to the [TestCoroutineScope]. This is done so to simplify using assertions inside [handler].
2630
*/
2731
public fun TestExceptionHandler(
2832
linkedScope: TestCoroutineScope? = null,
@@ -60,11 +64,23 @@ internal class TestExceptionHandlerContextElement(
6064
this.owner = null
6165
}
6266

67+
@Suppress("INVISIBLE_MEMBER")
6368
override fun handleException(context: CoroutineContext, exception: Throwable) {
64-
synchronized(lock) {
69+
val scope = synchronized(lock) {
6570
testCoroutineScope
6671
?: throw RuntimeException("Attempting to handle an exception using a `TestExceptionHandler` that is not linked to a `TestCoroutineScope`")
67-
}.handler(context, exception)
68-
/** it's okay if [handler] throws: [handleCoroutineException] deals with this. */
72+
}
73+
try {
74+
scope.handler(context, exception)
75+
} catch (e: ExceptionReportAfterCleanup) {
76+
// can only be thrown if the test coroutine scope is already closed.
77+
handleCoroutineExceptionImpl(context, e)
78+
} catch (e: Throwable) {
79+
try {
80+
scope.reportException(e)
81+
} catch (_: ExceptionReportAfterCleanup) {
82+
handleCoroutineExceptionImpl(context, e)
83+
}
84+
}
6985
}
7086
}

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

+3-1
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,6 @@ open class OrderedExecutionTestBase {
6262
}
6363
}
6464

65-
internal fun <T> T.void() { }
65+
internal fun <T> T.void() { }
66+
67+
internal class TestException(message: String? = null): Exception(message)

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

+49
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,55 @@ class TestCoroutineScopeTest {
120120
assertFalse(handlerCalled)
121121
}
122122

123+
/** Tests that uncaught exceptions are thrown at the cleanup. */
124+
@Test
125+
fun testThrowsUncaughtExceptionsOnCleanup() {
126+
val scope = createTestCoroutineScope(SupervisorJob())
127+
val exception = TestException("test")
128+
scope.launch {
129+
throw exception
130+
}
131+
assertFailsWith<TestException> {
132+
scope.cleanupTestCoroutines()
133+
}
134+
}
135+
136+
/** Tests that uncaught exceptions take priority over uncompleted jobs when throwing on cleanup. */
137+
@Test
138+
fun testUncaughtExceptionsPrioritizedOnCleanup() {
139+
val scope = createTestCoroutineScope(SupervisorJob())
140+
val exception = TestException("test")
141+
scope.launch {
142+
throw exception
143+
}
144+
scope.launch {
145+
delay(1000)
146+
}
147+
assertFailsWith<TestException> {
148+
scope.cleanupTestCoroutines()
149+
}
150+
}
151+
152+
/** Tests that cleaning up twice is forbidden. */
153+
@Test
154+
fun testClosingTwice() {
155+
val scope = createTestCoroutineScope()
156+
scope.cleanupTestCoroutines()
157+
assertFailsWith<IllegalStateException> {
158+
scope.cleanupTestCoroutines()
159+
}
160+
}
161+
162+
/** Tests that throwing after cleaning up is forbidden. */
163+
@Test
164+
fun testReportingAfterClosing() {
165+
val scope = createTestCoroutineScope()
166+
scope.cleanupTestCoroutines()
167+
assertFailsWith<IllegalStateException> {
168+
scope.reportException(TestException())
169+
}
170+
}
171+
123172
companion object {
124173
internal val invalidContexts = listOf(
125174
Dispatchers.Default, // not a [TestDispatcher]

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

+112
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/*
2+
* Copyright 2016-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
3+
*/
4+
5+
package kotlinx.coroutines.test
6+
7+
import kotlinx.coroutines.*
8+
import kotlin.coroutines.*
9+
import kotlin.random.*
10+
import kotlin.test.*
11+
12+
class TestExceptionHandlerTest {
13+
14+
/** Tests that passing a [TestExceptionHandler] to [TestCoroutineScope] overrides the exception handling there. */
15+
@Test
16+
fun testPassingToCoroutineScope() {
17+
var enteredHandler = false
18+
var observedScope: TestCoroutineScope? = null
19+
val handler = TestExceptionHandler { _, throwable ->
20+
assertTrue(throwable is TestException)
21+
observedScope = this
22+
enteredHandler = true
23+
}
24+
val scope = createTestCoroutineScope(handler + SupervisorJob())
25+
scope.launch {
26+
throw TestException("test")
27+
}
28+
scope.runCurrent()
29+
assertTrue(enteredHandler)
30+
assertSame(scope, observedScope)
31+
scope.cleanupTestCoroutines()
32+
}
33+
34+
/** Tests that passing a [TestCoroutineScope] to the [TestExceptionHandler] will link, but won't affect the
35+
* coroutine context of [TestCoroutineScope]. */
36+
@Test
37+
fun testExplicitLinking() {
38+
var observedScope: TestCoroutineScope? = null
39+
val scope = createTestCoroutineScope(SupervisorJob())
40+
val handler = TestExceptionHandler(scope) { _, throwable ->
41+
assertTrue(throwable is TestException)
42+
observedScope = this
43+
}
44+
scope.launch(handler) {
45+
throw TestException("test1")
46+
}
47+
scope.launch {
48+
throw TestException("test2")
49+
}
50+
scope.runCurrent()
51+
assertSame(scope, observedScope)
52+
try {
53+
scope.cleanupTestCoroutines()
54+
throw AssertionError("won't reach")
55+
} catch (e: TestException) {
56+
assertEquals("test2", e.message)
57+
}
58+
}
59+
60+
/** Tests that passing a [TestExceptionHandler] that's already linked to another [TestCoroutineScope] will cause it
61+
* to be copied. */
62+
@Test
63+
fun testRelinking() {
64+
val encountered = mutableListOf<TestCoroutineScope>()
65+
val handler = TestExceptionHandler { _, throwable ->
66+
assertTrue(throwable is TestException)
67+
encountered.add(this)
68+
}
69+
val scopes = List(3) { createTestCoroutineScope(handler + SupervisorJob()) }
70+
val events = List(10) { Random.nextInt(0, 3) }.map { scopes[it] }
71+
events.forEach {
72+
it.launch {
73+
throw TestException()
74+
}
75+
it.runCurrent()
76+
}
77+
assertEquals(events, encountered)
78+
}
79+
80+
/** Tests that throwing inside [TestExceptionHandler] is reported. */
81+
@Test
82+
fun testThrowingInsideHandler() {
83+
val handler = TestExceptionHandler { _, throwable ->
84+
assertEquals("y", throwable.message)
85+
throw TestException("x")
86+
}
87+
val scope = createTestCoroutineScope(handler)
88+
scope.launch {
89+
throw TestException("y")
90+
}
91+
scope.runCurrent()
92+
try {
93+
scope.cleanupTestCoroutines()
94+
throw AssertionError("can't be reached")
95+
} catch (e: TestException) {
96+
assertEquals("x", e.message)
97+
}
98+
}
99+
100+
/** Tests that throwing inside [TestExceptionHandler] after the scope is cleaned up leads to problems. */
101+
@Test
102+
@Ignore // difficult to check on JS and Native, where the error is simply logged
103+
fun testThrowingInsideHandlerAfterCleanup() {
104+
val handler = TestExceptionHandler { _, throwable ->
105+
reportException(throwable)
106+
}
107+
val scope = createTestCoroutineScope(handler)
108+
scope.cleanupTestCoroutines()
109+
handler.handleException(EmptyCoroutineContext, TestException())
110+
}
111+
112+
}

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

-2
Original file line numberDiff line numberDiff line change
@@ -420,5 +420,3 @@ class TestRunBlockingTest {
420420
assertSame(coroutineContext[CoroutineExceptionHandler], exceptionHandler)
421421
}
422422
}
423-
424-
private class TestException(message: String? = null): Exception(message)

0 commit comments

Comments
 (0)