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

MvRxLifecycleAwareObserver breaks unit testability #198

Closed
tasomaniac opened this issue Feb 12, 2019 · 4 comments · Fixed by #235

Comments

@tasomaniac
Copy link
Contributor

@tasomaniac tasomaniac commented Feb 12, 2019

Currently all kind of subscribe and asyncSubscribe methods strongly depend on MvRxLifecycleAwareObserver which then depends on Android APIs.

Example call:

viewModel.asyncSubscribe(LoginState::token, onSuccess = {
    navController.navigate(R.id.open_search)
 })

It is almost impossible to unit test the implementation of functions given into asyncSubscribe and its siblings. Basically in unit tests, the fragment lifecycle will never be in resumed state and that's why MvRxLifecycleAwareObserver never calls the subsriber function.

@rom4ek

This comment has been minimized.

Copy link
Contributor

@rom4ek rom4ek commented Feb 17, 2019

Agree here, looks like this needs to be exposed, or at least be VisibleForTesting for that.

@tasomaniac

This comment has been minimized.

Copy link
Contributor Author

@tasomaniac tasomaniac commented Feb 17, 2019

@rom4ek Technically the link you mentioned make sense. I can even use that function right now since it is only protected by lint and is public.

But since I'm testing the view layer, my code uses the one defined in MvRxView and that automatically retrieves the lifecycleOwner from the given Fragment.

@rom4ek

This comment has been minimized.

Copy link
Contributor

@rom4ek rom4ek commented Feb 17, 2019

@tasomaniac Yeah, that's what I meant - it should be exposed via extension in MvRxView I guess.

@tasomaniac

This comment has been minimized.

Copy link
Contributor Author

@tasomaniac tasomaniac commented Mar 29, 2019

Another idea to solve this is to introduce this functionality into already existing TestRule: https://github.com/airbnb/MvRx/blob/master/testing/src/main/kotlin/com/airbnb/mvrx/test/MvRxTestRule.kt

This test rule would hook up into internals of MvRx and replace MvRxLifecycleAwareObserver with a regular one which will always subscribe in unit tests.

@gpeal gpeal closed this in #235 May 3, 2019
gpeal added a commit that referenced this issue May 3, 2019
Using MvRxTestRule to force not using life cycle aware observable for unit tests.
Fixes #198
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.