kotlinx.coroutines icon indicating copy to clipboard operation
kotlinx.coroutines copied to clipboard

New "ConfinedTestDispatcher" variation

Open Mugurell opened this issue 3 years ago • 8 comments
trafficstars

(Continuing from https://github.com/Kotlin/kotlinx.coroutines/issues/3246#issuecomment-1123713267)

kotlin-coroutines 1.6.0 came with a new API and multiplatform support.

Trying to decide between UnconfinedTestDispatcher and StandardTestDispatcher we saw the need for maybe a new variation that would combine the two to provide:

  • eager execution of all child coroutines
  • confined execution - always resume on the same thread no matter what the coroutines use.

with the broad goal of getting serial task execution on the same thread and in that allow for testing a bit more than just one task at a time without having to sprinkle runCurrent or advanceTimeUntilIdle all throughout.

Mugurell avatar May 11 '22 13:05 Mugurell

I don't know anything about "middleware", but from a cursory glance, it doesn't look like it utilizes the concept of a thread at all, being a thing for JS. So, are there any downsides to just using UnconfinedTestDispatcher for your use case?

dkhalanskyjb avatar May 12 '22 07:05 dkhalanskyjb

Thank you for looking into this @dkhalanskyjb ! Continuing from the example from https://github.com/Kotlin/kotlinx.coroutines/issues/3246#issuecomment-1119345128 with one remark: everything is Kotlin used for and Android app.

Our app uses MVI stores with middlewares that may launch new coroutines that may emit new actions launching then new coroutines and so on.

In extreme cases we have to now use

store.dispatch(
    EngineAction.LoadUrlAction(
        "test-tab",
        "https://www.firefox.com"
    )
).join()

testScheduler.advanceUntilIdle()
store.waitUntilIdle() // Wait for CreateEngineSessionMiddleware to create the new session

testScheduler.advanceUntilIdle()
store.waitUntilIdle() // Wait for the LinkingMiddleware to load url
testScheduler.advanceUntilIdle()

MVI and the middleware concept can be seen for us as basically the "chain or responsability pattern" - a series of objects for handling a stream of specific actions, each action potentially being handled in a new coroutine possibily on another thread. This architecture and StandardTestDispatcher will lead to the above code snippet just to have everything running. Using UnconfinedTestDispatcher removes the need for all of those advanceUntilIdle calls which is great and the direction we went on here.

Confining everything to one thread would then come to assure us that no delay calls or heavy work being done in one coroutine (possibly running in another thread) would lead to out of sync middlewares / actions / asserts.

Mugurell avatar May 12 '22 14:05 Mugurell

Confining everything to one thread would then come to assure us that no delay calls or heavy work being done in one coroutine (possibly running in another thread) would lead to out of sync middlewares / actions / asserts.

This is the part I don't understand: what is the thing that StandardTestDispatcher guards against that UnconfinedTestDispatcher doesn't? Could you give an example of an "out of sync" something that may happen with UnconfinedTestDispatcher? As I see it, StandardTestDispatcher and UnconfinedTestDispatcher are equivalent in regards to preventing data races: if you mock everything in your test to run with aTestDispatcher, then no races are possible, but if you don't and there are separate threads doing their thing during the tests, the races are possible with both dispatchers.

dkhalanskyjb avatar May 13 '22 07:05 dkhalanskyjb

We were thinking about the unconfined nature of the appropriately named dispatcher.. If because of a previous coroutine a next one starts and runs on a different thread But we wanted the test to run synchronously and block on the execution of all nested coroutines would it be possible for the test to not wait for the execution of that second coroutine? If not and everything will run synchronously then we are fine with the UnconfinedTestDispatcher.

Mugurell avatar May 13 '22 16:05 Mugurell

See the docs on Dispatchers.Unconfined: if your test does not use more than one thread on its own (by manually creating threads or using non-mocked Dispatchers.Default or Dispatchers.IO), UnconfinedTestDispatcher will not create a new one.

dkhalanskyjb avatar May 13 '22 18:05 dkhalanskyjb

We do have some threads "manually" created. For the most common scenarios we can access it / the CoroutineScope using a ExecutorService.asCoroutineDispatcher and join that from the test but just thinking that it would be nice (not sure how it would work under the hood) to not have to do this. Also based on the example from the documentation it would be nice to have a guaranteed "top to bottom" order of execution.

Mugurell avatar May 18 '22 13:05 Mugurell

I'm trying to find this usage that you're describing in the android-components project, but am not successful. Could you give me some pointers? I found the remark about using join unclear, so just pointing me to some tests that use this pattern would help a lot.

Regarding serialization of events, I think that, typically, UnconfinedTestDispatcher is as safe as StandardTestDispatcher, even in the presence of other threads, so if there is some test where you're uneasy about using UnconfinedTestDispatcher, could you please show it as well?

dkhalanskyjb avatar May 18 '22 13:05 dkhalanskyjb

We have to use in many places (not all appearing in the Github query) our custom waitUntilIdle method which will join all children coroutines launched for every event in the app.

The waitUntilIdle is often used in conjuction with another joinBlocking call, seen in the search result from above.

Mugurell avatar May 20 '22 10:05 Mugurell