I have an issue where engineers are not realizing ...

# coroutines

[David Corrado]

I have an issue where engineers are not realizing coroutines are being used from a unit test so they do not add runTest. But then a later unrelated test has runTest will then throw these exceptions. So it causes some chaos around someone adding run test and then it causes other errors. Also tests with coroutine failures are sometimes being merged. Obviously the solution is use runTest when you are using coroutines but is there architectures or paths you have found to prevent these errors from accidentally being merged and secondly to keep the tests isolated so you get errors related to the test that is actually in failure rather than an unrelated test.

[Daniel Pitts]

I don't understand what problem you're running into. What exceptions get thrown that you don't expect?

[Marco Pierucci]

Got plenty of coroutine test without runTest and we don't have any issues

[Robert Williams]

You haven't provided examples so I'm just guessing here but if someone doesn't realise there's coroutines in the code under test that sounds like you're exposing regular (not suspend) funs which launch coroutines and don't require injection of Dispatcher/ Context/ Scope. Implementing this should remind/ force people to use runTest and make tests more reliable generally https://developer.android.com/kotlin/coroutines/coroutines-best-practices#inject-dispatchers

[David Corrado]

Finally getting back to this with a code example.
https://github.com/DavidCorrado/CoroutineTestIssue/blob/main/app/src/test/java/com/example/coroutinetest/ExampleUnitTest.kt
(All the code is in this one file for ease of reading)

To replicate this issue you need to have 3 tests. One test that runTest that succeeds. One test that does not have runTest but silently fails in the coroutine. Then the 3rd test which has runTest and it catches the issue from 2 before 3 starts.

Obviously the 2nd test should have runTest but what kind of things can I use to detect this or prevent this. Optimally I would like to have something that checks if runTest is not used and coroutines are used. Or that coroutines are still running after a test. So I want to have solutions that prevent developer error globally. If its not easy to detect this situation is there any feature requests to make this probably common situation able to be detected.

[Daniel Pitts]

I mean, its not obvious that it needs runTest. What error do you see when you run this code?

[David Corrado]

There were uncaught exceptions before the test started. Please avoid this, as such exceptions are also reported in a platform-dependent manner so that they are not lost.
kotlinx.coroutines.test.UncaughtExceptionsBeforeTest: There were uncaught exceptions before the test started. Please avoid this, as such exceptions are also reported in a platform-dependent manner so that they are not lost.
at app//kotlinx.coroutines.test.TestScopeImpl.enter(TestScope.kt:238)
at app//kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:309)
at app//kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0(Unknown Source)
at app//kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0(TestBuilders.kt:168)
at app//kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0(Unknown Source)
at app//kotlinx.coroutines.test.TestBuildersKt__TestBuildersKt.runTest-8Mi8wO0$default(TestBuilders.kt:160)
at app//kotlinx.coroutines.test.TestBuildersKt.runTest-8Mi8wO0$default(Unknown Source)
at app//com.example.coroutinetest.ExampleUnitTest.3) runTest failure before starting(ExampleUnitTest.kt:48)
at java.base@21.0.4/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(Unknown Source)
at java.base@21.0.4/java.lang.reflect.Method.invoke(Unknown Source)
at app//org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at app//org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at app//org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at app//org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at app//org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:61)
at app//org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at app//org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
at app//org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
at app//org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
at app//org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
at app//org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
at app//org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
at app//org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
at app//org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
at app//org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
at app//org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at app//org.junit.runners.ParentRunner.run(ParentRunner.java:413)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.runTestClass(JUnitTestClassExecutor.java:112)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:58)
at org.gradle.api.internal.tasks.testing.junit.JUnitTestClassExecutor.execute(JUnitTestClassExecutor.java:40)
at org.gradle.api.internal.tasks.testing.junit.AbstractJUnitTestClassProcessor.processTestClass(AbstractJUnitTestClassProcessor.java:54)
at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.processTestClass(SuiteTestClassProcessor.java:53)
at java.base@21.0.4/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(Unknown Source)
at java.base@21.0.4/java.lang.reflect.Method.invoke(Unknown Source)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:92)
at jdk.proxy1/jdk.proxy1.$Proxy4.processTestClass(Unknown Source)
at org.gradle.api.internal.tasks.testing.worker.TestWorker$2.run(TestWorker.java:183)
at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:132)
at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:103)
at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:63)
at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:121)
at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71)
at app<//worker.org>.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
at app<//worker.org>.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
Suppressed: java.lang.Exception: test
at com.example.coroutinetest.MyViewModel$fetch$1.invokeSuspend(ExampleUnitTest.kt:62)
at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
at kotlinx.coroutines.internal.DispatchedContinuationKt.resumeCancellableWith(DispatchedContinuation.kt:363)
at kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable(Cancellable.kt:26)
at kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable$default(Cancellable.kt:21)
at kotlinx.coroutines.CoroutineStart.invoke(CoroutineStart.kt:88)
at kotlinx.coroutines.AbstractCoroutine.start(AbstractCoroutine.kt:123)
at kotlinx.coroutines.BuildersKt__Builders_commonKt.launch(Builders.common.kt:52)
at kotlinx.coroutines.BuildersKt.launch(Unknown Source)
at kotlinx.coroutines.BuildersKt__Builders_commonKt.launch$default(Builders.common.kt:43)
at kotlinx.coroutines.BuildersKt.launch$default(Unknown Source)
at com.example.coroutinetest.MyViewModel.fetch(ExampleUnitTest.kt:61)
at com.example.coroutinetest.ExampleUnitTest.2) no runtest success(ExampleUnitTest.kt:44)
... 40 more
Suppressed: kotlinx.coroutines.internal.DiagnosticCoroutineContextException: [CoroutineId(3), "coroutine#3":StandaloneCoroutine{Cancelled}@7103cb56, Dispatchers.Main]

