Skip to content
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

Add TimeSource implementation to TestCoroutineScheduler #3087

Closed
wants to merge 1 commit into from
Closed
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
2 changes: 2 additions & 0 deletions kotlinx-coroutines-test/api/kotlinx-coroutines-test.api
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public final class kotlinx/coroutines/test/TestCoroutineScheduler : kotlin/corou
public final fun advanceTimeBy (J)V
public final fun advanceUntilIdle ()V
public final fun getCurrentTime ()J
public final fun getTimeSource ()Lkotlin/time/TimeSource;
public final fun runCurrent ()V
}

Expand Down Expand Up @@ -113,6 +114,7 @@ public final class kotlinx/coroutines/test/TestScopeKt {
public static final fun advanceTimeBy (Lkotlinx/coroutines/test/TestScope;J)V
public static final fun advanceUntilIdle (Lkotlinx/coroutines/test/TestScope;)V
public static final fun getCurrentTime (Lkotlinx/coroutines/test/TestScope;)J
public static final fun getTestTimeSource (Lkotlinx/coroutines/test/TestScope;)Lkotlin/time/TimeSource;
public static final fun runCurrent (Lkotlinx/coroutines/test/TestScope;)V
}

Expand Down
13 changes: 11 additions & 2 deletions kotlinx-coroutines-test/common/src/TestCoroutineScheduler.kt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import kotlinx.coroutines.internal.*
import kotlinx.coroutines.selects.*
import kotlin.coroutines.*
import kotlin.jvm.*
import kotlin.time.*

/**
* This is a scheduler for coroutines used in tests, providing the delay-skipping behavior.
Expand All @@ -26,7 +27,6 @@ import kotlin.jvm.*
* haven't yet been dispatched (via [runCurrent]).
*/
@ExperimentalCoroutinesApi
// TODO: maybe make this a `TimeSource`?
public class TestCoroutineScheduler : AbstractCoroutineContextElement(TestCoroutineScheduler),
CoroutineContext.Element {

Expand All @@ -43,7 +43,7 @@ public class TestCoroutineScheduler : AbstractCoroutineContextElement(TestCorout
/** This counter establishes some order on the events that happen at the same virtual time. */
private val count = atomic(0L)

/** The current virtual time. */
/** The current virtual time in milliseconds. */
@ExperimentalCoroutinesApi
public var currentTime: Long = 0
get() = synchronized(lock) { field }
Expand Down Expand Up @@ -193,6 +193,15 @@ public class TestCoroutineScheduler : AbstractCoroutineContextElement(TestCorout
* Consumes the knowledge that a dispatch event happened recently.
*/
internal val onDispatchEvent: SelectClause1<Unit> get() = dispatchEvents.onReceive

/**
* Returns the [TimeSource] representation of the virtual time of this scheduler.
*/
@ExperimentalCoroutinesApi
@ExperimentalTime
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is worth marking it as ExperimentalCoroutinesApi as well in case we'll decide to implement TimeSource interface when it's stable?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!

Copy link
Contributor Author

@hfhbd hfhbd Dec 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, added

public val timeSource: TimeSource = object : AbstractLongTimeSource(DurationUnit.MILLISECONDS) {
override fun read(): Long = currentTime
}
}

// Some error-throwing functions for pretty stack traces
Expand Down
11 changes: 10 additions & 1 deletion kotlinx-coroutines-test/common/src/TestScope.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 kotlinx.coroutines.internal.*
import kotlin.coroutines.*
import kotlin.time.*

/**
* A coroutine scope that for launching test coroutines.
Expand Down Expand Up @@ -84,6 +85,14 @@ public fun TestScope.runCurrent(): Unit = testScheduler.runCurrent()
@ExperimentalCoroutinesApi
public fun TestScope.advanceTimeBy(delayTimeMillis: Long): Unit = testScheduler.advanceTimeBy(delayTimeMillis)

/**
* The [test scheduler][TestScope.testScheduler] as a [TimeSource].
* @see TestCoroutineScheduler.timeSource
*/
@ExperimentalCoroutinesApi
@ExperimentalTime
public val TestScope.testTimeSource: TimeSource get() = testScheduler.timeSource
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't it be timeSource or schedulerTimeSource?
Also lacks ExperimentalCoroutinesApi

Copy link
Collaborator

@dkhalanskyjb dkhalanskyjb Dec 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was initially timeSource, but I advised against it because seeing just a bare timeSource in the middle of a test wolud be disheartening, I assumed. schedulerTimeSource is not much better than testScheduler.timeSource, so I think it's either testTimeSource or no extension on TestScope at all.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works for me, thanks for the explanation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added annotation as well


/**
* Creates a [TestScope].
*
Expand Down Expand Up @@ -237,4 +246,4 @@ internal class UncaughtExceptionsBeforeTest : IllegalStateException(
* Thrown when a test has completed and there are tasks that are not completed or cancelled.
*/
@ExperimentalCoroutinesApi
internal class UncompletedCoroutinesError(message: String) : AssertionError(message)
internal class UncompletedCoroutinesError(message: String) : AssertionError(message)
12 changes: 12 additions & 0 deletions kotlinx-coroutines-test/common/test/TestCoroutineSchedulerTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package kotlinx.coroutines.test

import kotlinx.coroutines.*
import kotlin.test.*
import kotlin.time.*
import kotlin.time.Duration.Companion.seconds

class TestCoroutineSchedulerTest {
/** Tests that `TestCoroutineScheduler` attempts to detect if there are several instances of it. */
Expand Down Expand Up @@ -308,6 +310,16 @@ class TestCoroutineSchedulerTest {
}
}

@Test
@ExperimentalTime
fun testAdvanceTimeSource() = runTest {
val expected = 1.seconds
val actual = testTimeSource.measureTime {
delay(expected)
}
assertEquals(expected, actual)
}

private fun forTestDispatchers(block: (TestDispatcher) -> Unit): Unit =
@Suppress("DEPRECATION")
listOf(
Expand Down