The first invocation of mockk exceeds the `runTest` default timeout

[PaulWoitaschek]

Run Test Timeout Issues

The new 10s runTest timeout which was introduced by #3603 is currently creating a lot of issues.

There are various discussions about it here:

• Consider lowering runTest default timeout #3270
• https://kotlinlang.slack.com/archives/C1CFAFJSK/p1688240798449439

And it boils down to basically this:
Users of ByteBuddy (for instance mockk users) face a very long initialization phase on the very first mocking initialization.

Example

For a Mac M1 Max, this test:

  @Test
  fun myMockingTest() {
    measureTime {
      mockk<UUID>()
    }.also {
      println("That took $it")
    }
  }

Takes about 1.5 seconds on my machine. CI machines are often times slower than that and might run compilations in parallel or even run multiple projects at once using virtualization.
This has lead to the fact that we do see a lot of sporadically failing CI jobs for tests which don't do much, basides switching to an IO dispatcher to write to a test database.

Proposed Solution

I proposed several solutions here:
https://kotlinlang.slack.com/archives/C1CFAFJSK/p1688326989830229?thread_ts=1688240798.449439&cid=C1CFAFJSK

With my preferred one being:

Making the test timeout configurable through a global option (i.e. an env variable that can be set for all tests from gradle).

That way when running tests locally they could quickly time out but depending on the setup, developers could configure a longer time on the CI where they know that the machine is actually slow

[dkhalanskyjb]

I edited the title of the issue. The original one didn't describe the problem but proposed a solution. So far, it's unclear whether adding a global parameter is the way to go. So far, I didn't see any indications that the timeout we chose isn't sensible, we're dealing with an edge case here.

A solution to this specific problem, proposed in that same thread, is

/**
* Initialize ByteBuddy in advance so it doesn't skew the reported runtime of the test code due to a long initialization sequence.
*/
@BeforeTest
fun initializeByteBuddy() {
  // initialize here
}

This is an easy, straightforward fix for code bases with a class that every test class inherits from, and if you're using kotlinx-coroutines-test, it's likely that you have one in order to call Dispatchers.setMain.

[gildor]

This is an easy, straightforward fix for code bases with a class that every test class inherits from

This solution assumes that there is a class that all tests inherit, neither in our project nor in many projects which I know it's not the case.

It would be more reasonable if it would be a simple way to do this in a multi-module project to specify a custom test runner for all unit tests in all modules, but it's not the case.

One more problem with this proposal is that it will slow down every single affected test because of bytebyddy init, even ones that do not use mocking, and make the whole suite of tests is slower, not faster.

it's likely that you have one in order to call Dispatchers.setMain

We do not override setMain by default because it's unnecessary for most tests, so there is no base test class.
Situations between slow test init and setMain are very different because in case if setMain is required, test will fail immediately and will be fixed, but in case of timeout caused by long environment init, it most probably will pass locally, and maybe even on CI, but become a flaky test

10 seconds is a very arbitrary timeout, and an option to configure it would be very beneficial I see at least to reasonable scenarios:

1. Original issue mentions one more arbitrary but reasonable baseline "10 seconds works for 99.9% of cases". But if 10 seconds start working only for 99% or 97% of test runs, so directly hit test stability. And if one wants to increase stability back to this 99.9%, it may be to increase the timeout to 15 seconds as an easy fix without creating their wrapper on top of runTest
2. If I don't have any potential environment init issues, 10 seconds is just too long, and 1-2 seconds may be more than enough timeout for small classpaths without mock libs in it. Currently, one just cannot configure it and set smaller timeout. I would also point out that if the proposed workaround to move initializeByteBuddy before the test works for the first case, it doesn't work for this case when I just want to make sure that my tests are fast

It will never be a timeout that works for all projects, and the suggestion to significantly change the structure of all tests by using the user's wrapper on top of runTest or by having some base class doesn't look very friendly for projects affected by this.
Another issue is that it increases the chance of a developer's error if one accidentally uses runTest or creates a test class without extending this base class, test will probably pass but would introduce flakiness to the whole suite.

Using jvmArgs for junit runner probably would be the best solution, so at least it would be configured not on the level of a particular test but on the level of a module (and for most projects, it would be easy to apply it to all modules if necessary, or only to ones which have larger classpath, mocking libraries and so on)

[dkhalanskyjb]

One more problem with this proposal is that it will slow down every single affected test because of bytebyddy init, even ones that do not use mocking, and make the whole suite of tests is slower, not faster.

Are you sure? I see mentions of it being a one-time thing, like

Users of ByteBuddy (for instance mockk users) face a very long initialization phase on the very first mocking initialization.

If it's not the case, you are right, multiplying such heavy initialization costs throughout test suites is not acceptable.

It will never be a timeout that works for all projects

