Skip to content

Explain the test framework behavior in the withTimeout message #3623

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.

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 2 commits into from
Feb 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions kotlinx-coroutines-core/common/src/Delay.kt
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,19 @@ public interface Delay {
DefaultDelay.invokeOnTimeout(timeMillis, block, context)
}

/**
* Enhanced [Delay] interface that provides additional diagnostics for [withTimeout].
* Is going to be removed once there is proper JVM-default support.
* Then we'll be able put this function into [Delay] without breaking binary compatibility.
*/
@InternalCoroutinesApi
internal interface DelayWithTimeoutDiagnostics : Delay {
/**
* Returns a string that explains that the timeout has occurred, and explains what can be done about it.
*/
fun timeoutMessage(timeout: Duration): String
}

/**
* Suspends until cancellation, in which case it will throw a [CancellationException].
*
Expand Down
17 changes: 11 additions & 6 deletions kotlinx-coroutines-core/common/src/Timeout.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import kotlin.coroutines.*
import kotlin.coroutines.intrinsics.*
import kotlin.jvm.*
import kotlin.time.*
import kotlin.time.Duration.Companion.milliseconds

/**
* Runs a given suspending [block] of code inside a coroutine with a specified [timeout][timeMillis] and throws
Expand Down Expand Up @@ -135,9 +136,9 @@ public suspend fun <T> withTimeoutOrNull(timeMillis: Long, block: suspend Corout
* > Implementation note: how the time is tracked exactly is an implementation detail of the context's [CoroutineDispatcher].
*/
public suspend fun <T> withTimeoutOrNull(timeout: Duration, block: suspend CoroutineScope.() -> T): T? =
withTimeoutOrNull(timeout.toDelayMillis(), block)
withTimeoutOrNull(timeout.toDelayMillis(), block)

private fun <U, T: U> setupTimeout(
private fun <U, T : U> setupTimeout(
coroutine: TimeoutCoroutine<U, T>,
block: suspend CoroutineScope.() -> T
): Any? {
Expand All @@ -150,12 +151,12 @@ private fun <U, T: U> setupTimeout(
return coroutine.startUndispatchedOrReturnIgnoreTimeout(coroutine, block)
}

private class TimeoutCoroutine<U, in T: U>(
private class TimeoutCoroutine<U, in T : U>(
@JvmField val time: Long,
uCont: Continuation<U> // unintercepted continuation
) : ScopeCoroutine<T>(uCont.context, uCont), Runnable {
override fun run() {
cancelCoroutine(TimeoutCancellationException(time, this))
cancelCoroutine(TimeoutCancellationException(time, context.delay, this))
}

override fun nameString(): String =
Expand All @@ -173,7 +174,6 @@ public class TimeoutCancellationException internal constructor(
* Creates a timeout exception with the given message.
* This constructor is needed for exception stack-traces recovery.
*/
@Suppress("UNUSED")
internal constructor(message: String) : this(message, null)

// message is never null in fact
Expand All @@ -183,5 +183,10 @@ public class TimeoutCancellationException internal constructor(

internal fun TimeoutCancellationException(
time: Long,
delay: Delay,
coroutine: Job
) : TimeoutCancellationException = TimeoutCancellationException("Timed out waiting for $time ms", coroutine)
) : TimeoutCancellationException {
val message = (delay as? DelayWithTimeoutDiagnostics)?.timeoutMessage(time.milliseconds)
?: "Timed out waiting for $time ms"
return TimeoutCancellationException(message, coroutine)
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,8 @@ class TimeoutTest : TestBase() {
fun testUpstreamError() = testUpstreamError(TestException())

@Test
fun testUpstreamErrorTimeoutException() = testUpstreamError(TimeoutCancellationException(0, Job()))
fun testUpstreamErrorTimeoutException() =
testUpstreamError(TimeoutCancellationException("Timed out waiting for ${0} ms", Job()))

@Test
fun testUpstreamErrorCancellationException() = testUpstreamError(CancellationException(""))
Expand Down
3 changes: 2 additions & 1 deletion kotlinx-coroutines-test/api/kotlinx-coroutines-test.api
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,12 @@ public final class kotlinx/coroutines/test/TestCoroutineScopeKt {
public static final fun runCurrent (Lkotlinx/coroutines/test/TestCoroutineScope;)V
}

public abstract class kotlinx/coroutines/test/TestDispatcher : kotlinx/coroutines/CoroutineDispatcher, kotlinx/coroutines/Delay {
public abstract class kotlinx/coroutines/test/TestDispatcher : kotlinx/coroutines/CoroutineDispatcher, kotlinx/coroutines/Delay, kotlinx/coroutines/DelayWithTimeoutDiagnostics {
public fun delay (JLkotlin/coroutines/Continuation;)Ljava/lang/Object;
public abstract fun getScheduler ()Lkotlinx/coroutines/test/TestCoroutineScheduler;
public fun invokeOnTimeout (JLjava/lang/Runnable;Lkotlin/coroutines/CoroutineContext;)Lkotlinx/coroutines/DisposableHandle;
public fun scheduleResumeAfterDelay (JLkotlinx/coroutines/CancellableContinuation;)V
public synthetic fun timeoutMessage-LRDsOJo (J)Ljava/lang/String;
}

public final class kotlinx/coroutines/test/TestDispatchers {
Expand Down
11 changes: 10 additions & 1 deletion kotlinx-coroutines-test/common/src/TestDispatcher.kt
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package kotlinx.coroutines.test
import kotlinx.coroutines.*
import kotlin.coroutines.*
import kotlin.jvm.*
import kotlin.time.*

/**
* A test dispatcher that can interface with a [TestCoroutineScheduler].
Expand All @@ -17,7 +18,8 @@ import kotlin.jvm.*
* the virtual time.
*/
@ExperimentalCoroutinesApi
public abstract class TestDispatcher internal constructor() : CoroutineDispatcher(), Delay {
@Suppress("INVISIBLE_REFERENCE")
public abstract class TestDispatcher internal constructor() : CoroutineDispatcher(), Delay, DelayWithTimeoutDiagnostics {
/** The scheduler that this dispatcher is linked to. */
@ExperimentalCoroutinesApi
public abstract val scheduler: TestCoroutineScheduler
Expand All @@ -44,6 +46,13 @@ public abstract class TestDispatcher internal constructor() : CoroutineDispatche
/** @suppress */
override fun invokeOnTimeout(timeMillis: Long, block: Runnable, context: CoroutineContext): DisposableHandle =
scheduler.registerEvent(this, timeMillis, block, context) { false }

/** @suppress */
@Suppress("CANNOT_OVERRIDE_INVISIBLE_MEMBER")
@Deprecated("Is only needed internally", level = DeprecationLevel.HIDDEN)
public override fun timeoutMessage(timeout: Duration): String =
"Timed out after $timeout of _virtual_ (kotlinx.coroutines.test) time. " +
"To use the real time, wrap 'withTimeout' in 'withContext(Dispatchers.Default.limitedParallelism(1))'"
}

/**
Expand Down
14 changes: 14 additions & 0 deletions kotlinx-coroutines-test/common/test/TestScopeTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,20 @@ class TestScopeTest {
}
}

/**
* Tests that [TestScope.withTimeout] notifies the programmer about using the virtual time.
*/
@Test
fun testTimingOutWithVirtualTimeMessage() = runTest {
try {
withTimeout(1_000_000) {
Channel<Unit>().receive()
}
} catch (e: TimeoutCancellationException) {
assertContains(e.message!!, "virtual")
}
}

companion object {
internal val invalidContexts = listOf(
Dispatchers.Default, // not a [TestDispatcher]
Expand Down