TestScope.runTest should support the option to explicitly manage "currentTime" instead of auto advancing it
Opened this issue · 7 comments
Summary
runTest is hard coded to advance time and it would be nice to make that configurable.
Use case
I am writing tests that involve the a Postgres database and time. To avoid waiting the actual amount of time, I used a runTest {} based test. The problem is that my implementation of the database is with jasync suspending connection. During the test, a jasync call runs and a separate thread is running to handle the request. From the TestScope's point of view, there are no more coroutines to run. So, it advances time and hits a withTimout I have above the jasync command. I understand that a lot of the time the workRunner is really nice. I would like to have the option of it only running current tasks instead of incrementing the time automatically.
This is the section that is firing.
val workRunner = launch(CoroutineName("kotlinx.coroutines.test runner")) {
while (true) {
// This `tryRunNextTaskUnless` automatically advances `currentTime`
val executedSomething = testScheduler.tryRunNextTaskUnless { !isActive }
if (executedSomething) {
/** yield to check for cancellation. On JS, we can't use [ensureActive] here, as the cancellation
* procedure needs a chance to run concurrently. */
yield()
} else {
// waiting for the next task to be scheduled, or for the test runner to be cancelled
testScheduler.receiveDispatchEvent()
}
}
}
Example real life failing test case
@Test
fun `should notify when your replacement doesn't show up`() = testContext.withLogic {
schedule("adalovelace", "1991-08-26T07:57:08.123456Z")
// Timeout here because jasync processing is unknown to runTest
delay(12.hours())
notifications.sent("adalovelace", "Your replacement hasn't arrived yet.")
}
Example simple failing test case
@Test
fun `should allow manually updating the clock`() {
val suspendingConnection = PostgreSQLConnectionBuilder
.createConnectionPool("jdbc:postgresql://$host:$port/$databaseName?user=$username&password=$password")
.asSuspending
runTest {
withTimeout(1_000) {
suspendingConnection.sendQuery("SELECT 1;")
}
}
}
I can try to create a project to demonstrate the problem if this isn't clear enough. Just let me know.
The Shape of the API
I am not quite sure on how we can replace this. I mean it would be nice if there was just another dispatcher like StandardDispatcher that had this behavior. Something like PauseDispatcher, ManualDispatcher, or something like that. I don't think that would allow you to replace the code in question. I think it would require a larger change.
Hey David! Without diving into details about your case, I have a few vectors in mind, in no particular order.
- You don't have to use
runTestfor tests, you could userunBlocking(usually not advised, but could work for you). - Consider wrapping the misbehaving object into a coroutine-friendly wrapper.
- Could it be that something is required from the
jasyncside?
If none of them work for you, please come back and we'll think about your case more.
You don't have to use runTest for tests, you could use runBlocking (usually not advised, but could work for you).
I originally chose to use runTest because the use case involved waiting an entire shift of 12 hours. I could try to cheat it, but the real life use case is 12 hours.
Consider wrapping the misbehaving object into a coroutine-friendly wrapper.
jasync is already "coroutine-friendly", but not "TestScope" friendly. It is kind of weird. If jasync was a blocking library, I wouldn't hit this issue.
Could it be that something is required from the jasync side?
I wouldn't think so just because it is required anywhere there is a coroutine friendly API call dependent on a external thread. IDK if that helps much.
I was able to hack a solution together. What I ended up doing was using a coroutine that just continuously yields in order to not advance the clock. Here is a snippet from a "SuspendingConnectionTestScopeCompatible" wrapper I wrote.
override suspend fun sendQuery(query: String): QueryResult = forceWait { wrapped.sendQuery(query) }
private suspend fun <T> forceWait(action: suspend () -> T): T {
return coroutineScope {
val job = launch {
while (true) {
yield()
}
}
try {
action()
} finally {
job.cancel()
}
}
}
The problem though is way bigger than just one library. So since I use Ktor, I had to do something similar to Ktor so that we don't advance the test while sending a HTTP request.
Maybe my "solution" isn't the right way to fix this? I don't know. Maybe you could add a flag to the coroutine definition to say this is bridging from coroutines to a separate thread? That way the TestScope can do the best default in all cases?
Thanks for taking the time and trying to understand my issue. I know this isn't easy. :)
Your case makes sense for me, thank you for raising this. And your case generalistic enough so that we should provide a ready-made recipe or guidance in docs.
Full disclosure, I might not be aware of an existing solution / workaround for it.
As a last resort, possibly there's a way to supply a context to the external library to force it to use threads known to runTest?
Duplicate of #3179
The workaround for now is #3179 (comment)
@dkhalanskyjb That does appear to be the same problem I am hitting. I will try to double check, but I could swear
withContext(Dispatchers.Default.limitedParallelism(1)) {
// delays are not skipped here
}
didn't stop the virtual clock from advancing.
I think both tickets beat around the bush though. After sitting on this a while, I think Kotlin need to provide a way for the scheduler to know this job depends on external threads. Doing something like I or #3179 suggests is better than nothing, but still leaves all the problems on the user.
@dmstocking , what do you think of this API shape?
As for knowing whether a Job depends on external threads, do you propose that whenever that happens, the test framework shouldn't skip delays, but instead, should wait for the external work to finish? We have considered this approach when designing the test framework and discovered that it wasn't viable.
Consider this test:
class BlackBox {
private lateinit var myCont: CancellableContinuation<Int>
suspend fun await() = suspendCancellableCoroutine { cont ->
myCont = cont
hiddenImpl()
}
fun tick() {
// I'm withholding what happens here
}
private fun hiddenImpl() {
thread {
// I'm withholding what happens here
}
}
}
runTest {
val blackBox = BlackBox()
launch {
delay(100.milliseconds)
blackBox.tick()
}
blackBox.await()
}Should the test framework proceed to blackBox.tick(), or should it wait for await() to finish first? The test framework has no way of knowing what tick() and hiddenImpl() do, so it has to blindly make a choice right now.
If it chose to advance the virtual time, it can indeed lead to something happening too soon in terms of real-world time, before the code in hiddenImpl knows how to react to it—exactly the situation that you found yourself in. This can be worked around by ensuring the delay(100.milliseconds) happens in terms of real-world time, but yes, this has to be done manually.
But if the test framework chose to wait for myCont to resume, we will have a deadlock on our hands in this case:
fun tick() {
myCont.resume(5)
}
private fun hiddenImpl() {
thread {
Thread.sleep(50)
println("Hello :)")
}
}The delayed coroutines may be the key to resuming the main one. This means that choosing to wait for external work to finish before advancing time would break commonplace primitives like Flow, Channel, or CompletableDeferred if the sender side contains a delay. If this wasn't bad enough, there's also no clear way to work around this in general.
@dkhalanskyjb I am trying to get a chance to respond. Sorry. (Dealing with medical issues with family) Honestly, I should have put it on the ticket this one duplicates. I might respond on that ticket when I get a chance.