Skip to content

Commit

Permalink
Improve runTest timeout messages (#4184)
Browse files Browse the repository at this point in the history
* Clearly separate the cases when the test body finished and
  was just waiting for the children from the cases where
  the test body itself kept running;
* When the test only awaits the child coroutines, mention
  `TestScope.backgroundScope`

Fixes #4182
  • Loading branch information
dkhalanskyjb authored Jul 19, 2024
1 parent dbe1dda commit 219372f
Showing 1 changed file with 24 additions and 13 deletions.
37 changes: 24 additions & 13 deletions kotlinx-coroutines-test/common/src/TestBuilders.kt
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@

package kotlinx.coroutines.test

import kotlinx.atomicfu.atomic
import kotlinx.coroutines.*
import kotlinx.coroutines.flow.*
import kotlinx.coroutines.selects.*
import kotlin.coroutines.*
import kotlin.jvm.*
Expand Down Expand Up @@ -308,12 +308,17 @@ public fun TestScope.runTest(
): TestResult = asSpecificImplementation().let { scope ->
scope.enter()
createTestResult {
val testBodyFinished = AtomicBoolean(false)

This comment has been minimized.

Copy link
@lppedd

lppedd Jul 23, 2024

Just curious, why is atomicfu's atomic wrapped into another object that simply delegates to it?

/** TODO: moving this [AbstractCoroutine.start] call outside [createTestResult] fails on JS. */
scope.start(CoroutineStart.UNDISPATCHED, scope) {
/* we're using `UNDISPATCHED` to avoid the event loop, but we do want to set up the timeout machinery
before any code executes, so we have to park here. */
yield()
testBody()
try {
testBody()
} finally {
testBodyFinished.value = true
}
}
var timeoutError: Throwable? = null
var cancellationException: CancellationException? = null
Expand All @@ -336,17 +341,15 @@ public fun TestScope.runTest(
if (exception is TimeoutCancellationException) {
dumpCoroutines()
val activeChildren = scope.children.filter(Job::isActive).toList()
val completionCause = if (scope.isCancelled) scope.tryGetCompletionCause() else null
var message = "After waiting for $timeout"
if (completionCause == null)
message += ", the test coroutine is not completing"
if (activeChildren.isNotEmpty())
message += ", there were active child jobs: $activeChildren"
if (completionCause != null && activeChildren.isEmpty()) {
message += if (scope.isCompleted)
", the test coroutine completed"
else
", the test coroutine was not completed"
val message = "After waiting for $timeout, " + when {
testBodyFinished.value && activeChildren.isNotEmpty() ->
"there were active child jobs: $activeChildren. " +
"Use `TestScope.backgroundScope` " +
"to launch the coroutines that need to be cancelled when the test body finishes"
testBodyFinished.value ->
"the test completed, but only after the timeout"
else ->
"the test body did not run to completion"
}
timeoutError = UncompletedCoroutinesError(message)
cancellationException = CancellationException("The test timed out")
Expand Down Expand Up @@ -603,3 +606,11 @@ public fun TestScope.runTestLegacy(
marker: Int,
unused2: Any?,
): TestResult = runTest(dispatchTimeoutMs = if (marker and 1 != 0) dispatchTimeoutMs else 60_000L, testBody)

// Remove after https://youtrack.jetbrains.com/issue/KT-62423/
private class AtomicBoolean(initial: Boolean) {
private val container = atomic(initial)
var value: Boolean
get() = container.value
set(value: Boolean) { container.value = value }
}

0 comments on commit 219372f

Please sign in to comment.