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

runTest with combine flow and backgroundScope update bug #4319

Closed
cprestera opened this issue Jan 2, 2025 · 6 comments
Closed

runTest with combine flow and backgroundScope update bug #4319

cprestera opened this issue Jan 2, 2025 · 6 comments

Comments

@cprestera
Copy link

cprestera commented Jan 2, 2025

Trying to create a test case around a viewModel that has flows that trigger other flows while using combine() seems to be showing some weird behavior. I seemed to have narrowed it down to using the Combine() mixed with the backgroundScope in the runTest block, for some reason the combine() is detecting value changes only once and then not again.

Here is a generic example I made of the test:

@Test
    fun genericTest() = runTest(UnconfinedTestDispatcher(), timeout = 15.seconds) {

        val deferred = CompletableDeferred<Int>()

        backgroundScope.launch {
            viewModel.msf1.collectLatest { println("latest msf1: $it") }
        }

        backgroundScope.launch {
            viewModel.msf2.collectLatest { println("latest msf2: $it") }
        }

        backgroundScope.launch {
            viewModel.csf.collectLatest {
                println("latest csf: $it")
                if (it == 3) {
                    deferred.complete(it)
                }
            }
        }

        backgroundScope.launch {
            viewModel.sf.collectLatest {
                println("latest sf: $it")
                if (it == 4) {
                    deferred.complete(it)
                }
            }
        }

        viewModel.msf1.value = 1
        viewModel.msf2.value = 2
        viewModel.msf2.value = 3
        viewModel.msf1.value = 2
        viewModel.msf2.value = 4

        deferred.await()

        assertEquals(4, viewModel.sf.value)
        assertEquals(3, viewModel.csf.value)
    }

Here is the generic flows in my viewModel:

val msf1 = MutableStateFlow(0)
val msf2 = MutableStateFlow(1)
val csf = combine(msf1, msf2) { a, b -> a + b }.stateIn(
        scope = viewModelScope,
        started = SharingStarted.WhileSubscribed(stopTimeoutMillis = 5_000L),
        initialValue = 0
    )
val sf = msf2.mapLatest { it }.stateIn(
        scope = viewModelScope,
        started = SharingStarted.WhileSubscribed(stopTimeoutMillis = 5_000L),
        initialValue = 0
    )

Here is my test output failure:

latest msf1: 0
latest msf2: 1
latest csf: 0
latest csf: 1
latest sf: 0
latest sf: 1
latest msf1: 1
latest msf2: 2
latest sf: 2
latest msf2: 3
latest sf: 3
latest msf1: 2
latest msf2: 4
latest sf: 4

Expected :3
Actual   :1

I would expect:

The flows to update properly. I can verify these flows update correctly in app. Using the backgroundScope, mixed with the viewModelScope that my viewModel has seems to be causing the issue when mixed with combined. My other flow that does not use combine but does use the viewModelScope updates as expected. The combined flow will just never update and the test will timeout.

Not sure if I am missing something, but seems like a bug.

@cprestera cprestera added the bug label Jan 2, 2025
@dkhalanskyjb
Copy link
Collaborator

I couldn't reproduce the issue you're having. I get:

Expected :3
Actual   :6

Which does make sense, because the last value emitted by msf1 and msf2 are 2 and 4, respectively.