[David Corrado]

IMO if a coroutine is running after you run a test the test should fail. But in this case the coroutine we manually throw and the test does not fail. Which makes sense but it feels like there should be ways to guard against this. It causes unrelated tests to throw and it is pretty painful to deal with

[Daniel Pitts]

This seems more like there isn't enough isolation between your tests...

[David Corrado]

Yes that is what I am aiming for. Am I not following any best practice here?

[Daniel Pitts]

I'm not familiar with Android dev, so I'm probably just missing something in the setup.

[Daniel Pitts]

Let me see if I can repro without android.

[Daniel Pitts]

I was able to reproduce in Junit5 and without android:

import kotlinx.coroutines.*
import kotlinx.coroutines.test.*
import org.junit.jupiter.api.*
import kotlin.test.*
import kotlin.test.Test

@TestMethodOrder(MethodOrderer.MethodName::class)
class ExampleUnitTest {

    @Test
    fun `1) runtest success`() = runTest {
        assertTrue(true)
    }

    @Test
    fun `2) no runtest success`() {
        GlobalScope.launch {
            error("error!")
        }
    }

    @Test
    fun `3) runTest failure before starting`() = runTest {
        assertTrue(
            true,
        )
    }
}

[David Corrado]

Thanks. I created a new branch with basically what you have above and replicates the issue
https://github.com/DavidCorrado/CoroutineTestIssue/blob/no_android/app/src/test/java/com/example/coroutinetest/ExampleUnitTest.kt

[David Corrado]

Can we clarify what is going on here. This is my understanding
When we have runTest it creates a scheduler which is used for all tests in the entire test run. So since the first test has runTest it creates the scheduler. Then the 2nd test uses that scheduler and throws and that exception is kept in the scheduler and the next test(3) has runTest and has code to review the scheduler for failures and throw before it starts.
Questions)
1. Am I right?
2. So the scheduler is what causes lack of isolation?
3. If I delete test 1 this issue does not happen. I imagine that is because runTest creates a singleton scheduler and that is not there yet so I have 2 schedulers in one class which I hear is a bad practice. Also kind of weird that there is this relationship in general

[Daniel Pitts]

From what I can see, coroutines-test registers kotlinx.coroutines.test.internal.ExceptionCollector to the kotlinx.coroutines.internal.platformExceptionHandlers. the ExceptionCollector keeps track of all exceptions that occur once it's been registered. Since the tests aren't run in isolation, the ExceptionCollector receives exceptions.

[Daniel Pitts]

Interestingly, even if I put runTest around the second method, it still fails on the 3rd.

[David Corrado]

I think its because of the globalScope you are using in your example. In the android example the 2nd one fails if oyu do that

[Daniel Pitts]

@AfterTest
fun forceFailIfException() {
    runTest {  }
}

This at least makes it fail at the right place 😉

[David Corrado]

