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

Unit Test examples for testing the interactors in Mosby MVI #272

Open
curiousily opened this issue Aug 29, 2017 · 6 comments
Open

Unit Test examples for testing the interactors in Mosby MVI #272

curiousily opened this issue Aug 29, 2017 · 6 comments

Comments

@curiousily
Copy link

curiousily commented Aug 29, 2017

Hi,

Not 100% sure if this should be here or on SO, but it looks like a "feature request" so...

Reading the series of blog posts (starting here: http://hannesdorfmann.com/android/mosby3-mvi-1) I stumbled upon the following lines:

  1. Testability
    ...
    With a Model representing State we can simplify our unit test’s code as we can simply check assertEquals(expectedModel, model). This eliminates a lot of objects we otherwise have to mock. Additionally, this removes many verification tests that a certain method has been called i.e. Mockito.verify(view, times(1)).showFoo(). Eventually, this makes our unit test’s code more readable, understandable and finally maintainable as we don’t have to deal that much with implementation details of our real code.

From what I've implemented so far, all of this seems to have a lot of merit. I use mosby (mvi) + mosby-conductor (mvi) + conductor and couldn't be happier. I write Interactors or UseCases (or whatever they should be called) using RxJava and want to unit test them. After all, the real logic of the app is there and I want to test it without much of a hassle.

I would imagine unit testing an Interactor should look something like this:

val expectedViewState = AuthViewState(loggedIn=true)

val mockedAuthProvider:authProvider = mock()
val interactor = AuthInteractor(mockedAuthProvider)
val viewState = interactor.login() (+ some magic to convert RxJava Observable to ViewState)
assertEquals(expectedViewState, viewState)

I would imagine this should work whether or not State Reducer is used. I believe I checked every relevant sample but couldn't find one that delivers.

Can we see some kind of an example related to all that? And, please, no presenter testing code. Of course, all of this might not be relevant to mosby at all. Still, this might be helpful for the community.

Thanks for the great library!

@sockeqwe
Copy link
Owner

sockeqwe commented Aug 29, 2017

Hey here, what exactly do you want to test?
Interactors?

We had a discussion lately (see #168) where I have explained why I think integration testing makes more sense especially with MVI. You may find this approach on testing interesting too. Would like to hear your feedback.

@curiousily
Copy link
Author

Hi again,

Yes, interactors (business logic) only. Don't want to test anything related to Mosby.

It seems like #168 doesn't have anything related to testing? Maybe a typo?

Regards

@sockeqwe
Copy link
Owner

sockeqwe commented Aug 29, 2017

Hey,
sorry, the correct issue is #268 (not #168)

sockeqwe added a commit that referenced this issue Sep 5, 2017
@curiousily
Copy link
Author

curiousily commented Sep 5, 2017

Hi,

This took some time 😄

TL; DR

I want to test my "business logic" and don't want my tests coupled with the delivery mechanism. I don't want my tests to know anything about the existence of Presenters.

The long version:

Read the discussion at #268 and your posts about MVI (about 2-3 times). Also, watched Uncle Bob's Clean Coders videos on TDD and Clean Architecture lectures.

I want my tests to be:

  • As readable/lean as possible
  • As easy to write/add as possible
  • Depend on as few test doubles as possible (that is, seldom use of any insert your favorite mock library )
  • Covering my business logic heavily and test everything else based on developer preference (E.g. I don't want to test a Presenter that simply navigates to another page on click)

I believe that the argument should be "Why skip the Interactor and test the Presenter?". Since the latter will require (much) more work to test properly. For the sake of argument, let's review why Interactor testing is a must (In my opinion, of course):

  • If that's an example of PresenterTest: https://github.com/sockeqwe/mosby/blob/master/sample-mvi/src/test/java/com/hannesdorfmann/mosby3/sample/mvi/view/home/HomePresenterTest.java I would say - it just hurt my eyes.
  • Presenters are part of the delivery mechanism, that is, they are Android specific (in our case). They stay outside of the boundary of my app (Black vertical line in the diagram). If I change the framework (Mosby) or simply remove the need for Presenters in my app I would have to write whole new tests or isolation layer for that.
  • If I want to deliver my app via the web and have some additional use cases for that, how should I test them? Create new isolation layer for "Web" Use Cases?
  • Presenter tests are required, but not for every Presenter (as discussed previously). So tests covering them should be written (and for state reducers) as the need for that arises.
  • One Use Case can be used by multiple Presenters - should I duplicate my test logic just to cover all Presenters?
  • The apparent overuse of a "Test Double" (HomeViewRobot) in the Presenter Test (same as the referenced above) shows that testing is not as simple as MVI promised at first:
assertEquals(expectedViewState, viewState)

That is, of course, because MVI just is a presentation pattern. It shouldn't dictate how we architecture or test our app.

With all that being said, I don't believe that Mosby does anything wrong. Just saying that good samples should include proper testing of Interactors.

Feel free to ask, if any questions arise! 🙂

@sockeqwe
Copy link
Owner

sockeqwe commented Sep 8, 2017

Thanks for taking the time to write down your feedback.

Regarding your original question:
If you just want to test Interactors, then go ahead and write tests like:

@Test void interactorIsReturningFoo(){
   Observable<Foo> observale =  interactor.doSomething();
   Foo foo = observable.blockingFirst();

   assertEquals(expectedFoo, foo);
}

and so on ... But I don't understand how this is related to MVI or Mosby?

If that's an example of PresenterTest: https://github.com/sockeqwe/mosby/blob/master/sample-mvi/src/test/java/com/hannesdorfmann/mosby3/sample/mvi/view/home/HomePresenterTest.java I would say - it just hurt my eyes.

This is not a Presenter Test, this is a Integration test as it tests all layers down from Presenter inclusive Business Logic state reducer etc. Perhaps we have a different understanding / definition of what a presenter test is. The only "Test Double" in this test is the View Layer, but this could easily be run on a real android view too.

Presenters are part of the delivery mechanism, that is, they are Android specific (in our case). They stay outside of the boundary of my app (Black vertical line in the diagram).

I see your point, but I have a different opinion about that which I think comes from the fact that you are advocating for writing unit tests on business logic / Interactors whereas I recommend to write more integration tests instead of unit tests. So the question is: where does a integration test begins? If you have two parts in your business logic an put them together and test them together then it is already an integration test. From my point of view integration tests should cover as much as possible, ideally from (android) View layer to the deepest part of your business logic and all the way back. Unfortunately, real android view tests (instrumentation tests) are slow, hence often we mock the View layer. Nevertheless, I recommend to write integration tests with android View layer running on a real android device.

Hence, in most cases I would draw the boundary of my MVI based app (Black vertical line in the diagram) between view (interface) and presenter unless Presenter is really android specific which I only rarely can remember that this was the case in one of my apps. Either android specific components were in the view layer or the very end of the business logic like a sqlite database. Both implementation details should be hidden behind a Interface (boundary).

If I change the framework (Mosby) or simply remove the need for Presenters in my app I would have to write whole new tests or isolation layer for that

That is the big advantage of integration testing. You see your whole app as a single big black box. The View Interface defines the boundary and usually that is all you need to know. Think of it in a mathematical way. If you have a mathematic function definition like this:

f(x,y) = x + y

then the only thing you need to know to test this function is the public api this function offers: inputs x, y and the return value. But you don't care about the implementation detail (x+y).

If you apply that idea on a app:

f (intentX, intentY) = ... compute state ...  // implementation details

So if f(...) is a function describing your whole app (or for simplicity lets say 1 screen) this is all your blackbox / integrationn needs to know (as public API) to test your app / screen. There is no presenter in this equation. If you remove the presenter, the public API doesn't change. Hence you don't need to write new tests (or adjust any existing test) if you remove the presenter.

If I want to deliver my app via the web and have some additional use cases for that, how should I test them? Create new isolation layer for "Web" Use Cases?

i don't understand the question but if your question is if I take my app and replace the android View with a website or whatever else then your website obviously have to implement the View Interface and that's it. No need to change other internals (implementation details) within the Boundary (Black vertical line in the diagram) .

Presenter tests are required, but not for every Presenter (as discussed previously). So tests covering them should be written (and for state reducers) as the need for that arises.

Again, we might have a different definition of what a presenter test is. I, personally, prefer integration test over unit tests. If you write unit tests for Interactor, State Reducer and Presenter in isolation, you can never be sure that they integrate and work together as expected.

this is a nice illustration of the issue with unit tests: https://twitter.com/ThePracticalDev/status/845638950517706752

So you may need some kind of integration test for those components anyway. But then, what is the point of writing integration and unit tests? You gain nothing if you test things twice.���

One Use Case can be used by multiple Presenters - should I duplicate my test logic just to cover all Presenters?

It sounds rough, but yes you should, otherwise it is not an integration test since this one Use Case is interacting with other parts too, right (otherwise it is exactly the same presenter as an already existing one which is senseless)? Also this ensures that if you refactor that use case, you are sure that it works for all presenters / screens that use this use case, which is not 100% guaranteed if you just write a single unit test for the use case.

The apparent overuse of a "Test Double" (HomeViewRobot) in the Presenter Test (same as the referenced above) shows that testing is not as simple as MVI promised at first:

There is exactly one "Test Double" for the View layer. Everything else are the real app components so the promise:

assertEquals(expectedViewState, viewState)

still holds (if you write integration tests / black box tests aiming to cover all components) because it is exactly like:

state = f (intentX, intentY)
assertEquals(expectedState, state)

@curiousily
Copy link
Author

curiousily commented Sep 12, 2017

Thanks for taking the time. This is a real treat 👍

But I don't understand how this is related to MVI or Mosby?

As I stated previously

Of course, all of this might not be relevant to mosby at all. Still, this might be helpful for the community.

Yes, it is not related to Mosby. I am perfectly fine with not addressing this issue here.

This is not a Presenter Test, this is a Integration test as it tests all layers down from Presenter inclusive Business Logic state reducer etc.

I completely agree. It is an integration test. Integration tests have a (very large) place in developing "proper" apps and should cover all the things you've mentioned.

MVI is awesome beyond a reasonable doubt, I completely agree here.

i don't understand the question but if your question is if I take my app and replace the android View with a website or whatever else then your website obviously have to implement the View Interface and that's it. No need to change other internals (implementation details) within the Boundary (Black vertical line in the diagram) .

Yes, that is correct. I am talking about going to Web (for example) from Android. Let's have another look at the diagram:

The presenter is outside the black vertical line. That makes sense because the presenter prepares ViewModels that are specific for the view. Take for example an Android app that displays a Todo list. Each Todo ViewModel is composed of {name, dueDate, isCompleted} and all is great. Now, you want to present that list via a Web page. Due to the large screens that Web apps are displayed on, we (the designer) decides to display the duration of a task. A GUI is added for editing it, as well. So, we need new ViewModel and Presenter to handle all that.

Again, Integration Tests are needed to ensure that everything is glued together correctly.

What I meant by that:

The apparent overuse of a "Test Double" (HomeViewRobot) in the Presenter Test (same as the referenced above) shows that testing is not as simple as MVI promised at first:

is the fact that we have to use "Test Double" for every one of our tests. Test doubles make testing a bit harder to write and read compared to testing return values of methods. I would prefer to write simpler tests whenever I can, I tend not to skip writing them when that is the case 😄

Personally, the promised simplicity, explicit state presentation and immutability by MVI are there only if my tests are simple to write & read too.

Again, thank you for your time!

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

No branches or pull requests

2 participants