EspressoIdlingResource integration

ZakTaccardi commented 6 years ago

Would be nice to have a wrapper to easily provide support for IdlingResource.

Some examples

RxJava: RxIdler
OkHttpIdlingResource

EDIT: I'm currently injecting my CoroutineDispatchers like so:

open class AppDispatchers(
    val ui: CoroutineDispatcher = UI,
    val background: CoroutineDispatcher = backgroundDefault,
    val network: CoroutineDispatcher = networkDefault
)

For espresso testing, which monitors the async task pool and UI thread for idle conditions, I'm injecting the following:

class EspressoDispatchers : AppDispatchers(
    ui = UI,
    background = AsyncTask.THREAD_POOL_EXECUTOR.asCoroutineDispatcher(),
    network = AsyncTask.THREAD_POOL_EXECUTOR.asCoroutineDispatcher()
)

Here's the problem I'm experiencing, which happens about 1% of the time on our automated tests.

Network call completes.
- monitored by okhttp-idling-resource
a repository level ConflatedBroadcastChannel is updated with the information from the network call.
- done by the background dispatcher
Espresso thinks app is now idle, and throws exception because the app isn't idle yet (see number 4)
a ConflatedBroadcastChannel in the ViewModel (which has been observing the repository level ConflatedBroadcastChannel the whole time) is updated
- done by the background dispatcher
- this happens after Espresso thinks the app is idle (espresso is wrong)

jcornaz commented 6 years ago

Something like the folowing ?

suspend fun IdlingResource.awaitIdle() {
  if (isIdleNow) return

  suspendCoroutine<Unit> { cont ->
    registerIdleTransitionCallback { cont.resume(Unit) }
  }
}

jcornaz commented 6 years ago

Or maybe you meant a way to create an IdlingResource from a Job ?

fun Job.asIdlingResource() = object : IdlingResource {
  override fun getName() = "Coroutine job ${this@asIdlingResource}"

  override fun isIdleNow() = isCompleted

  override fun registerIdleTransitionCallback(callback: IdlingResource.ResourceCallback) {
    invokeOnCompletion { callback.onTransitionToIdle() }
  }
}

ZakTaccardi commented 6 years ago

