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

Add ScriptableStateStore #88

Merged
merged 5 commits into from
Sep 19, 2018
Merged

Add ScriptableStateStore #88

merged 5 commits into from
Sep 19, 2018

Conversation

BenSchwab
Copy link
Collaborator

@BenSchwab BenSchwab commented Sep 14, 2018

This builds upon #81, but uses an interface instead of a companion object to manage mocking.

The motivation behind this is to provide some infrastructure for UI tests which are powered by MvRx. In these tests, you just want to set a state and asssert that the UI is what you expect. To achieve this a state store which ignores standard calls to setState, but can be scripted via a separate method appears to be effective.

@BenSchwab BenSchwab force-pushed the bschwab--testing-changes branch from 72c40b2 to c477152 Compare September 17, 2018 22:59
@BenSchwab BenSchwab changed the title [WIP] ScriptableStateStore Add ScriptableStateStore Sep 17, 2018
@BenSchwab BenSchwab requested review from gpeal, hellohuanlin and elihart and removed request for gpeal September 18, 2018 21:18
Copy link
Contributor

@hellohuanlin hellohuanlin left a comment

Choose a reason for hiding this comment

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

left a question

* how your UI code reacts to different ViewModel states. This is not as useful for unit testing your view model,
* as business logic in state reducers will not be used.
*/
class ScriptableMvRxStateStore<S : Any>(initialState: S) : MvRxStateStore<S> {
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be internal?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's great for other people to use this for testing

// No-op set the state via next
}

fun next(state: S) = subject.onNext(state)
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to use the real store, and call set { newState }, rather than next(newState) here?

Copy link
Contributor

Choose a reason for hiding this comment

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

set on the real store is asynchronous, we need this to be synchronous for tests. Also, we want set to be a no-op so that observables in the real viewmodel don't override our test state

Copy link
Contributor

Choose a reason for hiding this comment

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

i think all tests run synchronously

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@hellohuanlin This is for use in integration tests, specifically screenshot testing. Unit tests only run synchronously because we override RxJava schedulers via plugins, which are global settings, and not something we would want to change in an integration test as it would effect non-mvrx code.

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.

@BenSchwab this seems great, thanks :)

* how your UI code reacts to different ViewModel states. This is not as useful for unit testing your view model,
* as business logic in state reducers will not be used.
*/
class ScriptableMvRxStateStore<S : Any>(initialState: S) : MvRxStateStore<S> {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's great for other people to use this for testing

// No-op set the state via next
}

fun next(state: S) = subject.onNext(state)
Copy link
Contributor

Choose a reason for hiding this comment

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

set on the real store is asynchronous, we need this to be synchronous for tests. Also, we want set to be a no-op so that observables in the real viewmodel don't override our test state


fun next(state: S) = subject.onNext(state)

override fun isDisposed() = false
Copy link
Contributor

Choose a reason for hiding this comment

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

it might be worth tracking this correctly, since tests will deal with lifecycle and this may need to be accurate for whatever reason

@BenSchwab BenSchwab merged commit 250a99c into master Sep 19, 2018
@BenSchwab BenSchwab deleted the bschwab--testing-changes branch January 11, 2019 19:56
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