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

New "ConfinedTestDispatcher" variation #3279

Open
Mugurell opened this issue May 11, 2022 · 8 comments
Open

New "ConfinedTestDispatcher" variation #3279

Mugurell opened this issue May 11, 2022 · 8 comments
Assignees
Labels

Comments

@Mugurell
Copy link

(Continuing from #3246 (comment))

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.

@dkhalanskyjb dkhalanskyjb self-assigned this May 11, 2022
@dkhalanskyjb
Copy link
Collaborator

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?

@Mugurell
Copy link
Author

Mugurell commented May 12, 2022

Thank you for looking into this @dkhalanskyjb !
Continuing from the example from #3246 (comment) 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.

@dkhalanskyjb
Copy link
Collaborator

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.

@Mugurell
Copy link
Author

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.

@dkhalanskyjb
Copy link
Collaborator

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.

@Mugurell
Copy link
Author

Mugurell commented May 18, 2022

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.

@dkhalanskyjb
Copy link
Collaborator

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?

@Mugurell
Copy link
Author

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.

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