Skip to content

Improve runTest timeout messages #4184

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.

Sign up for GitHub

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 3 commits into from
Jul 19, 2024
Merged

Improve runTest timeout messages #4184

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bc6bd8b
Improve runTest timeout messages
dkhalanskyjb Jul 18, 2024
b825221
fixup
dkhalanskyjb Jul 18, 2024
0215e02
Mention the plans to have common-code atomics
dkhalanskyjb Jul 19, 2024
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
36 changes: 23 additions & 13 deletions kotlinx-coroutines-test/common/src/TestBuilders.kt
View file
Open in desktop
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)
/** 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,10 @@ public fun TestScope.runTestLegacy(
marker: Int,
unused2: Any?,
): TestResult = runTest(dispatchTimeoutMs = if (marker and 1 != 0) dispatchTimeoutMs else 60_000L, testBody)

private class AtomicBoolean(initial: Boolean) {
private val container = atomic(initial)
var value: Boolean
get() = container.value
set(value: Boolean) { container.value = value }
}