kotest icon indicating copy to clipboard operation
kotest copied to clipboard

`Dispatchers.setMain()` in `beforeTest` doesn't set dispatcher

Open lukaszkalnik opened this issue 2 years ago • 13 comments

Kotest version 5.6.2 coroutines-test version 1.7.2 Mockk version 1.13.5

I'm using BehaviorSpec. When executing suspending functions in viewmodel under test using testCoroutineScheduler.advanceUntilIdle() inside a Given block, the shouldBe assertion inside the same Given block fails. However, when moving the shouldBe assertion into a When block, the assertion passes.

Also when there is a shouldBe assertion inside the Given block which doesn't rely on any suspending functions execution (i.e. there is no advanceUntilIdle() call needed before the assertion), the assertion passes.

import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import io.kotest.core.spec.IsolationMode.InstancePerTest
import io.kotest.core.spec.style.BehaviorSpec
import io.kotest.core.test.testCoroutineScheduler
import io.kotest.matchers.shouldBe
import io.mockk.coEvery
import io.mockk.mockk
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.setMain
import kotlin.coroutines.coroutineContext

data class ProductResponse(val id: String)

class ProductRepository {

    suspend fun getProducts(ean: String): ProductResponse {
        delay(500)
        return ProductResponse("")
    }
}

data class UiState(
    val product: ProductResponse?,
)

class MyViewModel(
    private val productRepository: ProductRepository = ProductRepository(),
) : ViewModel() {

    val uiState = MutableStateFlow(UiState(null))

    fun onProductEanScan(ean: String) {
        viewModelScope.launch {
            val product = productRepository.getProducts(ean)
            uiState.value = UiState(product)
        }
    }
}

@OptIn(ExperimentalStdlibApi::class, ExperimentalCoroutinesApi::class)
class MyViewModelTest : BehaviorSpec({
    coroutineTestScope = true
    isolationMode = InstancePerTest

    val productResponse = ProductResponse("testId")

    val productRepository = mockk<ProductRepository> {
        coEvery { getProducts(any()) } returns productResponse
    }

    val initialUiState = UiState(null)
    val expectedUiState = UiState(productResponse)

    beforeTest {
        Dispatchers.setMain(coroutineContext[CoroutineDispatcher]!!)
    }

    Given("the shouldBe assertion about viewmodel state after advanceUntilIdle fails") {
        val viewModel = MyViewModel(productRepository)
        viewModel.onProductEanScan("ean")
        testCoroutineScheduler.advanceUntilIdle()
        viewModel.uiState.value shouldBe expectedUiState // The assertion fails, as though the suspending function was not executed

        When("just a wrapper here") {
            Then("some more assertions") {
            }
        }
    }

    Given("the assertion about viewmodel state inside of the When block passes") {
        val viewModel = MyViewModel(productRepository)
        viewModel.onProductEanScan("ean")
        testCoroutineScheduler.advanceUntilIdle()

        When("the shouldBe assertion here passes") {
            viewModel.uiState.value shouldBe expectedUiState // The assertion here passes, the expected state is as it should be after executing the suspending function

            Then("some more assertions") {
            }
        }
    }

    Given("when not using any suspending functions, the assertion about viewmodel state inside the Given block passes") {
        val viewModel = MyViewModel(productRepository)
        viewModel.uiState.value shouldBe initialUiState // The assertion here passes

        When("just a wrapper here") {
            Then("some more assertions") {
            }
        }
    }
})

lukaszkalnik avatar Jul 21 '23 11:07 lukaszkalnik

Could you verify that viewModelScope is running on the testCoroutineDispatcher every time the Given-code is invoked?

Kantis avatar Jul 21 '23 22:07 Kantis

I'm having a problem reproducing the issue currently. Closing it for now.

lukaszkalnik avatar Jul 24 '23 08:07 lukaszkalnik

I managed to reproduce it. The dispatcher inside the Given block was Dispatchers.Main.

lukaszkalnik avatar Jul 24 '23 09:07 lukaszkalnik

This doesn't change even when I set the main dispatcher in my test inside the beforeAny block:

beforeAny {
        val dispatcher = coroutineContext[CoroutineDispatcher]!!
        println("Dispatcher in beforeAny: $dispatcher")
        Dispatchers.setMain(dispatcher)
}

The test prints:

Dispatcher in beforeAny: StandardTestDispatcher[scheduler=kotlinx.coroutines.test.TestCoroutineScheduler@63b7b6a9]

but inside the viewModel the dispatcher is still Dispatchers.Main and the shouldBe assertion inside the Given block still fails.

lukaszkalnik avatar Jul 24 '23 09:07 lukaszkalnik

@OliverO2 don't know if you know the issue off the top of your head ?

sksamuel avatar Nov 12 '23 21:11 sksamuel

Maybe it's related to https://github.com/kotest/kotest/issues/3705

lukaszkalnik avatar Nov 12 '23 21:11 lukaszkalnik

I'm using neither Dispatchers.setMain nor the test dispatcher nor anything DI. Some thoughts:

  • My understanding is that stuff like advanceUntilIdle only makes sense if all coroutines are under control of the (single-threaded) test dispatcher, no exceptions.
  • Dispatchers.Main is possibly dangerous to use for anything but a real UI thread – so might not be suited for tests. One reason is that it uses a non-daemon thread on the JVM, so will block application exit until all of its coroutines terminate.
  • I have seen Compose failing to update the UI with the latest state on rare occasions (lower single-digit percentages in high-load stress tests).

I guess we'd need a self-explanatory minimal reproducer to properly analyze this case.

OliverO2 avatar Nov 12 '23 22:11 OliverO2

I'm using neither Dispatchers.setMain nor the test dispatcher nor anything DI. Some thoughts:

  • My understanding is that stuff like advanceUntilIdle only makes sense if all coroutines are under control of the (single-threaded) test dispatcher, no exceptions.

Why would you have mulitple test dispatchers in a unit test? Isn't the point of unit test to make coroutines "linear" by executing them all on one common testDispatcher?

  • Dispatchers.Main is possibly dangerous to use for anything but a real UI thread – so might not be suited for tests. One reason is that it uses a non-daemon thread on the JVM, so will block application exit until all of its coroutines terminate.

Please look at my original example. This is an Android ViewModel which I'm testing here (maybe it was not clear). It is only logical that it uses Dispatchers.Main. That's why it has to be replaced using Dispatchers.setMain().

  • I have seen Compose failing to update the UI with the latest state on rare occasions (lower single-digit percentages in high-load stress tests).

What does Compose have to do with the issue?

I guess we'd need a self-explanatory minimal reproducer to properly analyze this case.

Ok, I see that I made some shortcuts and omitted some code in my original example, I will update it.

lukaszkalnik avatar Nov 22 '23 13:11 lukaszkalnik

@OliverO2 I made my original example self-contained and executable. I would appreciate if you could take another look. Also maybe the problem is linked to https://github.com/kotest/kotest/issues/3705, namely that the testCoroutineScheduler is not inherited by the nested containers?

lukaszkalnik avatar Nov 22 '23 13:11 lukaszkalnik

I managed to reduce the example a little bit. The problem seems to be that Dispatchers.setMain(coroutineContext[CoroutineDispatcher]!!) inside beforeTest doesn't seem to work as expected. When I call Dispatchers.setMain(...) inside the Given block, the test passes.

import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import io.kotest.core.spec.IsolationMode.InstancePerTest
import io.kotest.core.spec.style.BehaviorSpec
import io.kotest.core.test.testCoroutineScheduler
import io.kotest.matchers.shouldBe
import io.mockk.coEvery
import io.mockk.mockk
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.launch
import kotlinx.coroutines.test.setMain
import kotlin.coroutines.coroutineContext

data class ProductResponse(val id: String)

class ProductRepository {

    suspend fun getProducts(ean: String): ProductResponse {
        delay(500)
        return ProductResponse("")
    }
}

data class UiState(
    val product: ProductResponse?,
)

class MyViewModel(
    private val productRepository: ProductRepository = ProductRepository(),
) : ViewModel() {

    val uiState = MutableStateFlow(UiState(null))

    fun onProductEanScan(ean: String) {
        viewModelScope.launch {
            val product = productRepository.getProducts(ean)
            uiState.value = UiState(product)
        }
    }
}