Yeah I found that. I might have to do that but I do think this is an interesting problem worth a feature request if not already filed

[David Corrado]

So the afterTest would be interesting as it would make sure that all coroutines are finished and no exeptions are there. Just feels a little weird tho

[Daniel Pitts]

@Test
fun `2) no runtest success`() = runTest {
    CoroutineScope(<http://Dispatchers.IO|Dispatchers.IO>).launch {
        delay(1000)
        error("error!")
    }
}

This throws a wrench into it though.

[David Corrado]

Yes but my understanding the above is bad practice as you should always pass in dispatchers and not use Dispatchers.IO directly but the test versions

[David Corrado]

So that after would not catch it but hopefully a code review would

[Daniel Pitts]

It's the delay that I'm pointing out.

[Daniel Pitts]

With delay, the exception isn't caught by any of the code.

[David Corrado]

I think if you use the test version of the dispatchers.io it would

[Dmitry Khalanskiy [JB]]

Hi! Author of the test framework here.

> Am I right?
Nope. The reason for the behavior you're seeing is that your program is not using structured concurrency. Normally, when coroutines fail, their failures propagate to the parents of the coroutines, and so on until the root (in case of tests, runTest). Launching coroutines without a hierarchy of coroutine scopes is an antipattern. It's useful in some cases, but very rarely.

If there is no hierarchy of coroutine scopes, there is no way for`runTest` to normally learn about failures in some unrelated coroutines. It's the same as when one of your threads crashes: the other threads won't learn about it.

However, people do write code without structured concurrency support. To help them find the places where their code crashes, we've added special behavior: runTest keeps track of all unhandled exceptions in the system, and if there are any, reports them somewhere, even if they are unrelated to the test. Note that if you launch coroutines outside of structured concurrency, there is no way for runTest to even learn that a coroutine is there: GlobalScope.launch is completely unrelated to runTest, and it can't know to wait for the coroutine to finish.

The way to fix this is not to let your code crash your applications.

> This at least makes it fail at the right place 😉
Not always. It's subject to race conditions: if you write something like GlobalScope.launch { delay(2.seconds); throw IllegalStateException } , then (any) runTest will only catch the error two seconds later.

[Daniel Pitts]

Thanks for chiming in @Dmitry Khalanskiy [JB]! From what I can see, this break from structured concurrency is implicit in the Android library.

[Daniel Pitts]

At least, for David's case.

[David Corrado]

Here is the android version of the code
https://github.com/DavidCorrado/CoroutineTestIssue/blob/main/app/src/test/java/com/example/coroutinetest/ExampleUnitTest.kt

[Dmitry Khalanskiy [JB]]

The recommended practice is described in https://developer.android.com/kotlin/coroutines/test#inject-scope

[David Corrado]

So my android example above is using the best practices from my understanding(except for the one case where a engineer forgets to add runTest on test 2). The only issue I am trying to detect is user error of not marking something as runTest but it has coroutines and actually throws but is ignored. So if another engineer adds another unit test that afterwards has runTest(example 3) it will now start throwing on this new unrelated unit test. I want to avoid this optimally. So optimally by detecting this. Or also we could have no relationship between tests and ignore the exception(Sounds not optimal)

[Dmitry Khalanskiy [JB]]

https://github.com/DavidCorrado/CoroutineTestIssue/blob/main/app/src/test/java/com/example/coroutinetest/ExampleUnitTest.kt does not mock the scope (as is recommended in https://developer.android.com/kotlin/coroutines/test) and instead uses a hardcoded viewModelScope, making it impossible for runTest to know anything about coroutines running there.

[David Corrado]

The docs say to not use viewModelScope? Can you grab a line of text that I can search for in that doc for that?

[Dmitry Khalanskiy [JB]]

The section is "Injecting a scope".

[David Corrado]

I might be mistaken but I read the doc as use that if you need to inject the scope. They have examples in the doc of using viewModelScope. "Here’s an example of a ViewModel implementation that uses viewModelScope to launch a coroutine that loads data:"

[David Corrado]

Does that mean any use of viewModelScope if you need to test the code is incorrect?
NowInAndroid uses it
https://github.com/search?q=repo%3Aandroid%2Fnowinandroid%20viewmodelscope&amp;type=code

[Dmitry Khalanskiy [JB]]