one problem is that the application can be idle when a coroutine is still running (you're observing a receive channel)

ZakTaccardi commented 6 years ago

updated original description with details.

I'd like the IdlingResource to be done at the CoroutineDispatcher level, similar to how RxIdler works, if possible. Also note: I'll have .consumeEach { } calls on ReceiveChannels, so a coroutine can still be suspended when the app is actually idle.

matejdro commented 5 years ago

This is what I came up with:

class JobCheckingDispatcherWrapper(private val parent: CoroutineDispatcher) :
    CoroutineDispatcher() {
    private val jobs = Collections.newSetFromMap(WeakHashMap<Job, Boolean>())

    var completionEvent: (() -> Unit)? = null

    override fun dispatch(context: CoroutineContext, block: Runnable) {
        context[Job]?.let { addNewJob(it) }
        parent.dispatch(context, block)
    }

    @InternalCoroutinesApi
    override fun dispatchYield(context: CoroutineContext, block: Runnable) {
        context[Job]?.let { addNewJob(it) }
        parent.dispatchYield(context, block)
    }

    private fun addNewJob(job: Job): Boolean {
        job.invokeOnCompletion {
            completionEvent?.invoke()
        }
        return jobs.add(job)
    }

    @ExperimentalCoroutinesApi
    override fun isDispatchNeeded(context: CoroutineContext): Boolean {
        context[Job]?.let { addNewJob(it) }
        return parent.isDispatchNeeded(context)
    }

    val isAnyJobRunning: Boolean
        get() {
            jobs.removeAll { !it.isActive }
            return jobs.isNotEmpty()
        }
}

It seem to work okay for me. It wraps around existing dispatcher and steals its job objects to check whether those are running.

I cannot share IdlingResource implementation, since it is a bit specific to my setup, but you basically check if any injected dispatcher has isAnyJobRunning == true. You can also register completionEvent callback and forward it to IdlingResource.ResourceCallback if there is no other job running.

What this class does not handle though is your ReceiveChannel example. I'm not sure if this is even possible to do generally though (you may want to wait on some receive channels, but not on the others). Maybe inject separate dispatcher for the receive channel and separate one for jobs that need to finish?

yigit commented 5 years ago

I would actually like to have a solution that can work similar to how we can swap Dispatchers.Main (so ability to replace IO and Default as well).

In my toy app, I wrote a dispatcher like this and replace default and IO dispatchers with it for each test (via reflection :-1: ).

I would very much prefer a solution that does work with Dispatchers.IO, Default and Main out of the box so that developers do not need to pass down dispatchers around. It might be still preferred, just shouldn't be mandatory just to make things testable.

below is the tracked dispatcher:

@InternalCoroutinesApi
class TrackedDispatcher(
    private val name : String,
    private val onSubmit: () -> Unit,
    private val onFinish: () -> Unit,
    private val scheduled : ScheduledExecutorService = ScheduledThreadPoolExecutor(10,
        object : ThreadFactory {
            private val idCounter = AtomicInteger(0)
            override fun newThread(runnable: java.lang.Runnable?): Thread {
                return Thread(runnable, "[Tracked]$name-${idCounter.incrementAndGet()}")
            }

        })
) : CoroutineDispatcher(), Delay {
    override fun scheduleResumeAfterDelay(
        timeMillis: Long,
        continuation: CancellableContinuation<Unit>
    ) {
        onSubmit()
        scheduled.schedule(Runnable {
            try {
                continuation.resumeWith(Result.success(Unit))
            } finally {
                onFinish()
            }

        }, timeMillis, TimeUnit.MILLISECONDS)
    }
    @InternalCoroutinesApi
    override fun dispatch(context: CoroutineContext, block: Runnable) {
        onSubmit()
        scheduled.execute {
            try {
                block.run()
            } finally {
                onFinish()
            }
        }
    }

    fun shutdown() {
        scheduled.shutdown()
        scheduled.awaitTermination(10, TimeUnit.SECONDS)
    }
}

elizarov commented 5 years ago

This is related to the discussion in #890 about testing coroutine framework. It seem that an ability to replace built-in dispatchers Dispatchers.Default and Dispatchers.IO for test purposes just like replacing of Dispatchers.Main would greatly help with Espresso integration as you'd be able to to set a dispatcher that registers itself as IdlingResource. Here is a separate issue: #982

Consider the scenario by @ZakTaccardi:

When a repository level ConflatedBroadcastChannel is updated with the information from the network call it would resume a coroutine in the ViewModel (which has been observing the repository level ConflatedBroadcastChannel the whole time). If that coroutine's dispatcher is integrated with Espresso, it receives the the new task (to resume observing coroutine) and is not idle anymore, so Espresso knows that application is not idle yet.

yigit commented 5 years ago

I was checking how this can be implemented but not sure which direction to take. An option might be to do the same MainDispatchers loader mechanism similar to Dispatchers.setMain. Another option might be to open up CommonPool and DefaultScheduler to the test module and use it like TestBase.

Also, seems like there are 2 use cases here: 1) A basic Executor tracking where a Dispatcher is idle only if # of enqueued runnables is 0. 2) A Dispatcher that awaits for all jobs that are ever scheduled, similar to this one.

I think both are necessary. 2 is useful when interacting with external systems where nothing might be running in the app but it might still be awaiting for a system callback to continue. But it has the disadvantage in working with delayed tasks (or never ending jobs). Hence, option 1 might be necessary in some cases.

elizarov commented 5 years ago

It seems to me that if we add ability to setDefault, then one can simply replace it with #890 virtual-time enabled TestDispatcher. Would it help or not?

matejdro commented 5 years ago

We would also need setIO in this case, right?

yigit commented 5 years ago

https://github.com/Kotlin/kotlinx.coroutines/issues/242#issuecomment-485339796 works fine. Should we implement the same ServiceLocator discovery mechanism for the Default dispatcher? (similar to the one in Main?) Or do you have a different implementation in mind?

elizarov commented 5 years ago

The roadmap I have in mind:

261 -- Add extended APIs to DefaultDispatcher to create its subviews like "IO"
982 -- Add Dispatchers.setDefault (no setIO needed because of the above)
Do everything else.

matejdro commented 5 years ago

Wouldn't that be hardcoding implementation details? IO dispatchers is now subview of default dispatchers, but it might not be in the future.

elizarov commented 5 years ago

