-
Notifications
You must be signed in to change notification settings - Fork 32
Conversation
component.addListener(event, listener); | ||
} | ||
|
||
private class EmitterListener<T> implements Listener<T>, Cancellable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will generate synthetic accessor methods should be just class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call.
RxNavi.observe(emitter, Event.CREATE_PERSISTABLE).subscribe(testSubscriber); | ||
testSubscriber.assertNoValues(); | ||
TestObserver<BundleBundle> testObserver = new TestObserver<>(); | ||
RxNavi.observe(emitter, Event.CREATE_PERSISTABLE).subscribe(testObserver); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could use RxNavi.observe(emitter, Event.CREATE_PERSISTABLE).test()
instead which will give you a TestObserver
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funny, I was just reading about this and deciding whether I wanted to switch or not. I'm not coming up with any compelling reason to go through the effort, since all the code is already there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a few more minutes of pondering I've decided it's worth switching, if for no other reason than that it's more succinct. That'll make future tests easier to write.
@@ -13,15 +13,15 @@ | |||
import io.reactivex.observers.TestObserver; | |||
import org.junit.Test; | |||
|
|||
import static com.trello.navi2.rx.RxNavi.observe; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this done on purpose? Since this is the only test with a static import on navi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah good catch, no, that must've been a slip on the auto-complete when I was doing stuff.
No description provided.