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

fix issues hidden by Unconfined Dispatcher #169

Merged
merged 6 commits into from
Aug 2, 2021
Merged

Conversation

gabrielittner
Copy link
Member

@gabrielittner gabrielittner commented Jul 29, 2021

This replaces the dispatcher used in tests with Dispatchers.Default. There was a combination of things at play here.

  1. Initial state is now visible to all tests again, this was expected from me. I only removed those expects in StateFlow and CoroutineScope as constructor Parameter #130 because I had to use Unconfined
  2. Any test that relied on a dispatched action for the first state change (OnActionTest and CustomIsInStateDslTest) was affected by an issue in FlowReduxStateMachine. Because of the launch that is done to start the store and it using MutableSharedFlow for incoming actions, any action that is dispatched before the launch body executes was lost. So the tests relying on an action didn't work. In real world use cases this is probably not an issue because you don't start dispatching actions a few millisecods later. I've changed the implementation to a Channel which will suspend the dispatch until we start collecting the actions.
  3. The updated onEnter test was not set up for asynchronous events. expectNoEvents does not suspend or anything to wait that there are no more events, it just checks that there is nothing in it's queue. Because of the async dispatch of A1 the following assertEquals happens too early and the counter is still 0 because not even onEnter of S1 finished running yet. I've changed the test to use GenericState witch changing values so that we can wait for them with expectItem while still staying in the same state. The assertion that onEnter was only called once now happens after the test block when we know all invocations are done.

fixes #128, closes #171

Base automatically changed from tests-launch to main July 30, 2021 09:46
Copy link
Collaborator

@sockeqwe sockeqwe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, still not sure why tests are failing

@gabrielittner
Copy link
Member Author

gabrielittner commented Aug 2, 2021

Everything's green 🎉

@gabrielittner gabrielittner merged commit 007734d into main Aug 2, 2021
@gabrielittner gabrielittner deleted the tests-unconfined branch August 2, 2021 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

onEnter doesn't need FlatMapPolicy Cancel on<Action> job when surrounding inState condition doesn't anymore
2 participants