kotlinx-coroutines-test:1.6.1 - Issue migrating from TestCoroutineDispatcher

[AlexDochioiu]

On Android, the viewmodels rely on viewModelScope, which is essentially the same as CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate). If I update a MutableStateFlow multiple times inside viewModelScope.launch I don't seem to be able to collect/intercept anything except the last value set. I already tried changing dispatchers and/or adding runCurrent but it won't collect anything else other than the last value emitted in that scope.

P.S. This used to not be a problem with TestCoroutineDispatcher + TestCoroutineScope.

class SomeViewModelTest {
    
    @Before
    fun setup() {
        Dispatchers.setMain(UnconfinedTestDispatcher())
    }

    @Before
    fun teardown() {
        Dispatchers.resetMain()
    }

    @Test
    fun testAllEmissions() = runTest {
        val values = mutableListOf<Int>()
        val stateFlow = MutableStateFlow(0)
        val job = launch(UnconfinedTestDispatcher(testScheduler)) { // <------
            stateFlow.collect {
                values.add(it)
            }
        }
        val viewModelScope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate)

        viewModelScope.launch {
            runCurrent()
            stateFlow.value = 1
            runCurrent()
            stateFlow.value = 2
            runCurrent()
            stateFlow.value = 3
            runCurrent()
        }

        viewModelScope.cancel()
        job.cancel()
        // each assignment will immediately resume the collecting child coroutine,
        // so no values will be skipped.
        assertEquals(listOf(0, 1, 2, 3), values) // FAILURE
    }
}

Above test fails with is expected:<[0, 1, 2, 3]> but was:<[0, 3]>

[dkhalanskyjb]

The recommended solution, for now, is to use the Turbine library for testing flows.

A quick workaround is to do the following:

    @Test
    fun testAllEmissions() = runTest {
        val values = mutableListOf<Int>()
        val stateFlow = MutableStateFlow(0)
        val job = launch { // <------
            stateFlow.collect {
                values.add(it)
            }
        }
        runCurrent()
        val viewModelScope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate)

        viewModelScope.launch {
            stateFlow.value = 1
            runCurrent()
            stateFlow.value = 2
            runCurrent()
            stateFlow.value = 3
            runCurrent()
        }

        viewModelScope.cancel()
        job.cancel()
        // each assignment will immediately resume the collecting child coroutine,
        // so no values will be skipped.
        assertEquals(listOf(0, 1, 2, 3), values) // FAILURE
    }

In other words, don't make the collection procedure run in an unconfined context, but do spam runCurrent.

The reason this happens is that viewModelScope has an unconfined dispatcher, and the collecting coroutine also does run in an unconfined dispatcher, and when an unconfined coroutine resumes an unconfined coroutine, immediate resumption is not guaranteed.

I think (please do correct me if I'm wrong) that doing in production something similar to what is going on in the test—that is, collecting a MutableSharedFlow in Dispatchers.Unconfined and updating it from Dispatchers.Main.immediate—would yield the same result, that is, to values being conflated. If so, this is working as intended, as we want to avoid any magic that makes code behave differently in tests and production.

If I am wrong and the result in production would be different, this is an important detail of this issue: #3298.

[AlexDochioiu]

I think (please do correct me if I'm wrong) that doing in production something similar to what is going on in the test—that is, collecting a MutableSharedFlow in Dispatchers.Unconfined and updating it from Dispatchers.Main.immediate—would yield the same result, that is, to values being conflated. If so, this is working as intended, as we want to avoid any magic that makes code behave differently in tests and production.

What you said is correct, but you're overseeing some edge cases. In my particular case I have something along the lines of

viewModelScope.launch {
    uiFlow.emit(LoadingState)
    uiFlow.emit(fetchDataState())
}

Where fetchDataState is a suspended function making API call + mapping to a UI state.

In production the two emissions will have maybe 1 second in between due to the network call (so it doesn't matter that StateFlows are conflated). In test when I'm mocking the service and the mapper, fetchDataState() returns instantly.

P.s. That's not an actual code snippet but it's just a similar example to what I have.

[AlexDochioiu]

Also, the above means that one cannot easily do runCurrent since the emission happens in the class being tested. As a workaround I am configuring mocks to do

whenever(mockService.whatever()) doAnswer {
    runCurrent()
    myAnswer
}

but that also feels a bit ugly + it's easy to sneak in multiple emissions in the production code without causing a test failure (which kind of defeats the purpose of testing that I only get the expected emissions).

[dkhalanskyjb]

In production the two emissions will have maybe 1 second in between due to the network call (so it doesn't matter that StateFlows are conflated).

Wouldn't it be a good option then to model this for the test, so that a mocked fetchDataState does delay(1000)? This way, you'll observe just the behavior you need.

[AlexDochioiu]

But then I’d be testing my test implementation which makes no sense. The testing framework should provide a way to unit test all emissions despite conflation.

[dkhalanskyjb]

Do you mean that, in your tests, you actually do network calls etc., instead of mocking them, and the problem is that the test framework does not recognize that real time has passed?

[AlexDochioiu]

No, I don't do network calls in unit tests. I mentioned previously that I mock a service to return my data (via a suspended function).

Making my mock include an artificial delay is wrong (even though I can advance time without a real delay). It means that my service mock makes assumptions regarding the implementation of the real service class being mocked (aka the mock becomes tightly coupled with the actual implementation of the service class being mocked). This is conceptually bad because the implementation of the service class can change at any time.

[dkhalanskyjb]

because the implementation of the service class can change at any time.

If your implementation changes to one that fetches data immediately, then conflation can actually happen in this scenario, which the test currently correctly shows. If you want to test the lack of conflation in cases when the fetching is not immediate, then test that, by making the mocked fetching not immediate.

[michaldrabik]

Just gonna add my example here.
This is just a simple version of the issue but I believe it's wrong that we are unable to test it correctly.

The simple case impossible to test would be that, for instance, when you enter the screen with list of news the VM instantly loads cached news list, updates stateFlow and then starts loading fresh remote data if needed (and updates stateFlow).

I want to be able to test if stateFlow has received both cached data list and remote data list one after another.

@HiltViewModel
class TestViewModel @Inject constructor() : ViewModel() {

  private val _loadingState = MutableStateFlow(false)
  val loadingState = _loadingState.asStateFlow()

  fun loadItems() {
    viewModelScope.launch {
      _loadingState.update { true }
      delay(1000)
      _loadingState.update { false }
      delay(1000)
      _loadingState.update { true }
      delay(1000)
      _loadingState.update { false }
      delay(1000)
      _loadingState.update { true }
    }
  }
}

class TestViewModelTest : BaseMockTest() {

  @get:Rule
  val mainDispatcherRule = MainDispatcherRule()

  private lateinit var SUT: TestViewModel

  @Before
  override fun setUp() {
    super.setUp()
    SUT = TestViewModel()
  }

  @Test
  fun `Should post cached results and then fresh remote results`() = runTest {
    val stateResult = mutableListOf<Boolean>()
    val job = launch(UnconfinedTestDispatcher()) { SUT.loadingState.toList(stateResult) }

    SUT.loadItems()

    assertThat(stateResult.size).isGreaterThan(3) // Fails - only initial and final value is stored in the list.

    job.cancel()
  }
}

class MainDispatcherRule(
  private val testDispatcher: TestDispatcher = UnconfinedTestDispatcher(),
) : TestWatcher() {

  override fun starting(description: Description) {
    super.starting(description)
    Dispatchers.setMain(testDispatcher)
  }

  override fun finished(description: Description) {
    super.finished(description)
    Dispatchers.resetMain()
  }
}

[dkhalanskyjb]

The issue you're having is that loadItems only emits the last value after 4 seconds have passed—at the virtual time of 4 seconds since the start of the test—but the assertion happens at the virtual time 0. Try adding, let's say, a delay(4001), or advanceUntilIdle() between the call to loadItems and the assertion.

[michaldrabik]

@dkhalanskyjb That is correct so thanks for explanation but what about the case where we remove all the artificial delays(1000) from the above. Something like this:

@HiltViewModel
class TestViewModel @Inject constructor(
  private val dataRepository: DataRepository
) : ViewModel() {

  private val _dataState = MutableStateFlow<List<Data>?>(null)
  val dataState = _dataState.asStateFlow()

  fun loadItems() {
    viewModelScope.launch {
      val cachedData = dataRepository.loadCachedData() // Loads instantly
      _dataState.update { cachedData }
      
      val remoteData  = dataRepository.loadRemoteData() // Loads after few seconds 
      _dataState.update { remoteData }
    }
  }
}

In test environment we of course would like to mock both of these methods:

    coEvery { dataRepository.loadCachedData() } returns listOf(something)
    coEvery { dataRepository.loadRemoteData() } returns listOf(something)

[dkhalanskyjb]

dataRepository.loadRemoteData() // Loads after few seconds

coEvery { dataRepository.loadRemoteData() } returns listOf(something)

These lines are at odds with each other. The first line says that it's expected that loadRemoteData will take a few seconds, whereas the second one mocks this function with something that returns the result instantly. The proper way would be to mock it with a function that does delay(1000) or something like that. Then, this case will be the same as the first one you presented.

If dataRepository.loadRemoteData() returned instantly in production, then you would also observe the values conflating in production, which the test correctly shows right now.

[michaldrabik]

@dkhalanskyjb Yeah I have everything figured out by now. Thanks for thanks for explenations and confirming.