sockeqwe / mosby

A Model-View-Presenter / Model-View-Intent library for modern Android apps
http://hannesdorfmann.com/mosby/
Apache License 2.0
5.49k stars 841 forks source link

Testing MVI presenters #268

Closed qwert2603 closed 7 years ago

qwert2603 commented 7 years ago

Hello! In HomePresenterTest.java from sample app presenter is tested as indivisible item. Would it better to separate tests and create tests for stateReducer and tests for partialChangesObservable? (where partialChangesObservable is Observable that is created by merging partial changes from different intents)

sockeqwe commented 7 years ago

I prefer integration tests over unit tests ...

Alex Zhdanov notifications@github.com schrieb am So., 13. Aug. 2017, 14:40:

Hello! In HomePresenterTest.java https://github.com/sockeqwe/mosby/blob/master/sample-mvi/src/test/java/com/hannesdorfmann/mosby3/sample/mvi/view/home/HomePresenterTest.java from sample app presenter is tested as indivisible item. Would it better to separate tests and create tests for stateReducer and tests for partialChangesObservable? (where partialChangesObservable is Observable that is created by merging partial changes from different intents)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sockeqwe/mosby/issues/268, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjnriBTP8sHbHcHBFkT9pKGxW0g23Y5ks5sXu7OgaJpZM4O1rVA .

qwert2603 commented 7 years ago

@sockeqwe ok, but unit test are important, too. I'll try to write some utit tests for presenter from demo app later.

sockeqwe commented 7 years ago

I have to write my opinion down in a blog post, but in a nutshell:

Why do we write unit tests? Because we want to test a single unit in isolation. Why isolation? Because in "traditional software engineering" we write code that is stateful and has side effect. But what if we write software as composition of pure (no side effect, no state) functions? Then it doesn't matter if we test those function in isolation (unit test) or as part of a integration test because for the same input we are guaranteed to get the same output of a function, right? With MVI (if done right) and RxJava we do write such pure function alike code.

Actually, if we write integration tests, we are also testing the single units. For example HomePresenterTest is an integration test and also tests the state recuder? So what's the point of writing a unit test for the state reducer? You don't gain anything by testing your code twice (single unit test for state reducer and integration test "HomePresenterTest" that also covers state reducer).

Therefore, I am much more into integration testing than unit testing.

qwert2603 commented 7 years ago

I agree with it. Looks like we need to change basic approaches of developing apps, because MVI is different from others MV* patterns. But the game is worth candle.

dimsuz commented 7 years ago

Makes sense! Unit tests currently have some benefits attached to them though: like they run quicker and do not require an emulator to run - therefore making them a better suited for CI. The latter can be solved somehow by running emulator in docker I guess, but I didn't try yet.

qwert2603 commented 7 years ago

@dimsuz presenter's test from sample app doesn't require emulator

sockeqwe commented 7 years ago

Another blog post I have to write: In my apps I write the code for tests in such a way that allows me to separate actions on screens (aka Intents) from assertions / evaluations so that I can run the same test specification (a screen action and an assertion) on both, jvm (without emulator) for my daily app TDD alike development and instrumentation test (on emulator on CI server), by only having to maintain 1 test specification for both tests.

charbgr commented 7 years ago

You mean something like Robot pattern?

dimsuz commented 7 years ago

Oh, that would be interesting post to read @sockeqwe ! :)

sockeqwe commented 7 years ago

Actually it is not as fancy as it sounds :) yes, something like robot pattern.

interface HomeScreenRobot {
    fun fireXYIntent()
    fun getRenderedStates() : Observable<List<HomeViewState>>
}

and JvmHomeScreenRobot implements HomeScreenRobot and InstrumentationHomeRobot implements HomeScreenRobot

Then my assertions:

class HomeScreenAssertions (private val robot : HomeScreenRobot){

   privat val timeoutSeconds : Int = 10

     fun `on click on XY --> State1 is rendered`() {
         val expectedStates = listOf (state1)
          robot.fireXYIntent()
         val rendered = robot.getRenderedStates()
                          .timeout(timeoutSeconds)
                           .blockingGet(); // Since observable, we don't even need idling resources at all

        assertEquals(expectedStates, rendered)
     }
}

Then on Jvm Tests:

class JvmHomeScreenTest {

