Test timeout does not work when background scope loops forever and test uses runCurrent

[matejdro]

Describe the bug

When there is an infinite loop in a background scope coroutine, calling runCurrent() in test will cause test to lock up, ignoring the timeout.

Provide a Reproducer

Run following test:

@Test
fun timeoutTest() = runTest(timeout = 1.seconds) {
    backgroundScope.launch {
        while (isActive) {
            yield()
        }
    }

    runCurrent()
}

this test will never complete despite having timeout set as one second.

[dkhalanskyjb]

Yes, that's because runCurrent is not a suspending but a blocking operation, and they can't be cancelled. https://kotlinlang.org/docs/cancellation-and-timeouts.html#cancellation-is-cooperative

[matejdro]

Huh, but the test does complete when launching in regular scope instead of backgroundScope

[dkhalanskyjb]

Yes, because a timeout cancels the main test scope, but the background scope is independent from it cancellation-wise: https://kotlinlang.org/api/kotlinx.coroutines/kotlinx-coroutines-test/kotlinx.coroutines.test/-test-scope/background-scope.html

Failures in coroutines in this scope do not terminate the test. Instead, they are reported at the end of the test. Likewise, failure in the TestScope itself will not affect its backgroundScope, because there's no parent-child relationship between them.

When you just launch in a regular scope, on the other hand, the child coroutine also gets cancelled and yield will throw CancellationException.

So, at least this behavior is documented and can be predicted. Though maybe we could do better.

It would be helpful if you showed the actual test where you encountered this issue. Maybe it is sensible, and you have a point, in which case, yes, we will need to change backgroundScope so that it also gets cancelled; or maybe what you're doing can be rewritten more idiomatically and in a way that doesn't trigger this behavior. I hope it's the latter!

[matejdro]

My test includes test subject, a pretty complicated stack of corourines (lots of nested coroutine scopes etc.) that is collecting flows in infinite loop, so I have to run it in backgroundScope.

To ensure that things actually execute in that background scope, I have to call runCurrent() before performing any assertions inside tests.

For this case, I've managed to screwed up something inside test subject, causing the infinite loop and thus hanging the test. This would likely still be caught by tests, since they would hang forever, but I would much prefer if test would fail in this case, alerting us to the bug in subject's code.

[dkhalanskyjb]

I still don't understand this: runCurrent will loop the scheduled task forever if the loop is truly infinite. When is the test supposed to stop in your scenario?

Since you just want to process background tasks, instead of runCurrent(), you can also do while (true) { yield() }, but this, again, has the issue of never actually finishing. Another viable approach is to replace it with delay(n), where n is how long the background scope is supposed to do its thing. Becuase runTest uses virtual time, this delay will actually be instantaneous and will only affect the task ordering, not make your tests actually take the time n to execute.

[dkhalanskyjb]

Also, you don't have to run it in backgroundScope, you can val job = launch { ... } your task and job.cancel() at the end of the test. Semantically, backgroundScope is supposed to be for background work. If what you're testing is this flow collection, it's in the foreground of the test.

In any case, some code would be welcome. runCurrent is supposed to be needed only rarely, in very specific circumstances.

[matejdro]

Here is an example on why we need runCurrent:

@Test
fun `Demo`() = runTest {
    var value: Boolean = false
    
    launch {
        launch {
            value = true
        }
    }

    runCurrent()

    assertEquals(value, true)
}

where launch block would be the subject under test. runCurrent() is necessary to get nested coroutines to actually launch and complete.

We generally follow this kind of pattern in all our tests:

1. Launch some code that might also use launch underneath
2. runCurrent() (because code can potentially use nested launches, we cannot use simple yield(), so we always use runCurrent() just to be safe and to keep tests future proof)
3. Perform assertions

you can also do while (true) { yield() }

But isn't that essentially equal to runCurrent()? It's more code for the same outcome.

you can val job = launch { ... } your task and job.cancel()

But that is extra boilerplate. Isn't the entire premise of the backgroundScope to avoid this boilerplate?

[dkhalanskyjb]

Your example still doesn't explain the issue you have. The most important part is,

runCurrent will loop the scheduled task forever if the loop is truly infinite. When is the test supposed to stop in your scenario?

In what test do you want infinite loops, runCurrent, and backgroundScope to interplay and why?

Isn't the entire premise of the backgroundScope to avoid this boilerplate?

Background scope is for background work, it has several pieces to its behavior. Being cancelled on test finish is one of them, so it's not the entire premise. For one, neither the test itself nor advanceUntilIdle considers pending work in background scopes as active tasks whose termination must be awaited.

runCurrent() is necessary to get nested coroutines to actually launch and complete.

advanceUntilIdle() would also do that, but without performing work in the background scope. With your example, the reasons to choose one over the other are not clear.

[matejdro]

Our code was essentially something like this:

suspend fun subjectUnderTest(
    infiniteFlowA: Flow<Boolean>,
    infiniteFlowB: Flow<Boolean>
) = coroutineScope {
    val channelForTheFlowA = infiniteFlowA.produceIn(this)
    val channelForTheFlowB = infiniteFlowB.produceIn(this)

    var timeoutValue = 1_000

    try {
        while (isActive) {
            select {
                channelForTheFlowA.onReceive {
                    timeoutValue = someLogicBasedOnFlowAOutput(it)
                }
                channelForTheFlowB.onReceive {
                    timeoutValue = someLogicBasedOnFlowBOutput(it)
                }
                onTimeout(timeoutValue) {
                    setSomethingHereThatWeWantToAssert()
                    timeoutValue = resetTimeoutToSomeValue()
                }
            }
        }
    } finally {
        channelForTheFlowA.cancel()
        channelForTheFlowB.cancel()
    }
}

Code collects data from infinite flows (via select) and then performs value after some timeout. We had a bug in the code where resetTimeoutToSomeValue() method returned 0 all the time, resulting in select going into infinite loop (because it always skipped the timeout).

• That loop never completes, but that's fine, we just want to verify that when some specific value comes from those flows, we can assert the result of that. This made calling it in via normal launch inpractical (we would have to manually cancel it, otherwise test would fail with dangling coroutines. backgroundScope seems like a good alternative with less boilerplate).
• Because flows are produced in separate coroutines, we must call runCurrent after we emit something from the flow to let channels update. In production code, this is buried under several layers of launch, so single yield is not enough.

That leaves us with the following options:

• Avoid use of runCurrent() by using advanceUntilIdle(). But then we would have to use regular launch + manual cancel, which causes extra boilerplate
• Replace runCurrent() with while (true) { yield() }, but that does not solve the issue (as you mentioned.
• Just use backgroundScope and runCurrent(), which is the least boilerplate-y and seems to work fine, except for the loop forever part (this issue).

[dkhalanskyjb]

Ok, I think I got it! Is it correct that the problem is only that, with backgroundScope + runBlocking, the buggy situations when a background task accidentally enters an infinite loop, runCurrent effectively hangs and ignores timeouts? If so, looks like you don't need any workarounds urgently.

Still, your point is valid. Thank you for elaborating on it! This is one more argument for us to provide a suspend alternative to runCurrent: #3919

[matejdro]

Ah yes, sorry, I forgot to mention that the infinite loop is accidental, yes.

Thanks for looking into it. If you want, you can close this and merge it into #3919