@OptIn(ExperimentalStdlibApi::class, ExperimentalCoroutinesApi::class)
class MyViewModelTest : BehaviorSpec({
    coroutineTestScope = true
    isolationMode = InstancePerTest

    val productResponse = ProductResponse("testId")

    val productRepository = mockk<ProductRepository> {
        coEvery { getProducts(any()) } returns productResponse
    }

    val expectedUiState = UiState(productResponse)

    beforeTest {
        Dispatchers.setMain(coroutineContext[CoroutineDispatcher]!!)
    }

     Given("Dispatchers.setMain() in beforeTest doesn't work correctly") {
        val viewModel = MyViewModel(productRepository)
        viewModel.onProductEanScan("ean")
        testCoroutineScheduler.advanceUntilIdle()
        viewModel.uiState.value shouldBe expectedUiState // The assertion fails, as though the suspending function was not executed

        When("just a wrapper here") {
            Then("some more assertions") {
            }
        }
    }

    Given("Dispatchers.setMain() inside the test works correctly") {
        Dispatchers.setMain(coroutineContext[CoroutineDispatcher]!!)
        val viewModel = MyViewModel(productRepository)
        viewModel.onProductEanScan("ean")
        testCoroutineScheduler.advanceUntilIdle()
        viewModel.uiState.value shouldBe expectedUiState // The assertion passes

        When("just a wrapper here") {
            Then("some more assertions") {
            }
        }
    }
})

lukaszkalnik avatar Nov 22 '23 15:11 lukaszkalnik

Thanks for the reproducer. I'll be looking into it, but I'm currently occupied with other tasks. Just give me a week or so.

OliverO2 avatar Nov 22 '23 17:11 OliverO2

The example above (reasonably) assumes that, in beforeTest, the expression coroutineContext[CoroutineDispatcher] will return the TestDispatcher. However, in the first invocation of beforeTest this is not the case. A shortened example:

@OptIn(ExperimentalStdlibApi::class)
class MyViewModelTest : BehaviorSpec({
   coroutineTestScope = true
   isolationMode = IsolationMode.InstancePerTest

   suspend fun testDispatcherStatus(): String =
      if (coroutineContext[CoroutineDispatcher]!! is TestDispatcher) "TestDispatcher present" else "TestDispatcher absent"

   beforeTest {
      println("*** beforeTest: ${testDispatcherStatus()}")
   }

   Given("G1") {
      println("*** G1: ${testDispatcherStatus()}")

      When("W1") {
         println("*** W1: ${testDispatcherStatus()}")
         Then("T1") {
            println("*** T1: ${testDispatcherStatus()}")
         }
      }
   }
})

will print:

*** beforeTest: TestDispatcher absent
*** G1: TestDispatcher present
*** beforeTest: TestDispatcher present
*** G1: TestDispatcher present
*** beforeTest: TestDispatcher present
*** W1: TestDispatcher present
*** beforeTest: TestDispatcher present
*** G1: TestDispatcher present
*** beforeTest: TestDispatcher present
*** W1: TestDispatcher present
*** beforeTest: TestDispatcher present
*** T1: TestDispatcher present

This behavior is caused by an anomaly (from the Kotest user's point of view) in the nesting of coroutine scopes, as described in #3759. This issue will be resolved if a solution is implemented as suggested in https://github.com/kotest/kotest/issues/3759#issuecomment-1807258525:

And then, from a user's point of view, we should have a separate coroutine per spec, being the parent of everything at the next lower level (context or test). So the coroutine hierarchy should mirror the test hierarchy. That would be most in line with user expectations, I'd say.

Currently, you cannot assume that the entire spec uses a single TestDispatcher. In fact, each test and each test container gets its own TestDispatcher instance, unrelated to each other.

OliverO2 avatar Nov 27 '23 18:11 OliverO2

Thank you so much for the thorough and quick investigation! Your explanation is very helpful. I suspected something along the lines of unintuitive coroutine scoping.

lukaszkalnik avatar Nov 27 '23 20:11 lukaszkalnik