-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Fix UseCaseTest #139
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
Fix UseCaseTest #139
Conversation
amatkivskiy
commented
Apr 12, 2016
@jordifierro same thing with testSubscriptionWhenExecutingUseCase
test case.
Thanks @amatkivskiy, you are right!
amatkivskiy
commented
Apr 12, 2016
@jordifierro and I think we need to rethink the way the UseCase is tested.
What do you mean @amatkivskiy?
amatkivskiy
commented
Apr 12, 2016
@jordifierro I meant that your approach with DefaultThreadExecutor
made UseCaseTest
work correctly. Previously the tests were fake passing. For example you can comment line 63 in previous version of the UseCaseTest
and nothing changes.
I am telling this because while implementing #140 I need to write tests for it.
Sorry @amatkivskiy, I asked because I thought you wanted to change the way of testing UseCase
(not just fix the tests). As you say, tests were always passing and I just wanted to fix that here, not try to modify or improve them.
And about your pull request, feel free to use the same approach used here if you don't want to wait for this pull request, no problem for me!
@jordifierro Recently I've faced the problem that TestScheduler
executes the onNext
of the subscriber NOT in the thread that test run in.
To reproduce this scenario replace simple Observable.just(1, 2, 3)
with some delayed Observable.delay()
. As the result TestSubscriber
wont wait for the observable to finish.
The way to hold this scenario one needs to add testSubscriber.awaitTerminalEvent();
before asserting the subscriber but this leads the current thread to be blocked. I tried to use Schedulers.immediate()
instead of TestScheduler
.
Thanks for commenting @amatkivskiy!
I've faced a similar problem when testing my repositories against a MockWebServer
(I've written about it here) and, as you say, I've solved it adding testSubscriber.awaitTerminalEvent();
before the asserts to wait for the mocked server response.
However, in my opinion, the goal of this test is to check that the subscriber
receives whatever from wherever is emitted through the data stream after buildUseCaseObservable()
method from the UseCase
child class is called, so I see no point in testing deeply here.
amatkivskiy
commented
Apr 27, 2016
@jordifierro my point is that people will see this tests and will try to apply the same approach in their tests and it will fail because it works only for this situation (I mean testing base UseCase when result is return immediately).
Considering the nature of this specific project, you are right @amatkivskiy. I will work on this.
amatkivskiy
commented
Apr 27, 2016
@jordifierro Thanks)
After all, the update hasn't made the test more complex nor larger (which I didn't want) so it's fine. The Schedulers.immediate()
you told me did the trick.
@amatkivskiy Thanks to you for your help!
The
testBuildUseCaseObservableReturnCorrectResult
test fromUseCaseTest
is not testing correctly the execute method from theUseCase
class. It's not a crucial test but it's a good example of how to test RxJava subscriptions.