     @Test
     fun `on click on XY --> State1 is rendered`() {
            val robot = JvmHomeScreenrobot()
            val assertions = HomeScreenAssertions(robot)

           assertions.`on click on XY --> State1 is rendered`()
       } 
}

the rest is pretty similar to HomePresenterTest

On instrumentation tests: The trick is that HomeScreenActivity is not implementing the Intents and render method directly, but has a UiBinder injected via dagger:

class HomeActivity : MviActivity<HomeView, HomeViewState> {
    @Inject lateinit var uiBinder : UiBinder

    override fun onCreate(){
         setContentView(...)
        ViewGroup rootView = findViewbyId(...)
        // do dagger injection stuff
       application.getApplicationComponent()
             .homeModule(rootView) // rootView is used for UiBinder
             .build()
            .inject(this)
    }

   override fun xyIntent() : Obervable<XY> = uiBinder.xyIntent() // delegation
   override fun render(HomeViewState state) = uiBinder.render(state) // delegation
}
open class UiBinder(internal val rootView : ViewGroup) : HomeView {

   private val xyButton = rootView.findViewById(...);

   open override fun xyIntent() = RxView.clicks(xyButton);

  open override fun render(state : HomeViewState) {
      // render UI widgets
      ...
   }
}
class InstrumentationHomeScreenTest {
     // TODO prepare Dagger Modules to use InstrumentationUiBinder

     @Test
     fun `on click on XY --> State1 is rendered`() {
            val instrumentationUiBinder : UiBinder = ...
            val robot = InstrumentationHomeScreenrobot(instrumentationUiBinder)
            val assertions = HomeScreenAssertions(robot)

           assertions.`on click on XY --> State1 is rendered`()
       } 
}
class InstrumentationUiBinder(rootView : ViewGroup) : UiBinder(rootView) {
     private var renderdStates = emptyList<HomeState>()
     val stateSubjet  = PublishSubject.create<List<HomeState()>>() // used by robot to subscribe too

     override fun xyIntent() : Observable<xy> {
             // make screenshot of UI --> UI tests without screenshots doesn't make sense
           Screenshot.snap(rootView)
               .record(); 
        return super.xyIntent() // call real UiBinder
    }

    override fun render(state : HomeViewState) {
           super.render(state)

              // make screenshot of UI --> UI tests without screenshots doesn't make sense
            Screenshot.snap(rootView)
                   .record(); 

            // Thread safe since always runs on main ui thread
            renderdStates += state
            publishSubject.onNext(renderedStates) // Notify robot
     }
}

If you want to you can also build a nice DSL for robot and assertion in kotlin. For screenshots we use http://facebook.github.io/screenshot-tests-for-android/

of course this is just a small (pseudocode) illustration of the whole picture. Furthermore, you can easly write integration tests for different screens by using their robots and assertions like doing something in DetailsScreen´ also updatesHomeScreen´

Actually I dont need espresso that much nor idling resources at all. I use the state from ´view.render()to verify that the data (what other people do with espresso like verify that a TextView displays the text xy) and screenshots to verify that UI widgets are displaying things as expected. Makes much more sense to me than using espresso (and custom ViewMatchers) to verify your data is correct displayed. However, if you want to use espresso, feel free to use it inInstrumentationUiBinder. I just use espresso to "tirgger" the button click event inInstrumentationHomeScreenRobot` but not to verify things.

Overall, not that fancy. Just a matter of architecting your test code properly (separation of concerns, dependency injection) as you would do with your regular app code ... The "new" thing might be UiBinder ...

most likely I wouldn't write JVM tests if instrumentation tests run fast enough to do TDD alike development.

Does it make sense?

charbgr commented 7 years ago

Sure!

Nice approach! I don't like so much UI tests and I am more https://martinfowler.com/bliki/TestPyramid.html.

Do InstrumentationUiBinder and InstrumentationHomeRobot make the same work? I usually have a list with rendered states on the Robot class and not on a UiBinder.

dimsuz commented 7 years ago

@sockeqwe thanks for sharing! I will study it and try to apply to some presenter tests to learn this approach. The fact that it is not fancy should be thought as a compliment I guess, because often "fancy" means that there's some complexity involved on some level which could be the sign of a bad design... I'm all for simplicity :)