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

Figure out if we want automatic cancelation at the end of a test block #4

Closed
JakeWharton opened this issue Aug 4, 2020 · 7 comments · Fixed by #107
Closed

Figure out if we want automatic cancelation at the end of a test block #4

JakeWharton opened this issue Aug 4, 2020 · 7 comments · Fixed by #107

Comments

@JakeWharton
Copy link
Member

JakeWharton commented Aug 4, 2020

Right now a test block requires that you cancel, cancel and ignore events, or consume a terminal event before the test block will complete successfully. This creates a very explicit script that details the lifecycle of the underlying collection operation, but with infinite flows (such as those from SQLDelight or presenters) it means every block ends with cancel().

What are the implications of automatically canceling? We would still validate all received events were consumed. cancel() would almost certainly still be an API so that you could explicitly control it. We could rename cancelAndIgnoreRemainingEvents() to just ignoreRemainingEvents().

@Egorand
Copy link
Collaborator

Egorand commented Aug 4, 2020

There should definitely be a more lenient way to test hot Flows, where validating events captured thus far and ignoring the complete event are enough to make the test pass. Having to cancel() the Flow manually in every test does feel annoying.

@JakeWharton
Copy link
Member Author

JakeWharton commented Aug 4, 2020

Its intent is to be explicit. I'm sure someone testing finite flows would remark that having to consume the completion event is also annoying, but it means that if you erroneously make your stream infinite then it would be caught.

@kevincianfarini
Copy link
Contributor

Maybe this is too simple, but would testCancelling be undesirable?

@JakeWharton
Copy link
Member Author

I wouldn't want to ship that. Someone can write that extension themselves for their own codebase.

fun <T> Flow<T>.testCancelling(body: FlowTurbine<T>.() -> Unit) {
  body()
  cancelAndIgnoreRemainingEvents()
}

@mhernand40
Copy link

mhernand40 commented Mar 8, 2022

With Coroutines 1.6.0 and Turbine 0.7.0, I am seeing that cancellation is not required when testing Hot Flows. The following tests all pass for me:

class TestHotFlowWithoutCancellingFlowTurbine {
    @Test
    fun `test runTest + StandardTestDispatcher`() = runTest(StandardTestDispatcher()) {
        testHotFlowWithoutCancellingFlowTurbine()
    }

    @Test
    fun `test runTest + UnconfinedTestDispatcher`() = runTest(UnconfinedTestDispatcher()) {
        testHotFlowWithoutCancellingFlowTurbine()
    }

    @Test
    fun `test runBlockingTest`() = runBlockingTest {
        testHotFlowWithoutCancellingFlowTurbine()
    }

    @Test
    fun `test runBlocking`() = runBlocking {
        testHotFlowWithoutCancellingFlowTurbine()
    }

    private suspend fun testHotFlowWithoutCancellingFlowTurbine() {
        val flow = MutableSharedFlow<Int>()

        flow.test {
            flow.emit(0)
            assertEquals(0, awaitItem())
        }
    }
}

Is this intended? If not, should I file a separate issue for this?

@JakeWharton
Copy link
Member Author

Yes. That is surprising as the collector should stay active and thus test should not return.

@mhernand40
Copy link

mhernand40 commented Mar 8, 2022

Sorry, "Yes" as in this is intended or "Yes" as in I should file a separate issue? I am pretty sure you meant the latter but I just want to be sure. 🙂

I have filed #90.

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

Successfully merging a pull request may close this issue.

4 participants