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

Default state isn't emitted in tests #86

Closed
erikhuizinga opened this issue Oct 29, 2021 · 7 comments
Closed

Default state isn't emitted in tests #86

erikhuizinga opened this issue Oct 29, 2021 · 7 comments

Comments

@erikhuizinga
Copy link
Collaborator

erikhuizinga commented Oct 29, 2021

(Also reported through the Kotlin language Slack here.)

The testing docs (https://github.com/uniflow-kt/uniflow-kt/blob/master/doc/testing.md) suggest that it might be possible to test the default UIState of a DataFlow. In fact, on UniFlow 0.11.6 the following test passes, but on v1.0.10 it fails! This is unexpected and possibly even a bug! The suggestion is to also emit the default state for testing purposes, as this is likely what the user intends to test.

MWE:

// Imports skipped for brevity, and because they differ for v0.11.6 and v1.0.10

class ADFTest {
    @get:Rule
    val instantTaskExecutorRule = InstantTaskExecutorRule()

    @Test
    fun androidDataFlowEmitsDefaultState() {
        val androidDataFlow = object : AndroidDataFlow(defaultState = UIState.Success) {}
        val testObserver = androidDataFlow.createTestObserver()
        assertEquals(1, testObserver.statesCount)
    }
}
@arnaudgiuliani
Copy link
Collaborator

does the default state is not emitting anymore?

@erikhuizinga
Copy link
Collaborator Author

Not on v1.0.10 in the provided MWE.

@ggajews
Copy link
Contributor

ggajews commented Nov 2, 2021

If you look at the current implementation https://github.com/uniflow-kt/uniflow-kt/blob/master/uniflow-android/src/main/java/io/uniflow/android/livedata/LiveDataPublisher.kt#L25 default state is just set as _currentState but never makes it to the _states live data.

@arnaudgiuliani
Copy link
Collaborator

exact yes, it was the point that the first default state to be set is never emitted. Perhaps is something we could allow to setup.

@erikhuizinga is it a 1.0.10 only change? then we can revert

@erikhuizinga
Copy link
Collaborator Author

I tested the versions affected:

✅ 1.0.4 is the last version that passes the test.
❌ 1.0.5 is the first version that fails the test.

Either revert the change or make the states live data emit the initial state, which is for the observer to handle.
Or document it very clearly that the initial state is just internal, and never emitted.
Then this issue can be closed.

I would say that always emitting the initial state is logical, because what's the use otherwise?

@arnaudgiuliani
Copy link
Collaborator

I would say that always emitting the initial state is logical, because what's the use otherwise?

yes clearly. @MarcinChrapowicz any thought on that?

@arnaudgiuliani
Copy link
Collaborator

Ok, let's go for this then 👍 @MarcinChrapowicz will tell us if needed

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

No branches or pull requests

3 participants