Full code that I used:

    object viewModel {
        val scheduler = TestCoroutineScheduler()
        val scope = TestScope(UnconfinedTestDispatcher(scheduler))
        val viewModelScope = scope.backgroundScope
        val msf1 = MutableStateFlow(0)
        val msf2 = MutableStateFlow(1)
        val csf = combine(msf1, msf2) { a, b -> a + b }.stateIn(
            scope = viewModelScope,
            started = SharingStarted.WhileSubscribed(stopTimeoutMillis = 5_000L),
            initialValue = 0
        )
        val sf = msf2.mapLatest { it }.stateIn(
            scope = viewModelScope,
            started = SharingStarted.WhileSubscribed(stopTimeoutMillis = 5_000L),
            initialValue = 0
        )
    }

    @Test
    fun genericTest() = viewModel.scope.runTest(timeout = 15.seconds) {
        val deferred = CompletableDeferred<Int>()

        backgroundScope.launch {
            viewModel.msf1.collectLatest { println("latest msf1: $it") }
        }

        backgroundScope.launch {
            viewModel.msf2.collectLatest { println("latest msf2: $it") }
        }

        backgroundScope.launch {
            viewModel.csf.collectLatest {
                println("latest csf: $it")
                if (it == 3) {
                    deferred.complete(it)
                }
            }
        }

        backgroundScope.launch {
            viewModel.sf.collectLatest {
                println("latest sf: $it")
                if (it == 4) {
                    deferred.complete(it)
                }
            }
        }

        viewModel.msf1.value = 1
        viewModel.msf2.value = 2
        viewModel.msf2.value = 3
        viewModel.msf1.value = 2
        viewModel.msf2.value = 4

        deferred.await()

        assertEquals(4, viewModel.sf.value)
        assertEquals(3, viewModel.csf.value)
    }

I've also tried other variations (have viewModelScope just be a normal CoroutineScope with a properly injected test dispatcher, both using UnconfinedTestDispatcher and StandardTestDispatcher), but only the println output changed, the reason for the assertion failure stayed the same.

@cprestera
Copy link
Author

I think what you have works still because you are setting the viewModelScope to be the backgroundScope:
val scheduler = TestCoroutineScheduler() val scope = TestScope(UnconfinedTestDispatcher(scheduler)) val viewModelScope = scope.backgroundScope

The viewModel I am testing against uses the default scope defined in the viewmodel class. So my runTest block uses the UnconfinedTestDispatcher() with the backgroundScope for the coroutine calls and the flows I am trying to test against in my viewmodel use the default viewmodelScope.

This is the defined scope I am referring to for my viewModel:
public val ViewModel.viewModelScope: CoroutineScope get() = synchronized(VIEW_MODEL_SCOPE_LOCK) { getCloseable(VIEW_MODEL_SCOPE_KEY) ?: createViewModelScope().also { scope -> addCloseable(VIEW_MODEL_SCOPE_KEY, scope) } }

Can you confirm using the androidx.lifecycle ViewModel() class with the default scope if it still works?

@dkhalanskyjb
Copy link
Collaborator

dkhalanskyjb commented Jan 8, 2025

I think what you have works still because you are setting the viewModelScope to be the backgroundScope

I also checked this with other options (including what should be equivalent to the default viewModelScope).

Could it be that you are missing a call to Dispatchers.setMain? If you do call it, what do you pass there? Also, what version of kotlinx.coroutines are you using?

@cprestera
Copy link
Author

I do not call Dispatchers.setMain, within the ViewModel class the viewModelScope is passed to .stateIn().
I am using kotlinx.coroutines 1.9.0.

As mentioned before the combine() flow seems to not work as expected when using my setup. My guess is that the backgroundScope from runTest{} mixed with the combine flow which is scoped by the viewModelScope is causing the issue.

Any other recommendations?

@dkhalanskyjb
Copy link
Collaborator

I do not call Dispatchers.setMain

That's the root of your issues. Please see https://developer.android.com/kotlin/coroutines/test#setting-main-dispatcher and/or https://github.com/Kotlin/kotlinx.coroutines/blob/master/kotlinx-coroutines-test/README.md#dispatchersmain-delegation. The test framework doesn't know anything about tasks happening on your main dispatcher, so the coroutines that are internally spawned by combine effectively run in parallel to your normal test code. I don't know where your main dispatcher comes from if you are not setting it, so it could be that you need to poll its tasks manually, or just Thread.sleep until the actual main thread processes the tasks created by combine. The easiest fix, though, would certainly be making sure your code is properly interacting with the test framework.

@cprestera
Copy link
Author

I see, I must have missed that in the documentation. I was able to get combine to trigger now. I used Dispatchers.setMain().
Thank you!! I will close this bug.

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

No branches or pull requests

2 participants