So far, 10 seconds seems to be just fine for everyone but bytebuddy users. If it turns out bytebuddy is indeed the only case when this issue is significant, maybe we can work out some workaround specific to bytebuddy and keep the timeout.

[gildor]

Are you sure? I see mentions of it being a one-time thing, like

One time, yes, but there are multiple cases when one want to run tests only in a single module or in a file, or a particular test (during development), in all those cases single test run will load bytebuddy even when it's not needed

Also, it's not necessary a one-time, because multiple parallel test executions in different classloaders are possible each will have this

So far, 10 seconds seems to be just fine for everyone but bytebuddy users

Any slow init will cause it, the fact that butebuddy is often mentioned, is because it's a part of most popular mocking library, so many affected by this, it doesn't mean that there are no other cases. I don't see why some butebuddy specific handling is needed, let users override timeout and keep 10 sec as default.

maybe we can work out some workaround specific to bytebuddy and keep the timeout.

Why not just expose tests timeout with system properties? For one project 10 seconds can be too little, for another too large.

I don't see any actual reason why timeout should be 10 seconds specifically, not 100, or 5 seconds

[dkhalanskyjb]

want to run tests only in a single module or in a file, or a particular test (during development)

Got it, thanks!

Any slow init will cause it

Does any other slow init commonly happen?

let users override timeout and keep 10 sec as default

If it turns out there is a use case for this (or if it turns out there is no nicer way to support the slow ByteBuddy init; I haven't looked into this yet), we'll do it.

Why not just expose tests timeout with system properties?

kotlinx-coroutines-test also works on JS and Native. If it turns out that, indeed, only ByteBuddy is the culprit, system properties are enough. If there is a legitimate use case that is not just a workaround around unfortunate behavior, it won't be enough, some programmatic API will be needed.

[rolgalan]

We have around ~10k unit tests in one of our Android projects. We also use mockk quite extensively, and sometimes also Robolectric (which is also famously renowned by a quite slow loading time). We did this coroutines update last week and we had also some timeout issues in about a dozen of tests.

After inspection we noticed that the problem was actually because those tests were mocking heavily inside the runTest execution scope (which was not happening everywhere in our project, just in some places). As soon as we broke that pattern in favor of mocking outside of this scope, we stopped having timeout for our tests, as then the mockk/bytebuddy initialization penalty happens out of the test scope, and is not subject to any timeout.

Maybe @gildor something like this is what is happening to you and can solve your issue? See an example below:

@Test
fun `this test might timeout due mocking inside the test scope`() = runTest {
      // Test setup
      val what : Ever = mockk()
      every { some.method() } return mockk()

      //The actual test
      sut.testMe()
      verify { what.ever() }
}

@Test
fun `this test should work fine`() {
      // Test setup
      val what : Ever = mockk()
      every { some.method() } return mockk()

      runTest {
          // The actual test
          sut.testMe()
          verify { what.ever() }
      }
}

I believe this pattern might be quite extended because the official Android documentation recommends using runTest as expression body. I believe this is pattern is mainly to avoid using multiple runTest, so there is no other reason to follow it in tests that require to initialize multiple mocks. Actually their example contains only the actual test execution, so most probably this use-case (test methods that need to declare mocks) was not considered when designing that guideline. 🤷

[JakeWharton]

so there is no other reason to follow it

It's required to be used as an expression body for multiplatform tests where the return type of the function changes based on platform.

[rolgalan]

so there is no other reason to follow it

It's required to be used as an expression body for multiplatform tests where the return type of the function changes based on platform.

I was not aware of that requirement for multiplatform, good point. But it's still valid for pure Java/Android projects. I have not much experience with KMP, but also I wonder if multiplatform requires it to be an expression body, or just requires a TestResult as the return of the function? Because then the suggested idea would still apply by just returning the runTest executed inside the function body. What would be the problem with such approach?

@Test
fun `this test should work fine`() : TestResult {
      // Test setup

      return runTest {
          // The actual test
      }
}

[PaulWoitaschek]

Let's have a look at that linked PR that "fixes the test flakiness":
element-hq/element-x-android#1226

These are 406 lines of code which contributes to the overall test complexity and draws the developers attention away from what really matters.

"Add this warmup rule whenever you write a test" is putting a lot of burden on developers. And this will also introduce a lot of new feedback loops. Sometimes developer will forget this. And they won't directly notice it as it's just a source of flakiness. This will lead to situations where all of a sudden a new PR starts failing which is completely unrelated to the changed code.

A property would fix the JVM tests. For us that's the vast majority. Native tests are another story.
I have no real data to answer if 10 seconds is a good default for all currently existing multiplatform targets. From our observations, running tests on iOS / watchos simulators can sometimes be really slow, due to things I don't understand.

My preferred solution for now would be to set the default back to one minute and (maybe) introduce a property to change it on the jvm.