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 ordering issue of setState calls within withState #89

Merged
merged 1 commit into from
Sep 15, 2018

Conversation

BenSchwab
Copy link
Collaborator

@BenSchwab BenSchwab commented Sep 15, 2018

push adds to the front of the queue, but we want to add to the end to preserve order of setState. The only time the queue will have multiple elements is within a call to withState. This is because the withState block itself will be on the background thread, so setState calls will not run immediately. Currently this test fails with the value of foo on the second call being 1 as the ordering of the setState was reversed:

 withState {
            selectSubscribe(ViewModelTestState::foo) {
                callCount++
                assertEquals(when (callCount) {
                    1 -> 0
                    2 -> 2
                    else -> throw IllegalArgumentException("Unexpected call count $callCount")
                }, it)
            }
            // These will be coalesced into one update
            setState { copy(foo = 1) }
            setState { copy(foo = 2) }
        }
        assertEquals(2, callCount)

Now it passes. Note -- that this also tests the coalescing logic of setState's within with state.

@BenSchwab
Copy link
Collaborator Author

This may be related to what was seen here: #76

Copy link
Contributor

@elihart elihart left a comment

Choose a reason for hiding this comment

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

Very nice catch, glad we have a test case for this now 👍

@BenSchwab BenSchwab merged commit 8815800 into master Sep 15, 2018
@gpeal
Copy link
Collaborator

gpeal commented Sep 21, 2018

@BenSchwab Nice!

@BenSchwab BenSchwab deleted the bschwab--viewmodel-enqueue branch January 26, 2019 05:45
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 this pull request may close these issues.

3 participants