I'm not involved with Android, and so can't comment on what's considered stylistically appropriate there, sorry. I'm just a kotlinx-coroutines maintainer. Maybe you're completely right and using viewModelScope is the way to go, but then I don't see any technical possibility for runTest to report errors happening in viewModelScope accurately. If you have any ideas about how to solve this problem on our side, please share them: we are also not fans of errors that are reported far away from their sources.

[Robert Williams]

I see lots of Googlers (mostly on this Slack) recommending the new pattern of injecting scope but unfortunately there's very little official documentation using this approach sad panda

[David Corrado]

I would imagine you should inject the scope for things without android created scopes. But I would think android created scopes we are intended to use. Mainly its just viewmodelscope

[Daniel Pitts]

I wonder if it is a missing feature of the Android libraries. Perhaps there should be a way to tell the ViewModel class that the context is a unit test, and it could be made to behave better.

[Dmitry Khalanskiy [JB]]

If I remember correctly, any crashes in viewModelScope also crash the whole application, so if the problem is limited to viewModelScope, it shouldn't be widespread: those coroutines must be written in such a way that they never crash anyway.

[Daniel Pitts]

Bugs are bugs, whether they should be or not 😉

[Robert Williams]

But surely the "issue" is that you're not actually asserting anything in the test?
I know it's example code but in a real system you'd be expecting the VM to update some viewState when the coroutine is done
If you asserted on this expected state this would indeed require use of runTest or similar and it would also make the test more meaningful

[David Corrado]

One thing I am looking that I am not sure how it would work. I would like something at the end of all my unit tests are run that it basically waits for all coroutines and error our the gradle job if there was any exceptions. Not sure if that makes sense at all or if its even possible but would be nice. I want to avoid these silent exceptions making its way to prod

[David Corrado]

I would expect an engineer on test 2 to have runTest(anything that touches coroutines should have run test). I am trying to catch engineer error though.

[David Corrado]

My specific issue is more of a unit test issue. It would not affect prod. Basically I had some weird mocking issue that only affects unit tests but crashes coroutines

[David Corrado]

I want to prevent engineers from getting MRs approved and merged with these coroutine errors that cause issues with future MRs when engineers use runTest

[Robert Williams]

I still don't see how this could happen with well written tests. I would expect an engineer writing or reviewing test 2 to say that it's not testing anything. There's always a possibility of missing exceptions or bad behaviour if you don't explicitly require any particular behaviour

[Daniel Pitts]

Imagine the exception happens after the action is getting tested completes.

[Daniel Pitts]

fun theTest() {
   var value = false
   viewModelScope.launch {
       value = true
       error("Bah")
   }
   assertTrue(value)
}

[David Corrado]

The code is still being tested via other unit tests properly. Basically a test that really is not testing something has hidden exceptions gets merged. So the prod code is good. The prod code is still unit tested properly. But there is an unrelated test that happens to run coroutines has an exception because of an engineer doing mocks incorrectly and not seeing the error as the test succeed

[David Corrado]

I am not claiming here that my team is doing things right or even well 😃. But I would like to catch these issues.

[David Corrado]

So hypothetically an engineer can accidently commit 100 errors that have no runTest afterwards. Then later another engineer does a small fix with 1 unit test with runTest that happens to run later and has to fix all of the 100 errors

[Robert Williams]

Point is though, if you're calling code in tests, even if you don't know it uses coroutines (which is mostly an implementation detail), you must be expecting some external behaviour (either as a result, a state change, or interacting with an external dependency). If you test this behaviour that'll implicitly require the runTest to make the test pass. If you truly have methods that start coroutines that have nothing to do with the observable behaviour, that might be a different issue in responsibilities

[Robert Williams]

Complete side note but does InstantTaskExecutorRule change anything in how you expect these tests to behave? I know it's also something you have to remember to use but I'm just surprised because I don't think I've ever had a ViewModel test that didn't require this to pass

[David Corrado]

With instantTask
https://github.com/DavidCorrado/CoroutineTestIssue/blob/android_instant/app/src/test/java/com/example/coroutinetest/ExampleUnitTest.kt

I dont see anything in this case. We do have this in our repo but are not always using it. But it feels like from everyone's responses it is intended to work like this.

I would still like some means to detect and stop unitTests from passing in this case

[Oliver.O]

I'm not familiar with ViewModel setups for testing, but I'm wondering if using the constructor ViewModel(viewModelScope = this) within a runTest invocation's TestScope would make it play with structured concurrency.