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

Hookup MvRxMocker and add tests #81

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions mvrx/src/main/kotlin/com/airbnb/mvrx/BaseMvRxViewModel.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import io.reactivex.Single
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.CompositeDisposable
import io.reactivex.disposables.Disposable
import io.reactivex.disposables.Disposables
import io.reactivex.functions.Consumer
import io.reactivex.schedulers.Schedulers
import kotlin.reflect.KProperty1
Expand All @@ -24,10 +25,12 @@ abstract class BaseMvRxViewModel<S : MvRxState>(
initialState: S,
private val debugMode: Boolean = false
) : ViewModel() {

internal val stateStore: MvRxStateStore<S> = MvRxStateStore(initialState)

private val tag by lazy { javaClass.simpleName }
private val disposables = CompositeDisposable()
private val backgroundScheduler = Schedulers.single()
private val stateStore: MvRxStateStore<S> = MvRxStateStore(initialState)

init {
if (debugMode) {
Expand Down Expand Up @@ -57,17 +60,16 @@ abstract class BaseMvRxViewModel<S : MvRxState>(
* mutable variables or properties from outside the lambda or else it may crash.
*/
protected fun setState(reducer: S.() -> S) {
if (debugMode) {
// Must use `set` to ensure the validated state is the same as the actual state used in reducer
// Do not use `get` since `getState` queue has lower priority and the validated state would be the state after reduced
stateStore.set {
val firstState = this.reducer()
val secondState = this.reducer()
if (firstState != secondState) throw IllegalArgumentException("Your reducer must be pure!")
firstState
}
} else {
stateStore.set(reducer)
when {
debugMode -> // Must use `set` to ensure the validated state is the same as the actual state used in reducer
// Do not use `get` since `getState` queue has lower priority and the validated state would be the state after reduced
stateStore.set {
val firstState = this.reducer()
val secondState = this.reducer()
if (firstState != secondState) throw IllegalArgumentException("Your reducer must be pure!")
firstState
}
else -> stateStore.set(reducer)
}
}

Expand Down Expand Up @@ -137,6 +139,10 @@ abstract class BaseMvRxViewModel<S : MvRxState>(
successMetaData: ((T) -> Any)? = null,
stateReducer: S.(Async<V>) -> S
): Disposable {
if (stateStore.isMocked) {
return Disposables.disposed()
}

// This will ensure that Loading is dispatched immediately rather than being posted to `backgroundScheduler` before emitting Loading.
setState { stateReducer(Loading()) }

Expand Down
22 changes: 14 additions & 8 deletions mvrx/src/main/kotlin/com/airbnb/mvrx/MvRxStateStore.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import io.reactivex.disposables.CompositeDisposable
import io.reactivex.disposables.Disposable
import io.reactivex.schedulers.Schedulers
import io.reactivex.subjects.BehaviorSubject
import java.util.LinkedList
import java.util.*

/**
* This is a container class around the actual state itself. It has a few optimizations to ensure
Expand All @@ -15,7 +15,11 @@ import java.util.LinkedList
* conditions with each other.
*
*/
internal open class MvRxStateStore<S : Any>(initialState: S) : Disposable {
internal open class MvRxStateStore<S : Any>(
initialState: S
) : Disposable {

internal val isMocked = MvRxMocker.enabled
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 you need get() =, otherwise this won't update

/**
* The subject is where state changes should be pushed to.
*/
Expand All @@ -39,8 +43,8 @@ internal open class MvRxStateStore<S : Any>(initialState: S) : Disposable {
* current state.
*/
val state: S
// value must be present here, since the subject is created with initialState
get() = subject.value!!
// value must be present here, since the subject is created with initialState
get() = MvRxMocker.getMockedState(this) ?: subject.value!!

init {

Expand All @@ -57,8 +61,10 @@ internal open class MvRxStateStore<S : Any>(initialState: S) : Disposable {
* are guaranteed to run before the get block is run.
*/
fun get(block: (S) -> Unit) {
jobs.enqueueGetStateBlock(block)
flushQueueSubject.onNext(Unit)
MvRxMocker.getMockedState(this)?.let(block) ?: run {
jobs.enqueueGetStateBlock(block)
flushQueueSubject.onNext(Unit)
}
}

/**
Expand Down Expand Up @@ -131,8 +137,8 @@ internal open class MvRxStateStore<S : Any>(initialState: S) : Disposable {
val blocks = jobs.dequeueAllSetStateBlocks() ?: return

blocks
.fold(state) { state, reducer -> state.reducer() }
.run { subject.onNext(this) }
.fold(state) { state, reducer -> state.reducer() }
.run { subject.onNext(this) }
}

private fun handleError(throwable: Throwable) {
Expand Down
21 changes: 21 additions & 0 deletions mvrx/src/main/kotlin/com/airbnb/mvrx/TestUtil.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package com.airbnb.mvrx


object MvRxMocker {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you add some documentation to this file? always good to have it on the class and the methods to explain usage a bit


var enabled: Boolean = false
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using static variables can cause a lot of headaches for tests. This will not be reset between tests. So if some test turns this on and forgets to turn it off it will stay on for all remaining tests.

private val mockedState: MutableMap<MvRxStateStore<*>, MvRxState> = mutableMapOf()

fun <S : MvRxState> setMockedState(viewModel: BaseMvRxViewModel<S>, state: S) {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe state should be nullable so the mocked state can be cleared?

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 we may want to have the viewmodel trigger it's subscriber callbacks so they can act on the new state - that seems important for tests that want to test state transitions.

not immediately important - could be a follow up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah that makes sense will address in another PR

mockedState[viewModel.stateStore] = state
}

fun <S : MvRxState> setMockedStateFromArgs(viewModel: BaseMvRxViewModel<S>, args: Any?) {
mockedState[viewModel.stateStore] = _initialStateProvider(viewModel.state::class.java, args)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you chain this to setMockedState?

}

internal fun <S : Any> getMockedState(stateStore: MvRxStateStore<S>): S? {
@Suppress("UNCHECKED_CAST")
return if (enabled) mockedState[stateStore] as? S else null
}
}
60 changes: 60 additions & 0 deletions mvrx/src/test/kotlin/com/airbnb/mvrx/MvRxMockerTest.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package com.airbnb.mvrx

import junit.framework.Assert
import org.junit.Test

class MvRxMockerTest : BaseTest() {

private val initialState = TestState("initial", 0)
private val mockState = TestState("updated", 1)

@Test
fun testDisabled() {
Copy link
Contributor

Choose a reason for hiding this comment

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

nice tests!

MvRxMocker.enabled = false

val viewModel = TestViewModel(initialState)
MvRxMocker.setMockedState(viewModel, mockState)

withState(viewModel) { state ->
Assert.assertEquals(state, initialState)
}
}

@Test
fun testEnabled() {
MvRxMocker.enabled = true

val viewModel = TestViewModel(initialState)
MvRxMocker.setMockedState(viewModel, mockState)

withState(viewModel) { state ->
Assert.assertEquals(mockState, state)
}
}

@Test
fun testMockNotSet() {
MvRxMocker.enabled = true

val viewModel = TestViewModel(initialState)

withState(viewModel) { state ->
Assert.assertEquals(initialState, state)
}
}

@Test
fun testMockArgs() {
MvRxMocker.enabled = true

val viewModel = TestViewModel(initialState)
val args = ParcelableArgs("test args")
MvRxMocker.setMockedStateFromArgs(viewModel, args)

withState(viewModel) { state ->
Assert.assertEquals(TestState(args), state)
}
}

private class TestViewModel(initialState: TestState) : TestMvRxViewModel<TestState>(initialState)
}