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

cancelAndIgnoreRemainingEvents() does not cancel a Molecule StateFlow #139

Closed
Egorand opened this issue Sep 3, 2022 · 6 comments
Closed

Comments

@Egorand
Copy link
Collaborator

Egorand commented Sep 3, 2022

This might be Molecule-specific, but I still expect cancelAndIgnoreRemainingEvents() to be able to cancel any StateFlow.

Here's a simple Molecule presenter:

@Composable fun MoleculePresenter(): Model {
  return Model("Hello!")
}

And a test for it:

class MoleculePresenterTest {
  @Test fun test() = runBlocking {
    makePresenter().test {
      assertEquals(Model("Hello!"), awaitItem())
      cancelAndIgnoreRemainingEvents()
    }
  }

  private fun CoroutineScope.makePresenter(): StateFlow<Model> {
    return launchMolecule(RecompositionClock.Immediate) {
      MoleculePresenter()
    }
  }
}

test() never completes when executed, neither with cancelAndIgnoreRemainingEvents() nor without it.

The repro project is here: https://github.com/Egorand/turbine-cancel-hangs. Simply run ./gradlew app:testDebugUnitTest to reproduce the issue.

(Crappy) workaround:

@Test(expected = CancellationException::class)
fun test() = runBlocking {
  makePresenter().test {
    assertEquals(Model("Hello!"), awaitItem())
    coroutineContext.cancel()
  }
}
@JakeWharton
Copy link
Member

JakeWharton commented Sep 3, 2022

As far as Turbine is concerned, this is working as expected. The coroutine which is driving the StateFlow was launched separately from Turbine's own coroutine which collects from the StateFlow. When you cancel, Turbine can only cancel the one which is doing the collection.

You would see similar behavior with code like:

val flow = neverFlow().stateIn(this, Unit)
flow.test { cancelAndIgnoreRemainingEvents() }

If you launch any coroutines separately from Turbine they will also need to be canceled at the end of the test. You can do this by canceling all children of the runBlocking scope, or starting a child scope with an explicit Job that you can cancel, or, in this case, by switching to a moleculeFlow instead of launchMolecule.

@PaulWoitaschek
Copy link
Contributor

In my pet project I overcame this by using runTest in combination with the background dispatcher:
https://github.com/PaulWoitaschek/Voice/blob/4625edbc891327bfa15aff3641b78c153d9444ed/folderPicker/src/test/kotlin/voice/folderPicker/selectType/SelectFolderTypeViewModelTest.kt#L38

@JakeWharton
Copy link
Member

JakeWharton commented Sep 6, 2022

Nice! Can you contribute something to the README about that pattern?

@PaulWoitaschek
Copy link
Contributor

About which pattern? Utilizing molecule + turbine to test composables?

@JakeWharton
Copy link
Member

A bit more general in that the background scope can be used for launching things which produce a StateFlow which become the subject of a Turbine test.

Although, in general, testing a StateFlow with Turbine (at least with 0.9.0) is a bit sketchy. We no longer use the Unconfined dispatcher for collections so you are subject to races with which actual states you see reflected as events. Ideally you can just call .value after performing an action, but that relies on no threading to be in play. If there is threading at play, awaitItem() may conflate two updates into a single event which can surprise.

So I'm not actually sure how detailed we want to get here.

@Egorand
Copy link
Collaborator Author

Egorand commented Sep 8, 2022

Gonna close the issue cause it's not really a bug.

@Egorand Egorand closed this as not planned Won't fix, can't repro, duplicate, stale Sep 8, 2022
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

No branches or pull requests

3 participants