Virtual time can't actually be controlled #268
Comments
That is not how
Once again we are not in control of this. If |
|
I understand that the root of the issue is that If I make a change to which does not reproduce for me. I wonder if it's defending against behavior from an older implementation of Could there maybe be an option to create a Turbine that waits on virtual time instead of wall time? |
It's there because for most tests, the expectation is that in failing cases, My expectation is that if you make that change to If that's not the case, then our decision here is worth reconsidering! It would certainly simplify the code a lot. |
|
@jingibus making that change causes 4 tests to fail, 3 of which because the flow under test has delays that use real time with Dispatchers.Default. I only tested on jvm though. |
|
Made a PR and fixed those tests. Looks like all tests passed. https://github.com/mightyguava/turbine/actions/runs/6291465123/job/17079856671. Failed on lint Also added a test for delay testing |
|
Delay testing remains the same as any other suspend function. You either need to take control of virtual time and assert against the effects of delayed code, measure virtual time before and after a function call, or you can wrap your calls in Our timeout is always a wall clock timeout because it guards against a mismatched await without an associated item that would otherwise hang the test and produce an unhelpful stacktrace when |
If the test writer forgets to take control of virtual time in their test, Turbine will advance it for them, rendering their test for delays useless. That might be "correct" behavior and is due to runTest, but it is surprising behavior from my point of view. I'm interested in the use of
I think this test that @jingibus suggested exercises the behavior you are describing? The test passes with virtual time on my PR. @Test fun awaitFailsOnVirtualTime() = runTestTurbine {
assertFailsWith<AssertionError> {
flow<Nothing> {
awaitCancellation()
}.test {
awaitComplete()
}
}
} |
This behavior comes from |
|
What I do in these cases is use To me, it seems reasonable that |
|
I agree it's reasonable for As |
Turbine's timeout has nothing to do with |
I didn't know that the use of wall time here predates
^ this hang does not happen in virtual time. |
|
This test verifies the timeout behavior: turbine/src/commonTest/kotlin/app/cash/turbine/FlowTest.kt Lines 617 to 629 in fce249a |
|
Here's the test modified to measure how long it took to run @Test fun failsOnDefaultTimeout() = runTest {
val took = measureTime {
neverFlow().test {
val actual = assertFailsWith<AssertionError> {
awaitItem()
}
assertEquals("No value produced in 3s", actual.message)
assertCallSitePresentInStackTraceOnJvm(
throwable = actual,
entryPoint = "ChannelTurbine\$awaitItem",
callSite = "FlowTest\$failsOnDefaultTimeout",
)
}
}
println("This test actually took $took")
}With the timeout using virtual time instead of wall time, it passes, and prints: |
|
@JakeWharton @jingibus I'm wondering if there's enough evidence shown here that it would be safe to add an option to Turbine to do timeouts in virtual time instead of wall time? It would benefit people who use This statement has been mentioned several times
I tried my best to reproduce the issue described, and tried the test above, but as far as I can tell it does not happen using virtual time with the current versions of Turbine and Kotlin. |
|
I do not believe we need that option, no. The existing timeout not running on wall clock time is just a bug that needs fixed. Someone will get to it eventually. If you want to test delays you should either take control of virtual time with manual advancement or monitor the virtual clock to see how much time has passed between events. You can also use |
What bug is this? If you are referring to #268 (comment), that is with on my branch here #269, where I moved Turbine off wall clock for timeouts. This behavior is not a bug imo as I've said above. There doesn't appear to be any benefit for Turbine to timeout in wall time when the test is running in virtual time. There are no hangs moving to virtual time, it's easier to test delays, and tests run faster. Anyways, point taken, I'll look into alternatives for testing delays. |
|
Hi 👋 not sure I follow all the low level technical things here but it seems, at the end of the day, it's not currently possible to use this library when delays are involved and want to assert the delay (rather than skip). But please feel free to correct me if I'm misunderstanding. For example: Can't write a test like this in Turbine: @Test
fun minDelayBetweenUpdatesTest() = runTest {
val emittedValues = mutableListOf<Int>()
val flow = MutableSharedFlow<Int>(replay = 3)
flow
.minDelayBetweenUpdates(
delay = 1.seconds,
shouldDelay = { newItem, _ -> newItem != 2 },
now = { Instant.fromEpochMilliseconds(testScheduler.currentTime) },
)
.onEach { emittedValues.add(it) }
// auto close flow when test completes so test does not get stuck
.launchIn(backgroundScope)
// Emit 3 values very fast
flow.emit(1)
flow.emit(2)
flow.emit(3)
// Assert: Nothing emitted yet cause time has not advanced
emittedValues isEqualTo emptyList()
// Act: Move just enough for the 1st item
advanceTimeBy(1.milliseconds)
// Assert: 1st and 2nd item emitted because shouldDelay returns false for item 2
emittedValues isEqualTo listOf(1, 2)
// Act: Move up just before the 3rd item is emitted
advanceTimeBy(0.9.seconds)
// Assert: The 3rd item has not been emitted
emittedValues isEqualTo listOf(1, 2)
// Act: Advance just enough for the 3rd item to be emitted
advanceTimeBy(0.1.seconds)
// Assert: The 3rd item has been emitted
emittedValues isEqualTo listOf(1, 2, 3)
// Act: Move time forward close to the delay parameter
advanceTimeBy(0.9.seconds)
// Act: Emit a 4th item
flow.emit(4)
// Assert: The 4th item has not been emitted because it's not past the min delay parameter
emittedValues isEqualTo listOf(1, 2, 3)
// Act: Advance just enough for the 4th item to be emitted (taking it the 1 second min delay)
advanceTimeBy(0.1.seconds)
// Assert: The 4th item has been emitted
emittedValues isEqualTo listOf(1, 2, 3, 4)
// Act: Advance just enough for the 5th item to be emitted without any delay because it's past the min delay mark
advanceTimeBy(1.seconds)
// Act: Emit a 5th item
flow.emit(5)
// Act: Allow the flow to emit
advanceTimeBy(1.milliseconds)
// Assert: The 5th item has been emitted
emittedValues isEqualTo listOf(1, 2, 3, 4, 5)
}
/**
* Ensures there is a minimum time between emitted values. Does not delay more than required by calculating how much
* time has already passed between updates and not delaying if [shouldDelay] returns false.
*
* For example, if [delay] is 1 second and the next value was emitted 0.2 seconds later than the update is
* only delayed 0.8 seconds. No delay would occur if an update was after 1 second.
*
* This is useful when a service provides updates faster than is reasonable for UI to display. Slows down updates to
* allow the UI update without "flickering".
*/
fun <T> Flow<T>.minDelayBetweenUpdates(
delay: Duration,
shouldDelay: (newItem: T, currentItem: T?) -> Boolean = { _, _ -> true },
now: () -> kotlinx.datetime.Instant = { Clock.System.now() },
) : Flow<T> {
var lastUpdate: kotlinx.datetime.Instant? = null
var previousItem: T? = null
return onEach {
// Only space time if needed
if (shouldDelay(it, previousItem)) {
// How long since our last update?
val durationSinceLastUpdate = lastUpdate?.let { lastUpdate -> now() - lastUpdate }
// Only space time if enough time has not passed since the last update
if (durationSinceLastUpdate != null && delay > durationSinceLastUpdate) {
// Calculate how much time is left to reach the desired time spacing
val timeLeftToMeetDelayRequirement = delay - durationSinceLastUpdate
delay(timeLeftToMeetDelayRequirement)
}
}
// Track when we updated to know how much time has passed for the next update
lastUpdate = now()
previousItem = it
}
} |
Kotlin's implementation of virtual time does not allow for (or, at least, encourage) the style of testing you present in your example. You can't take a stopwatch and advance it by 1s, 2s, 3s, etc and observe what happens at each time period. The design intent is to flip the relationship on its head: instead of driving the test with the stopwatch, drive the test with the events you wish to observe. When each event occurs, observe the value of the stopwatch to see whether it arrived at the time you expected. Turbine mostly has no opinion on this (if you want to work around this paradigm, go right ahead - Turbine will not make your life any easier or harder than the rest of coroutines+flow), except that (as @JakeWharton explains above) it takes pains to timeout using wall clock time. This is because Turbine's timeouts are intended to detect bugs, not programmatic delays: i.e. I expected to observe event X, and event X never occurred. It would be inappropriate if that test were to fail because of a call to |
|
Thanks for taking the time to reply @jingibus !
Not sure I understand,
Ah I can see that design working well.. is there docs or an example somewhere?
Reading over this and I'm sure I lack knowledge on this subject. From my standpoint, I'm just a developer trying to understand how to write a test for a flow where I need to actually assert delay behavior, not skip it. |
Yes, it is there, and yes it does advance the virtual time as in your example. But it will also automatically advance virtual time.
JetBrains mostly seems content to let us figure this out ourselves from the reference documentation, rather than provide a clear guide on best practices. (I don't love this.)
I would try something like this: flow.test {
val start = currentTime
assertThat(awaitItem()).isEqualTo(1)
assertThat(currentTime - start).isEqualTo(1000)
assertThat(awaitItem()).isEqualTo(2)
assertThat(currentTime - start).isEqualTo(1000)
assertThat(awaitItem()).isEqualTo(3)
assertThat(currentTime - start).isEqualTo(2000)
} |
|
You can use the @Test
fun test_delay_period() = runTest {
val flow = flow { delay(50); emit(true) }
flow.test {
val duration = testTimeSource.measureTime { awaitItem() }
assertEquals(50.milliseconds, duration)
}
} |
Thank you @kevincianfarini for taking the time to provide this example test code. However, it does not seem to work as expected when more than 1 delay is used, which is the main use case. It appears all the delays are immediately applied so @Test
fun test_delay_period() = runTest {
val flow = flow {
delay(50)
emit(true)
delay(100)
emit(false)
}
flow.test {
val duration1 = testTimeSource.measureTime { awaitItem() }
assertEquals(50.milliseconds, duration1)
val duration2 = testTimeSource.measureTime { awaitItem() }
assertEquals(100.milliseconds, duration2)
awaitComplete()
}
} |
Thank you too @jingibus for the sample code! Just wasn't sure how to create the This code does not work for the same issue as my reply to @kevincianfarini above: (All delays are applied right away) @Test
fun test_delay_period() = runTest {
flow {
// Emit 3 values very fast
emit(1)
emit(2)
emit(3)
// Emit after 0.9 seconds, so additional delay should only be 0.1 seconds from minDelayBetweenUpdates (if we applied it here, not applying to get basic test working)
delay(0.9.seconds)
emit(4)
// Emit after 1 second, no additional delay should occur from minDelayBetweenUpdates (if we applied it here, not applying to get basic test working)
delay(1.1.seconds)
emit(5)
}.test {
val startTime = currentTime
assertThat(awaitItem()).isEqualTo(1)
assertThat(currentTime - startTime).isEqualTo(0)
assertThat(awaitItem()).isEqualTo(2)
assertThat(currentTime - startTime).isEqualTo(0)
assertThat(awaitItem()).isEqualTo(3)
assertThat(currentTime - startTime).isEqualTo(0)
assertThat(awaitItem()).isEqualTo(4)
assertThat(currentTime - startTime).isEqualTo(900)
assertThat(awaitItem()).isEqualTo(5)
assertThat(currentTime - startTime).isEqualTo(2000)
awaitComplete()
}
}Fails on the first If I use So to sum up, I still don't understand how to use the turbine design pattern to control virtual time for tests. The code in my comment which does not use Turbine does work and allows control of virtual time |
|
@ryanholden8 I'm not quite sure why my example isn't working for you. The following test passes locally for me. @OptIn(ExperimentalTime::class)
@Test fun foo() = runTest {
val flow = flow {
delay(50)
emit(1)
delay(100)
emit(2)
}
flow.test {
val duration1 = testTimeSource.measureTime { awaitItem() }
assertEquals(50.milliseconds, duration1)
val duration2 = testTimeSource.measureTime { awaitItem() }
assertEquals(100.milliseconds, duration2)
awaitComplete()
}
}In the debugger while running the test, you can see the values of
Which versions of Turbine and Coroutines are you using? |
ah good instinct. Was using |
|
Awesome 👍 |
|
Here's the final test using Turbine to assert virtual time, in case anybody finds this thread. Thanks @kevincianfarini & @jingibus for the practical help! The test is much cleaner than the original test using advanceTimeBy in #268 (comment) @Test
fun minDelayBetweenUpdates() = runTest {
flow {
// Emit 3 values very fast
emit(1)
emit(2)
emit(3)
// Emit after 0.9 seconds, so minDelayBetweenUpdates should only add an additional delay of 0.1 seconds
delay(0.9.seconds)
emit(4)
// Emit after 5 seconds, no additional delay should occur from minDelayBetweenUpdates
delay(5.seconds)
emit(5)
}
.minDelayBetweenUpdates(
delay = 1.seconds,
// Don't delay item 2
shouldDelay = { newItem, _ -> newItem != 2 },
now = { Instant.fromEpochMilliseconds(currentTime) },
)
.test {
with (testTimeSource) {
// Item 1 is not delayed
measureTimedValue { awaitItem() } isEqualTo TimedValue(1, 0.seconds)
// Item 2 is not delayed because shouldDelay returns false
measureTimedValue { awaitItem() } isEqualTo TimedValue(2, 0.seconds)
// Item 3 was emitted at the same time as 1 and 2 but should have a delay of 1 second
measureTimedValue { awaitItem() } isEqualTo TimedValue(3, 1.seconds)
// Item 4 was emitted at 0.9 seconds after item 3 so it should only be delayed 0.1 seconds
// taking it up to the min delay of 1 second
measureTimedValue { awaitItem() } isEqualTo TimedValue(4, 1.seconds)
// Item 5 was emitted at 5 seconds after item 4, so there should be no additional delay
measureTimedValue { awaitItem() } isEqualTo TimedValue(5, 5.seconds)
}
awaitComplete()
}
}
/**
* Ensures there is a minimum time between emitted values. Does not delay more than required by calculating how much
* time has already passed between updates and not delaying if [shouldDelay] returns false.
*
* For example, if [delay] is 1 second and the next value was emitted 0.2 seconds later than the update is
* only delayed 0.8 seconds. No delay would occur if an update was after 1 second.
*
* This is useful when a service provides updates faster than is reasonable for UI to display. Slows down updates to
* allow the UI to update without "flickering".
*/
fun <T> Flow<T>.minDelayBetweenUpdates(
delay: Duration,
shouldDelay: (newItem: T, currentItem: T?) -> Boolean = { _, _ -> true },
now: () -> kotlinx.datetime.Instant = { Clock.System.now() },
delayDispatcher: CoroutineDispatcher? = null,
): Flow<T> {
var lastUpdate: kotlinx.datetime.Instant? = null
var previousItem: T? = null
return onEach {
// Only space time if needed
if (shouldDelay(it, previousItem)) {
// How long since our last update?
val durationSinceLastUpdate = lastUpdate?.let { lastUpdate -> now() - lastUpdate }
// Only space time if enough time has not passed since the last update
if (durationSinceLastUpdate != null && delay > durationSinceLastUpdate) {
// Calculate how much time is left to reach the desired time spacing
val timeLeftToMeetDelayRequirement = delay - durationSinceLastUpdate
if (delayDispatcher != null) {
withContext(delayDispatcher) {
delay(timeLeftToMeetDelayRequirement)
}
} else {
delay(timeLeftToMeetDelayRequirement)
}
}
}
// Track when we updated to know how much time has passed for the next update
lastUpdate = now()
previousItem = it
}
}
/**
* Asserts that this object is equal to [expected].
* Syntax sugar to help unit test assertions read more like factual statements.
*/
infix fun <T> T?.isEqualTo(expected: T?) = Truth.assertThat(this).isEqualTo(expected)
|
|
That's a great example. @JakeWharton, I don't think we can fix this as long as we are heavily buffering the internal channel; we'd need to switch it to |
While trying to test delays with turbine I noticed that trying to control virtual time didn't actually do anything. Like the test here
turbine/src/commonTest/kotlin/app/cash/turbine/FlowTest.kt
Lines 711 to 735 in fce249a
If you just remove all of the
advanceTimeBy()andrunCurrent()calls, it still passesI'd expect the lack of
advanceTimeBy()calls to fail the firstassertEquals(). I think what's happening is that sinceawaitItem()will yield, the TestDispatcher will advance time automatically. This makes virtual time meaningless when testing with Turbine unless all you want to do is verify ordering.There's also another annoying problem that an
awaitItem()call will cause other coroutines in the same scope that are running awhile (true) { delay(...); doSomething(...) }to constantly spin.The text was updated successfully, but these errors were encountered: