Skip to content

Commit 84c65a8

Browse files
qwwdfsadpablobaxter
qwwdfsad
authored and
pablobaxter
committed
Fix js tests (Kotlin#2824)
* Properly detect asynchronous tests that try to use `runTest` multiple times * Refactor multi-shot tests * Wrap them in 'withContext(Job)' instead of 'runTest' to be properly working on JS * Skip few way too slow for JS tests only on JS platform Fixes Kotlin#2820
1 parent 691261e commit 84c65a8

File tree

7 files changed

+145
-74
lines changed

7 files changed

+145
-74
lines changed

‎kotlinx-coroutines-core/common/test/CancelledParentAttachTest.kt

+80-50
Original file line numberDiff line numberDiff line change
@@ -11,75 +11,105 @@ import kotlin.test.*
1111
class CancelledParentAttachTest : TestBase() {
1212

1313
@Test
14-
fun testAsync() = CoroutineStart.values().forEach(::testAsyncCancelledParent)
14+
fun testAsync() = runTest {
15+
CoroutineStart.values().forEach { testAsyncCancelledParent(it) }
16+
}
1517

16-
private fun testAsyncCancelledParent(start: CoroutineStart) =
17-
runTest({ it is CancellationException }) {
18-
cancel()
19-
expect(1)
20-
val d = async<Int>(start = start) { 42 }
21-
expect(2)
22-
d.invokeOnCompletion {
23-
finish(3)
24-
reset()
18+
private suspend fun testAsyncCancelledParent(start: CoroutineStart) {
19+
try {
20+
withContext(Job()) {
21+
cancel()
22+
expect(1)
23+
val d = async<Int>(start = start) { 42 }
24+
expect(2)
25+
d.invokeOnCompletion {
26+
finish(3)
27+
reset()
28+
}
2529
}
30+
expectUnreached()
31+
} catch (e: CancellationException) {
32+
// Expected
2633
}
34+
}
2735

2836
@Test
29-
fun testLaunch() = CoroutineStart.values().forEach(::testLaunchCancelledParent)
37+
fun testLaunch() = runTest {
38+
CoroutineStart.values().forEach { testLaunchCancelledParent(it) }
39+
}
3040

31-
private fun testLaunchCancelledParent(start: CoroutineStart) =
32-
runTest({ it is CancellationException }) {
33-
cancel()
34-
expect(1)
35-
val d = launch(start = start) { }
36-
expect(2)
37-
d.invokeOnCompletion {
38-
finish(3)
39-
reset()
41+
private suspend fun testLaunchCancelledParent(start: CoroutineStart) {
42+
try {
43+
withContext(Job()) {
44+
cancel()
45+
expect(1)
46+
val d = launch(start = start) { }
47+
expect(2)
48+
d.invokeOnCompletion {
49+
finish(3)
50+
reset()
51+
}
4052
}
53+
expectUnreached()
54+
} catch (e: CancellationException) {
55+
// Expected
4156
}
57+
}
4258

4359
@Test
44-
fun testProduce() =
45-
runTest({ it is CancellationException }) {
46-
cancel()
47-
expect(1)
48-
val d = produce<Int> { }
49-
expect(2)
50-
(d as Job).invokeOnCompletion {
51-
finish(3)
52-
reset()
53-
}
60+
fun testProduce() = runTest({ it is CancellationException }) {
61+
cancel()
62+
expect(1)
63+
val d = produce<Int> { }
64+
expect(2)
65+
(d as Job).invokeOnCompletion {
66+
finish(3)
67+
reset()
5468
}
69+
}
5570

5671
@Test
57-
fun testBroadcast() = CoroutineStart.values().forEach(::testBroadcastCancelledParent)
72+
fun testBroadcast() = runTest {
73+
CoroutineStart.values().forEach { testBroadcastCancelledParent(it) }
74+
}
5875

59-
private fun testBroadcastCancelledParent(start: CoroutineStart) =
60-
runTest({ it is CancellationException }) {
61-
cancel()
62-
expect(1)
63-
val bc = broadcast<Int>(start = start) {}
64-
expect(2)
65-
(bc as Job).invokeOnCompletion {
66-
finish(3)
67-
reset()
76+
private suspend fun testBroadcastCancelledParent(start: CoroutineStart) {
77+
try {
78+
withContext(Job()) {
79+
cancel()
80+
expect(1)
81+
val bc = broadcast<Int>(start = start) {}
82+
expect(2)
83+
(bc as Job).invokeOnCompletion {
84+
finish(3)
85+
reset()
86+
}
6887
}
88+
expectUnreached()
89+
} catch (e: CancellationException) {
90+
// Expected
6991
}
92+
}
7093

7194
@Test
72-
fun testScopes() {
73-
testScope { coroutineScope { } }
74-
testScope { supervisorScope { } }
75-
testScope { flowScope { } }
76-
testScope { withTimeout(Long.MAX_VALUE) { } }
77-
testScope { withContext(Job()) { } }
78-
testScope { withContext(CoroutineName("")) { } }
95+
fun testScopes() = runTest {
96+
testScope { coroutineScope { } }
97+
testScope { supervisorScope { } }
98+
testScope { flowScope { } }
99+
testScope { withTimeout(Long.MAX_VALUE) { } }
100+
testScope { withContext(Job()) { } }
101+
testScope { withContext(CoroutineName("")) { } }
79102
}
80103

81-
private inline fun testScope(crossinline block: suspend () -> Unit) = runTest({ it is CancellationException }) {
82-
cancel()
83-
block()
104+
private suspend inline fun testScope(crossinline block: suspend () -> Unit) {
105+
try {
106+
withContext(Job()) {
107+
cancel()
108+
block()
109+
}
110+
expectUnreached()
111+
} catch (e: CancellationException) {
112+
// Expected
113+
}
84114
}
85115
}

‎kotlinx-coroutines-core/common/test/TestBase.common.kt

+8
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,14 @@ public expect val isStressTest: Boolean
1414
public expect val stressTestMultiplier: Int
1515

1616
public expect open class TestBase constructor() {
17+
/*
18+
* In common tests we emulate parameterized tests
19+
* by iterating over parameters space in the single @Test method.
20+
* This kind of tests is too slow for JS and does not fit into
21+
* the default Mocha timeout, so we're using this flag to bail-out
22+
* and run such tests only on JVM and K/N.
23+
*/
24+
public val isBoundByJsTestTimeout: Boolean
1725
public fun error(message: Any, cause: Throwable? = null): Nothing
1826
public fun expect(index: Int)
1927
public fun expectUnreached()

‎kotlinx-coroutines-core/common/test/channels/ChannelUndeliveredElementTest.kt

+23-19
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,19 @@ import kotlin.test.*
1010

1111
class ChannelUndeliveredElementTest : TestBase() {
1212
@Test
13-
fun testSendSuccessfully() = runAllKindsTest { kind ->
14-
val channel = kind.create<Resource> { it.cancel() }
15-
val res = Resource("OK")
16-
launch {
17-
channel.send(res)
13+
fun testSendSuccessfully() = runTest {
14+
runAllKindsTest { kind ->
15+
val channel = kind.create<Resource> { it.cancel() }
16+
val res = Resource("OK")
17+
launch {
18+
channel.send(res)
19+
}
20+
val ok = channel.receive()
21+
assertEquals("OK", ok.value)
22+
assertFalse(res.isCancelled) // was not cancelled
23+
channel.close()
24+
assertFalse(res.isCancelled) // still was not cancelled
1825
}
19-
val ok = channel.receive()
20-
assertEquals("OK", ok.value)
21-
assertFalse(res.isCancelled) // was not cancelled
22-
channel.close()
23-
assertFalse(res.isCancelled) // still was not cancelled
2426
}
2527

2628
@Test
@@ -86,21 +88,23 @@ class ChannelUndeliveredElementTest : TestBase() {
8688
}
8789

8890
@Test
89-
fun testSendToClosedChannel() = runAllKindsTest { kind ->
90-
val channel = kind.create<Resource> { it.cancel() }
91-
channel.close() // immediately close channel
92-
val res = Resource("OK")
93-
assertFailsWith<ClosedSendChannelException> {
94-
channel.send(res) // send fails to closed channel, resource was not delivered
91+
fun testSendToClosedChannel() = runTest {
92+
runAllKindsTest { kind ->
93+
val channel = kind.create<Resource> { it.cancel() }
94+
channel.close() // immediately close channel
95+
val res = Resource("OK")
96+
assertFailsWith<ClosedSendChannelException> {
97+
channel.send(res) // send fails to closed channel, resource was not delivered
98+
}
99+
assertTrue(res.isCancelled)
95100
}
96-
assertTrue(res.isCancelled)
97101
}
98102

99-
private fun runAllKindsTest(test: suspend CoroutineScope.(TestChannelKind) -> Unit) {
103+
private suspend fun runAllKindsTest(test: suspend CoroutineScope.(TestChannelKind) -> Unit) {
100104
for (kind in TestChannelKind.values()) {
101105
if (kind.viaBroadcast) continue // does not support onUndeliveredElement
102106
try {
103-
runTest {
107+
withContext(Job()) {
104108
test(kind)
105109
}
106110
} catch(e: Throwable) {

‎kotlinx-coroutines-core/common/test/flow/sharing/SharedFlowTest.kt

+5-3
Original file line numberDiff line numberDiff line change
@@ -439,7 +439,8 @@ class SharedFlowTest : TestBase() {
439439
}
440440

441441
@Test
442-
fun testDifferentBufferedFlowCapacities() {
442+
fun testDifferentBufferedFlowCapacities() = runTest {
443+
if (isBoundByJsTestTimeout) return@runTest // Too slow for JS, bounded by 2 sec. default JS timeout
443444
for (replay in 0..10) {
444445
for (extraBufferCapacity in 0..5) {
445446
if (replay == 0 && extraBufferCapacity == 0) continue // test only buffered shared flows
@@ -456,7 +457,7 @@ class SharedFlowTest : TestBase() {
456457
}
457458
}
458459

459-
private fun testBufferedFlow(sh: MutableSharedFlow<Int>, replay: Int) = runTest {
460+
private suspend fun testBufferedFlow(sh: MutableSharedFlow<Int>, replay: Int) = withContext(Job()) {
460461
reset()
461462
expect(1)
462463
val n = 100 // initially emitted to fill buffer
@@ -678,6 +679,7 @@ class SharedFlowTest : TestBase() {
678679

679680
@Test
680681
fun testStateFlowModel() = runTest {
682+
if (isBoundByJsTestTimeout) return@runTest // Too slow for JS, bounded by 2 sec. default JS timeout
681683
val stateFlow = MutableStateFlow<Data?>(null)
682684
val expect = modelLog(stateFlow)
683685
val sharedFlow = MutableSharedFlow<Data?>(
@@ -795,4 +797,4 @@ class SharedFlowTest : TestBase() {
795797
job.join()
796798
finish(5)
797799
}
798-
}
800+
}

‎kotlinx-coroutines-core/js/test/TestBase.kt

+27-2
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ public actual val isStressTest: Boolean = false
1010
public actual val stressTestMultiplier: Int = 1
1111

1212
public actual open class TestBase actual constructor() {
13+
public actual val isBoundByJsTestTimeout = true
1314
private var actionIndex = 0
1415
private var finished = false
1516
private var error: Throwable? = null
17+
private var lastTestPromise: Promise<*>? = null
1618

1719
/**
1820
* Throws [IllegalStateException] like `error` in stdlib, but also ensures that the test will not
@@ -70,7 +72,6 @@ public actual open class TestBase actual constructor() {
7072
finished = false
7173
}
7274

73-
// todo: The dynamic (promise) result is a work-around for missing suspend tests, see KT-22228
7475
@Suppress("ACTUAL_FUNCTION_WITH_DEFAULT_ARGUMENTS")
7576
public actual fun runTest(
7677
expected: ((Throwable) -> Boolean)? = null,
@@ -79,7 +80,29 @@ public actual open class TestBase actual constructor() {
7980
): dynamic {
8081
var exCount = 0
8182
var ex: Throwable? = null
82-
return GlobalScope.promise(block = block, context = CoroutineExceptionHandler { context, e ->
83+
/*
84+
* This is an additional sanity check against `runTest` mis-usage on JS.
85+
* The only way to write an async test on JS is to return Promise from the test function.
86+
* _Just_ launching promise and returning `Unit` won't suffice as the underlying test framework
87+
* won't be able to detect an asynchronous failure in a timely manner.
88+
* We cannot detect such situations, but we can detect the most common erroneous pattern
89+
* in our code base, an attempt to use multiple `runTest` in the same `@Test` method,
90+
* which typically is a premise to the same error:
91+
* ```
92+
* @Test
93+
* fun incorrectTestForJs() { // <- promise is not returned
94+
* for (parameter in parameters) {
95+
* runTest {
96+
* runTestForParameter(parameter)
97+
* }
98+
* }
99+
* }
100+
* ```
101+
*/
102+
if (lastTestPromise != null) {
103+
error("Attempt to run multiple asynchronous test within one @Test method")
104+
}
105+
val result = GlobalScope.promise(block = block, context = CoroutineExceptionHandler { context, e ->
83106
if (e is CancellationException) return@CoroutineExceptionHandler // are ignored
84107
exCount++
85108
when {
@@ -102,6 +125,8 @@ public actual open class TestBase actual constructor() {
102125
error?.let { throw it }
103126
check(actionIndex == 0 || finished) { "Expecting that 'finish(...)' was invoked, but it was not" }
104127
}
128+
lastTestPromise = result
129+
return result
105130
}
106131
}
107132

‎kotlinx-coroutines-core/jvm/test/TestBase.kt

+1
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ public val stressTestMultiplierCbrt = cbrt(stressTestMultiplier.toDouble()).roun
5050
* ```
5151
*/
5252
public actual open class TestBase actual constructor() {
53+
public actual val isBoundByJsTestTimeout = false
5354
private var actionIndex = AtomicInteger()
5455
private var finished = AtomicBoolean()
5556
private var error = AtomicReference<Throwable>()

‎kotlinx-coroutines-core/native/test/TestBase.kt

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ public actual val isStressTest: Boolean = false
88
public actual val stressTestMultiplier: Int = 1
99

1010
public actual open class TestBase actual constructor() {
11+
public actual val isBoundByJsTestTimeout = false
1112
private var actionIndex = 0
1213
private var finished = false
1314
private var error: Throwable? = null

0 commit comments

Comments
 (0)