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

Coroutine runTest causes unrelated tests to fail #4265

Open
DavidCorrado opened this issue Oct 31, 2024 · 3 comments
Open

Coroutine runTest causes unrelated tests to fail #4265

DavidCorrado opened this issue Oct 31, 2024 · 3 comments
Labels

Comments

@DavidCorrado
Copy link

Use case

When you have runTest after another test with coroutine failure without runTest the unrelated new test(test 3) now fails. See this https://kotlinlang.slack.com/archives/C1CFAFJSK/p1730215759751549?thread_ts=1729193960.135239&cid=C1CFAFJSK
https://github.com/DavidCorrado/CoroutineTestIssue/blob/main/app/src/test/java/com/example/coroutinetest/ExampleUnitTest.kt

Error

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)

Obviously the engineer should make (test 2) runTest and the issue does not exist but it would be good to have a means to handle this more gracefully. Maybe in coroutines if you use it in a test without runTest it crashes.

Note for anyone watching you can work around this issue via
@After fun cleanup(){ runTest{} }

@dkhalanskyjb
Copy link
Collaborator

it would be good to have a means to handle this more gracefully.

We are also not fond of this spooky action at a distance, but I can't think of a way to make tests signal about arbitrary errors without using kotlinx-coroutines-debug and keeping track of every coroutine that was ever created, which introduces some overhead and, by the way, doesn't work on Android: https://github.com/Kotlin/kotlinx.coroutines/tree/master/kotlinx-coroutines-debug#debug-agent-and-android

Maybe in coroutines if you use it in a test without runTest it crashes

There's a common pattern of launching coroutines before the test has started. For example (pseudocode):

class MyTests {
  val scope = TestScope()
  val systemUnderTest = SystemUnderTest(scope.backgroundScope)

  @Test
  fun testSomething() = scope.runTest {
    assertSomethingAboutSystemUnderTest()
  }
}

class SystemUnderTest(val scope: CoroutineScope) {
  init {
    scope.launch {
      while (true) {
        doSomething()
      }
    }
  }
}

If scope.launch were to throw exceptions just because runTest hasn't yet started, this would make writing tests much less flexible and force everyone to put everything in runTest.

Obviously the engineer should make (test 2) runTest and the issue does not exist

That's one solution, but it really is non-obvious.

The recommended practice is to avoid breaking structured concurrency. Instead, consider establishing and propagating a hierarchy of coroutine scopes, making it impossible to create a coroutine-launching class without passing some coroutine scope to it.

@dkhalanskyjb
Copy link
Collaborator

Note for anyone watching you can work around this issue via @After fun cleanup(){ runTest{} }

If this were so easy, by the way, we would do it ourselves somehow, but it's not. The reason you get the error is that the runTest that actually tests something finishes before your coroutines throw exceptions. cleanup() can indeed catch some of the errors if this was a close call, but if those out-of-band coroutines do a bit more work, cleanup() will also finish by the time the exception is actually thrown.

If you write @After fun cleanup() { Thread.sleep(1000); runTest { } }, you'll catch much more exceptions (but still not all: an exception will still be lost if it's thrown 2 seconds later, like GlobalScope.launch { delay(2.seconds); throw IllegalStateException() }), but also, you test suite will run for much longer. It would be best to replace Thread.sleep(1000) with waitUntilAllCoroutinesFinish()... which happens automatically with structured concurrency, but can not happen at all without it.

@DavidCorrado
Copy link
Author

Yeah my solution is not perfect. But it does catch the cases that I think would get through in my app. Hopefully someone can come up with an idea of how to make this weird situation easier to catch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants