sample() operator on Hot Flow makes runTest hang

[mhernand40]

Describe the bug

Using runTest while testing an Android ViewModel that uses viewModelScope to launch a coroutine that collects from a MutableSharedFlow that gets the sample() operator applied to it, the test hangs forever. If I remove the sample() operator, the test completes.

Provide a Reproducer

  @Test
  fun repro() = runTest {
    val dispatcher = coroutineContext[CoroutineDispatcher]!!
    val flow = MutableSharedFlow<Int>()
    val scope = CoroutineScope(dispatcher)
    scope.launch {
      flow.sample(300L).collect {}
    }
  }

  @Test
  fun noRepro() = runTest {
    val dispatcher = coroutineContext[CoroutineDispatcher]!!
    val flow = MutableSharedFlow<Int>()
    val scope = CoroutineScope(dispatcher)
    scope.launch {
      flow.collect {}
    }
  }

[dkhalanskyjb]

Both of these tests are written incorrectly, because they break structured concurrency. Manually creating a CoroutineScope, passing the dispatcher to it, can lead to various issues; in this case, the coroutine simply will leak without completing. For example, in CoroutineScope(dispatcher).launch { try { flow.collect { } } finally { println("cleaning up resources") } }, the resources won't get cleaned up.

A correct test would be something like this:

  @Test
  fun noRepro() = runTest {
    val flow = MutableSharedFlow<Int>()
    backgroundScope.launch {
      flow.collect {}
    }
  }

[mhernand40]

Thank you for your reply @dkhalanskyjb. But if both tests are written incorrectly, shouldn't both of them hang? My concern is that there are probably plenty of Android ViewModel tests that are being written incorrectly even though they are following the documentation for Testing Kotlin coroutines on Android, particularly the section Setting the Main dispatcher.

Here is a more realistic example that hopefully conveys what I am getting at:

class ReproViewModel : ViewModel() {
  private val flow = MutableSharedFlow<Int>()

  init {
    viewModelScope.launch {
      flow
        .sample(300L) // Test on longer hangs if you comment out this line
        .collect {}
    }
  }
}

class ReproTest {

  // Normally this @Before/@After would be extracted out into a reusable JUnit rule.
  @Before
  fun setUp() {
    Dispatchers.setMain(StandardTestDispatcher())
  }

  @After
  fun tearDown() {
    Dispatchers.resetMain()
  }

  @Test
  fun repro() = runTest {
    ReproViewModel()
  }
}

[dkhalanskyjb]

Yes, that's better, thanks.

For testing coroutines on Android, the recommended approach is to inject the scope: https://developer.android.com/kotlin/coroutines/test#inject-scope This way, you can use viewModelScope in production, but during testing, pass backgroundScope and have the work properly cancelled.

In the "setting the Main dispatcher" section, one-shot operations are shown, ones that perform some task and then finish. For testing such tasks, just setting the main dispatcher is fine. When a task in a viewModelScope outlives the test, you need to cancel it somehow, and injecting the scope becomes mandatory.

The reason repro hangs but noRepro doesn't is that the test dispatcher always knows it still has extra things to do in the repro case (that is, check every 300 ms for new elements), but in the noRepro case, it sees that there is no more work to execute and finishes, even though some coroutines are still running and weren't cleaned up.