@matejdro That's not just an implementation that. That is the whole idea to have Dispatchers.IO in the core library, because it is a subview of Dispatchars.Default and you don't need to switch threads to go between the two (see #261 for details)

objcode commented 5 years ago

@elizarov I'm starting to take a look at this a bit more and there's a common test case to consider as a use case:

suspend fun foo() {
    withContext(Dispatchers.IO) { }
}

suspend fun bar() {
    withContext(Dispatchers.Default) { }
}

// tests
@Test
fun foo_runsOnIO() = runBlockingTest {
    foo()
    // here I want to assert that a task was dispatched to IO
}

@Test
fun bar_runsOnDefault() = runBlockingTest {
    bar()
    // here I want to assert bar runs on Default
}

I'm currently thinking the isIdle method @yigit mentioned in the code review for #890 is the right solution for those assertions - but it'd be good to avoid introducing a separate API than what's needed for Espresso if possible.

elizarov commented 5 years ago

Can you please clarify the code in the above comment. When you say "here I want to assert that a task was dispatched to IO" do you really mean that you want to assert that the task was already complete in IO dispatcher (and it is now idle) or what? What is the expected behavior of test coroutine dispatcher when the task goes to "outside" dispatcher? Shall it wait for all other dispatchers to become idle or... ?

objcode commented 5 years ago

Ah yes I see the confusion, let me clarify a bit with the rest of the test.

suspend fun foo() {
    withContext(Dispatchers.IO) { }
}

@Test
fun foo_runsOnIO() = runBlockingTest {
    // setup a testing IO dispatcher
    val testDispatcher = TestCoroutineDispatcher()
    testDispatcher.pauseDispatcher()
    Dispatchers.setIO(TestCoroutineDispatcher)

    foo()

    // assertion that the IO dispatcher was dispatched to
    assertThat(testDispatcher).hasPendingCoroutine()

    // cleanup stuff
    testDispatcher.resumeDispatcher()
    testDispatcher.cleanupTestCoroutines()
}

The actual API surface for how to write that assertion could take a few forms. Imo idle status is probably the easiest one to understand.

 // calls testDispatcher.isIdle to get the status of dispatcher
assertThat(testDispatcher).hasPendingCoroutine()

As an alternative, the arguments passed to withContext could be intercepted directly by the test (in an argument captor style). That would be a great solution that doesn't involve diving into dispatcher implementation to test this - however it is not obvious how one would intercept that since withContext is a top level function.

As an alternative, all of this can be solved by wrapping either withContext or Dispatchers.Default behind an appropriate abstraction. However, it would be better if the APIs exposed a testable surface.

The important part, to me, is that I need to have some way of determining that IO was dispatched to instead of Default in a test to ensure the correctness of the code.

objcode commented 5 years ago

Re: Espresso dispatchers (the original issue). After thinking it over I think @yigit has the right direction in https://github.com/Kotlin/kotlinx.coroutines/issues/242#issuecomment-485316699

In most cases, an Espresso test does not need (or want) time control and should simply wrap the existing Dispatchers with instrumentation. Both of the wrappers Yigit spelled out make sense in different cases, with the default option of "idle when nothing is currently running or queued." A delegate pattern would be a great to add this tracking:

val trackedDispatcher = IdlingResourceDispatcher(Dispatchers.Default)
Dispatchers.setDefault(trackedDispatcher)
idlingRegistry.register(trackedDispatcher)

The second option, "idle when all jobs that have ever passed through the dispatcher are complete" is a more complicated (and surprising) IdlingResource to work with but interesting only for one-shot requests. It could use the same delegate pattern:

val trackedDispatcher = OneShotIdlingResourceDispatcher(Dispatchers.Default)
Dispatchers.setDefault(trackedDispatcher)
idlingRegistry.register(trackedDispatcher)

In both cases, I don't think the correct choice would be to use TestCoroutineDispatcher here since a test of e.g. a button click should not be testing the implementation details of other layers. If a UI test did need to control the return order of multiple coroutines, it could do so without TestCoroutineDispatcher by supplying fakes and mocks.

Q: Are there any use cases that would require a `TestCoroutineDispatcher` to supply an espresso `IdlingResource`?

JoseAlcerreca commented 5 years ago

Q: Are there any use cases that would require a TestCoroutineDispatcher to supply an espresso IdlingResource?

I agree with you: If a UI test did need to control the return order of multiple coroutines, it could do so without TestCoroutineDispatcher by supplying fakes and mocks.

But, would it be hard to allow TestCoroutineDispatcher to be used in a IdlingResourceDispatcher or OneShotIdlingResourceDispatcher in case someone needs to control dispatcher timing inside a UI test? Actually, can you even prevent it?

What if an app uses Dispatchers.Default for both one-shot operations and channels? You'd have to use both types of IdlingResourceDispatchers and I'm not sure they can be used at the same time. In that case you might want to inject different test dispatchers (even if you use the same one, Default, in production).

objcode commented 5 years ago

Yea, once you have anything other than a one shot request you'd have to use a IdlingResourceDispatcher. I don't see a way to consider a suspended coroutine busy in the presence of a potentially infinite loop.

From a larger perspective - the underlying resource that's causing the suspend (e.g. Retrofit, Room etc) should also expose an idling resource in this case to tell espresso work is happening, or the code should be updated to use a counting idling resource.

If the underlying resource exposed an idling resource, the flow would be complicated but create the desired effect. Consider a streaming database read.

(all idle) -> (coroutine idle, database busy) -> (database busy, coroutine busy) -> (database idle, coroutine busy) -> result sent to UI which blocks main -> (all idle)

Q: Integrating with `TestCoroutineDispatcher`

As for integrating this with TestCoroutineDispatcher, right now there's strict type checking in TestCoroutineScope that would make both of these delegates not work with runBlockingTest. It would work with setMain.

The two options I see there:

Relax the type checking to allow any `DelayController` that's also a dispatcher to be passed.

This has a disadvantage of requiring separate idling resource implementations for TestCoroutineDispatcher and regular dispatchers, but it does allow the same pattern to be used for both.

Provide an idle mechanism (https://github.com/Kotlin/kotlinx.coroutines/issues/1202) that allows for the creation of `IdlingResources` from a `TestCoroutineDispatcher`.

This creates a separate API, but maybe that's OK since they're quite different - however it may be surprising that runBlockingTest fails when a TestCoroutineDispatcher is wrapped in IdlingResourceDispatcher

cc @JoseAlcerreca ^

LUwaisA commented 4 years ago

We have been using a delegate pattern as mentioned in objcode's comment. It is a similar idea to the code written by @yigit but allows tests to behave more similarly to production code by wrapping production dispatchers.

class EspressoTrackedDispatcher(private val wrappedCoroutineDispatcher: CoroutineDispatcher) : CoroutineDispatcher() {
    private val counter: CountingIdlingResource = CountingIdlingResource("EspressoTrackedDispatcher for $wrappedCoroutineDispatcher")
    init {
        IdlingRegistry.getInstance().register(counter)
    }

    override fun dispatch(context: CoroutineContext, block: Runnable) {
        counter.increment()
        val blockWithDecrement = Runnable {
            try {
                block.run()
            } finally {
                counter.decrement()
            }
        }
        wrappedCoroutineDispatcher.dispatch(context, blockWithDecrement)
    }

    fun cleanUp() {
        IdlingRegistry.getInstance().unregister(counter)
    }
}

I'm then using the above Dispatcher in a TestRule:

class DispatcherIdlerRule: TestRule {
    override fun apply(base: Statement?, description: Description?): Statement =
        object : Statement() {
            override fun evaluate() {
                val espressoTrackedDispatcherIO = EspressoTrackedDispatcher(Dispatchers.IO)
                val espressoTrackedDispatcherDefault = EspressoTrackedDispatcher(Dispatchers.Default)
                MyDispatchers.IO = espressoTrackedDispatcherIO
                MyDispatchers.Default = espressoTrackedDispatcherDefault
                try {
                    base?.evaluate()
                } finally {
                    espressoTrackedDispatcherIO.cleanUp()
                    espressoTrackedDispatcherDefault.cleanUp()
                    MyDispatchers.resetAll()
                }
            }
        }
}

In the absence of setIO()/setDefault() methods the MyDispatchers class is something we've added to prod code to allow the extra flexibility of setting the dispatchers. It's fairly simple and mimics (and adds to) the public API of Dispatchers. I would rather not have this class but thought it is a small addition and is easily replaced when better options become available:

object MyDispatchers {
    var Main: CoroutineDispatcher = Dispatchers.Main
    var IO: CoroutineDispatcher = Dispatchers.IO
    var Default: CoroutineDispatcher = Dispatchers.Default

    fun resetMain() {
        Main = Dispatchers.Main
    }

    fun resetIO() {
        IO = Dispatchers.IO
    }

    fun resetDefault() {
        Default = Dispatchers.Default
    }

    fun resetAll() {
        resetMain()
        resetIO()
        resetDefault()
    }
}

With this approach, we've unblocked our Espresso testing. Caveat: For our use case we don't currently need it to behave more effectively but the above code tells Espresso that the app is idle during a delay() in production code.

Hope this helps for anyone else struggling with this issue

ZakTaccardi commented 4 years ago

I have found that simply monitoring a CoroutineDispatcher is not sufficient.

Imagine the following scenario :

2 coroutines pass data between each other using .offer(..) (not sure if this matters) via a Channel. Is it not possible for both coroutines to be suspended at the same time and therefore a monitored CoroutineDispatcher would still report that the app is idle, even if there is an item in the Channel waiting to be sent to the other coroutine?

EDIT: this seems to be the scenario that @objcode raised an issue for https://github.com/Kotlin/kotlinx.coroutines/issues/1202#issue-445209002

matejdro commented 4 years ago

You can use CoroutineDispatcher as a hook to get all the jobs though, and then listen to all jobs manually: https://github.com/Kotlin/kotlinx.coroutines/issues/242#issuecomment-458046292

VysotskiVadim commented 4 years ago

Thank you guys, I'm glad that so many people shared their implementation of dispatcher wrappers. You're amaizing :+1:

I used to work with Rx using Rx Idler which is basically scheduler wrapper, example:

override fun onCreate(savedInstanceState: Bundle?) {
    super.onCreate(savedInstanceState)
    setContentView(R.layout.activity_main)
    disposable = Single.create<Int> {
        Thread.sleep(3000)
        it.onSuccess(1)
    }
    .observeOn(AndroidSchedulers.mainThread())
    .subscribeOn(Schedulers.computation())
    .subscribe { next ->
        textView.text = next.toString()
    }
}

@Test
fun testExample() {
    onView(withId(R.id.textView)).check(matches(withText("1")))
}

Example works fine because result of computation schedules to UI thread and then computation scheduler marked as Idle. Almost always (I run this test for a half an hour in cycle with 100% success execution, but I think it should fails sometime) UI thread have enough time to update view with result of computation before test checks text view.

I tried to do something similar for coroutines using dispatcher wrapper provided by @matejdro

private val _state = MutableLiveData<ScreenState>()
val state by lazy<LiveData<ScreenState>> {
        _state.value = ScreenState.Loading
        viewModelScope.launch {
            _state.value = ScreenState.Loaded(useCase.getFeatureData())
        }
        _state
    }

class FeatureUseCaseImpl @Inject constructor(
    @IODispatcher private val ioDispatcher: CoroutineDispatcher
): FeatureUseCase {
    override suspend fun getFeatureData(): FeatureData = withContext(ioDispatcher) {
        delay(1500)
        FeatureData("boo")
    }
}

@Test
fun testDataUpdated() {
    onView(withId(R.id.textView)).check(matches(withText("boo")))
}

Full example

When you run the test you can see how UI is updated with the text 'boo', but test fails. The trick is coroutines have different order of execution rather then in Rx. With coroutines dispatcher wrapper is notified that task is completed and only then result is dispatched to Main dispatcher to update UI. As a result, test checks UI few milliseconds before it's updated.

I tried to fix it by modifying @matejdro `s dispatcher wrapper:

private const val ONE_FRAME = 17L
private fun addNewJob(job: Job): Boolean {
        job.invokeOnCompletion {
            if (isAnyJobRunning.not()) {
                GlobalScope.launch(Dispatchers.Main) {
                    delay(ONE_FRAME * 2)
                    completionEvent?.invoke()
                }
            }
        }
        return jobs.add(job)
    }

This workaround works, but not very stable, if you run test in cycle for about half an hour it would fails at least once.

@matejdro , does your dispatcher work stable at your project? Can you suggest something to improve stability? @elizarov , is the any way to notify dispatcher wrapper after computation result was dispatched to parent coroutine?

fkirc commented 4 years ago

What is the current status about this?

I believe that something like @LUwaisA 's solution is the right way to go. In particular, I would like to have the following properties:

Ability to instrument Dispatchers.IO/Default via a delegate pattern.
Setup this instrumentation from the tests, such that the production code is not polluted.
Out of scope for the core library: By using such an instrumentation, make Coroutines and Espresso work out of the box (similar to AsyncTask, which has been working with Espresso since years).

An example of how this could be implemented is Dispatchers.setMain() in org.jetbrains.kotlinx:kotlinx-coroutines-test. As outlined by @elizarov, a Dispatchers.setDefault() method might do the job just fine.

In the meantime, a workaround is to replace Dispatchers.IO/Default with custom wrapper classes.

sanjeevirajm commented 3 years ago

Check this file. It might be useful. It's fully decoupled from development code. No single line was written in development package. Easy to integrate in any app.

https://github.com/sanjeevirajm/simple_idling_resource_android/blob/main/README.md

A thread named "idlingMonitor" will keep checking whether any other background thread is running for every 20ms and notify EspressoIdlingResource.

TWiStErRob commented 2 years ago

Please tell me that "Close" was a mistake... 🫣 4 years old issue, top 4 voted, that affects pretty much every tested Android project. With this scope, a comment accompanying the close would be nice 🙏.

What would be necessary to get first-party support? Would Google (android-test) consider taking this? Are all the APIs existing and open to implement this correctly without hacking?

qwwdfsad commented 2 years ago

Unfortunately, the issue is out of our scope.

We haven't even decided on https://github.com/Kotlin/kotlinx.coroutines/issues/982 during all these years, and there is always more close-to-the-core important work that cannot be pushed towards the 3rd-party-integration side. Taking into account the fact that we are by no means knowledgeable of Espresso or, what's more important, how exactly it is being used, we'd rather admit that we are unlikely to provide the solution for Espresso, giving a direct hint that Google-first or community-first solution is welcomed instead, than trying to introduce a half-baked solution.

dalewking commented 1 year ago

I had a working solution that worked with version 1.5.2 by creating my own subclass of TestDispatcher. But now trying to upgrade to 1.6.0 I find my old solution is impossible, because a TestScope cannot have a custom implementation of CoroutineDispatcher, it must be an instance of TestCoroutineDispatcher, but that class is locked down and you cannot extend it in any way.

qwwdfsad commented 1 year ago

@dalewking could you please file a separate issue regarding TestDispatcher that explains your use case? We are working on stabilizing TestDispatcher and such feedback may be valuable

dalewking commented 1 year ago

To answer the last comment the easiest change would be to simply make TestDispatcher an interface or at least expose its constructor so users can provide their own implementation for tests. In my case I wanted to simply use delegation to wrap a regular instance of TestDispatcher so that I could then control and Espresso Idling Resource.

Someone asked me for my solution to this with 1.5.2 and here is what I did with 1.5.2, but this solution is not possible with anything after 1.5.2 because TestDispatcher cannot be subclassed due to its constructor being internal and to use Test coroutines you have to have an instance of TestDispatcher:

public class CoroutineDispatcherIdlingResource(
    val counter: CountingIdlingResource,
    val wrapped: TestCoroutineDispatcher,
) : CoroutineDispatcher(), Delay by wrapped, DelayController by wrapped {
    fun wrapBlock(block: Runnable) = Runnable {
        try {
            block.run()
        } finally {
            counter.decrement()
        }
    }

    override fun dispatch(context: CoroutineContext, block: Runnable) {
        counter.increment()
        wrapped.dispatch(context, wrapBlock(block))
    }

    override fun scheduleResumeAfterDelay(
        timeMillis: Long,
        continuation: CancellableContinuation<Unit>,
    ) {
        wrapped.scheduleResumeAfterDelay(
            timeMillis,
            object : CancellableContinuation<Unit> by continuation {
                override fun cancel(cause: Throwable?) =
                    continuation.cancel(cause).also { counter.decrement() }

                @InternalCoroutinesApi
                override fun completeResume(token: Any) =
                    continuation.completeResume(token).also { counter.decrement() }
            },
        )
    }
}

dkhalanskyjb commented 1 year ago

to use Test coroutines you have to have an instance of TestDispatcher

Only for the top level. You can do withContext(yourDispatcherWrappingTestDispatcher) { ... } immediately inside runTest, and it will work just fine.

To answer the last comment the easiest change would be to simply make TestDispatcher an interface

I'm not sure it would help. I didn't check specifically, but it looks like, if we just made TestDispatcher an interface and you implemented it like what you've shown, the new test framework would count anything happening in the main test body as a signal of non-idleness as well, which defeats the purpose.

TWiStErRob commented 1 year ago

You can do withContext(yourDispatcherWrappingTestDispatcher) { ... } immediately inside runTest, and it will work just fine. the new test framework would count anything happening in the main test body

@dkhalanskyjb I might be wrong here, but that's not how Espresso works. In Espresso tests, there's no runTest, no need for a scope in the test, there's no suspend calls. You fire up an Activity which is hidden away deeply as a sync operation, and then everything happens on the main thread's event loop or a background thread outside of the test. We don't have control over the main thread, background threads, coroutines at all from the test, so we have to wait for things to settle in some other async way. This is why Idling resources exist, they need to be able to asynchronously block the execution of the test thread by peeking into the dispatcher/networking/rx chains.

Example:

https://github.com/android/testing-samples/blob/0f22e684a1e31d5c330e5709d0194c3993d2f632/ui/espresso/BasicSample/app/src/androidTest/java/com/example/android/testing/espresso/BasicSample/ChangeTextBehaviorTest.java#L56-L68

After perform(click()) there might be a coroutine fired up in the app code, which does some work in a scope / suspend fun and then withContext's back to the Android main (not test) thread and sets the text (this "how" is not visible to the test). At this point the app is idle and the 3rd onView text verification can proceed.

dkhalanskyjb commented 1 year ago

In Espresso tests, there's no runTest, no need for a scope in the test, there's no suspend calls.

Then it's irrelevant to the discussion with @dalewking, whose complaint is specifically about being unable to pass a wrapped dispatcher as an argument to runTest.

Though the explanation is helpful, thanks!

dalewking commented 1 year ago

Then it's irrelevant to the discussion with @dalewking, whose complaint is specifically about being unable to pass a wrapped dispatcher as an argument to runTest.

That's funny because nowhere did I ever mention runTest. But I do use runTest (actually runBlockingTest because I am stuck at 1.5.2 because of this issue) in my espresso tests. I have methods to launch a fragment for the test that is wrapped in one. May not strictly be necessary but is convenient.

But that is beside the point in this discussion because runTest is not the only way to get a testing coroutine scope to be used. My code is set up to inject scopes into the code beneath the UI using dependency injection so I inject testing scopes that way. There is also the possibility of injecting a test scope using the Dispatchers support in kotlinx-coroutines-test (which I don't use due to being on 1.5.2, which in reality is 1.5.2 copied into my code base to allow it to support iOS).

The real requirement here for hooking it up to Espresso is to have some way to be notified when a test scope has added something to the queue and also to know when something that was added to the queue has now finished executing. The way I did that in 1.5.2 was to subclass CoroutineDispatcher to wrap the true dispatcher and wrap the bit of work to execute in a bit of code that incremented and decremented a CountingIdlingResource. That no longer works because you require a TestDispatcher which no one on the outside can implement.

That certainly is not the only way this could be done. Another way would simply be the ability to register a listener that is called when work is being added to the queue and when that work completes. Another possibility is to allow registering a function that can transform any bit of work.

bobob306 commented 1 year ago

sorry, jumping on the end of this here, @dalewking have you had any luck on this? I've been trying a few different things but without luck

dalewking commented 1 year ago

I have only been able to do it because we have copied the source code for coroutines-test in our project and I modified it to basically create an interface for TestDispatcher then I could create my own implementation of that interface that delegates to a real instance.

What we basically need is a way to intercept and replace the "markers" that the dispatcher is sending to the scheduler. The idea being that we increment the CountingIdlingResource when the dispatcher registers an event with the scheduler and the marker is changed to decrement the counter when the block finishes (or in the case of CancellableContinuation it is cancelled).

This could be done by extending the dispatcher of the scheduler like I am doing, but that is not possible because they locked them down to only allow the class that they wrote. Alternatively they could provide a mechanism to register an interceptor that allow modification.

I'm definitely not done investigating this, but I have something that works for us for now and am in the process of completing upgrade to Kotlin 1.7. After that I need to reinvestigate our whole coroutine strategy.

The other reason we have the source code in our project is because we previously did that to make it usable to common code. I know that it now supports KMM but they totally changed the API at the same time and our code was written to the old API. While they have the old API available in deprecated form, that is only in the JVM version, not common. So we also have the source code to move the migration folder to common until we convert our tests to the new API (which is a major change)

dalewking commented 1 year ago

OK, I have figured out how to do it and it is actually pretty straightforward. My problem was that I was trying to create my own dispatcher that controlled a CountingIdlingResource and pass that into the constructor to TestScope. That will not work as TestScope can only be called with a TestCoroutineDispatcher. However the solution is to create a TestScope and then add to it a dispatcher that controls the Idling resource and defers to another CoroutineDispatcher. So here is my wrapper class now:

class CoroutineDispatcherIdlingResource(
    private val counter: CountingIdlingResource,
    private val wrapped: TestDispatcher,
) : CoroutineDispatcher(), Delay {
    private fun wrapBlock(block: Runnable) = Runnable {
        try {
            block.run()
        } finally {
            counter.decrement()
            counter.dumpStateToLogs()
        }
    }

    @OptIn(ExperimentalStdlibApi::class)
    override fun dispatch(context: CoroutineContext, block: Runnable) {
        counter.increment()
        counter.dumpStateToLogs()
        wrapped.dispatch(wrapped, wrapBlock(block))
    }

    @OptIn(ExperimentalStdlibApi::class)
    override fun scheduleResumeAfterDelay(
        timeMillis: Long,
        continuation: CancellableContinuation<Unit>,
    ) {
        wrapped.scheduleResumeAfterDelay(
            timeMillis,
            object : CancellableContinuation<Unit> by continuation {
                override fun cancel(cause: Throwable?) =
                    continuation.cancel(cause).also { counter.decrement() }

                @InternalCoroutinesApi
                override fun completeResume(token: Any) =
                    continuation.completeResume(token).also { counter.decrement() }
            },
        )
    }
}```

Which I can use like this:

val dispatcher = UnconfinedTestDispatcher() // or StandardTestDispatcher()
val testScope = TestScope(dispatcher)
val idlingDispatcher = CoroutineDispatcherIdlingResource(idlingResource, dispatcher)


idlingDispatcher can be used for Dispatchers.setMain or if you need a CoroutineScope you can simply do:

val idlingScope = testScope + idlingDispatcher

karanjhinga commented 1 year ago

@LUwaisA's solution does seem to work for me with production dispatchers when I use Thread.Sleep but does not wait for delay(timeInMillis) function to finish.

To address this, I can see other's have an implementation of the Delay interface using TestDispatcher, wherein the scheduleResumeAfterDelay method is overridden to Increment or Decrement the counter value before forwarding calls to TestDispatcher.

Is there an alternative way to handle the delay function invocations? Wanted to avoid swapping out production Dispatchers with TestDispatchers just for supporting this use case.

dkhalanskyjb commented 1 year ago

does not wait for delay(timeInMillis) function to finish.

This is interesting. Should it? Arguably, when there is pending work, the system is still idle. For example, the RxJava IdlingResource integration states that there is no more work when nothing is currently running: https://github.com/square/RxIdler/blob/master/rx3-idler/src/main/java/com/squareup/rx3/idler/DelegatingIdlingResourceScheduler.java This makes sense: imagine that there's a task scheduled to run periodically (every 2 seconds, for example). This would mean that the system is never idle if we consider scheduled work to also mean that the system is not idle.

trevjonez commented 1 year ago

does not wait for delay(timeInMillis) function to finish.

This is interesting. Should it? Arguably, when there is pending work, the system is still idle. For example, the RxJava IdlingResource integration states that there is no more work when nothing is currently running: https://github.com/square/RxIdler/blob/master/rx3-idler/src/main/java/com/squareup/rx3/idler/DelegatingIdlingResourceScheduler.java This makes sense: imagine that there's a task scheduled to run periodically (every 2 seconds, for example). This would mean that the system is never idle if we consider scheduled work to also mean that the system is not idle.

Your assessment seems correct to me. This IMO is the idling system pointing out very subtle issues in your test logic. Where I have seen it bite us is on an RPC based search where you POST your query and get back a UUID as a response, then you poll on a GET endpoint with the UUID until the query completes.

Initial workaround was a withContext(Dispatchers.IO) { Thread.sleep(2_000) } in place of the delay(2_000)

Then later we reworked the test code to loop the test thread watching the view hierarchy until said work triggered a matching view tree update with either an error or success condition. Very important to stop the view actions wait loop on both error and success conditions so you can fail as fast as possible.

Looks roughly like this:

onView(isRoot())
  .perform(
    waitForView(
      withId(R.id.some_text_id),
      withId(R.id.some_error_id),
    )
  )

fun waitForView(vararg matchers: Matcher<View>, timeout: Duration = 5.seconds): ViewAction {
    return object : ViewAction {
        private val timeoutMillis = timeout.inWholeMilliseconds

        override fun getConstraints() = isRoot()

        override fun getDescription(): String {
            val subDescription = StringDescription()
            matchers.forEach { it.describeTo(subDescription) }
            return "Wait for a view matching one of: $subDescription; with a timeout of $timeout."
        }

        override fun perform(uiController: UiController, rootView: View) {
            uiController.loopMainThreadUntilIdle()
            val startTime = System.currentTimeMillis()
            val endTime = startTime + timeoutMillis

            do {
                for (child in TreeIterables.breadthFirstViewTraversal(rootView)) {
                    if (matchers.any { matcher -> matcher.matches(child) }) {
                        return
                    }
                }
                uiController.loopMainThreadForAtLeast(100)
            } while (System.currentTimeMillis() < endTime)

            throw PerformException.Builder()
                .withCause(TimeoutException())
                .withActionDescription(this.description)
                .withViewDescription(HumanReadables.describe(rootView))
                .build()
        }
    }
}

Another option would be to put an IdlingResource in your prod logic to keep track of it, but that's gross don't do that.

Kotlin / kotlinx.coroutines