1.x: support Observable.test() #4777

davidmoten · 2016-10-28T04:04:12Z

The new method chaining TestSubscriber in 2.x is too good not to have in 1.x as well (while I wait for 2.x to mature and to make my existing unit tests less verbose).

Observable
  .just(1)
  .test()
  .assertValue(1)
  .assertCompleted();

Observable.test() returns a TestSubscriber2 that wraps a TestSubscriber and enables method chaining where it can.

I've added tests to get to 100% test coverage though I've been pretty slap-dash about testing some of the details (for example I haven't tested that awaitXXX really does wait for stuff).

I've noted it as @since 1.2.3 but can update to 1.2.2 if you are happy to squeak it into the release.

codecov-io · 2016-10-28T04:13:16Z

Current coverage is 84.08% (diff: 100%)

Merging #4777 into 1.x will decrease coverage by 0.08%

@@                1.x      #4777   diff @@
==========================================
  Files           286        287     +1   
  Lines         17754      17828    +74   
  Methods           0          0          
  Messages          0          0          
  Branches       2702       2702          
==========================================
+ Hits          14943      14990    +47   
- Misses         1952       1967    +15   
- Partials        859        871    +12

Powered by Codecov. Last update 6385051...ef91729

akarnokd · 2016-10-28T16:56:44Z

I have mixed feelings about this. 2.x required massive amount of unit tests, often with the same structure across the base types where this was a valuable addition. Maybe you could flesh it out in rxjava-extras and provide a function for the to operator:

import static TestConsumer.*;

Observable.just(1)
.to(newTest(0L))
.assertEmpty()
.requestMore(1)
.assertResult(1);

You can base it on Subscriber and implement CompletableSubscriber but I wish we introduced a base interface for SingleSubscriber before going 1.2.

MyDogTom · 2016-10-28T20:10:10Z

Just for reference. There are at least two libraries that aim to provide similar functionality (chaining assertions):

RxTestWrapper - just wraps TestSubsriber in a similar manner as this PR
rxassertions - provides additional assertions, but also introduce some limitations.

davidmoten · 2016-10-28T22:24:01Z

@akarnokd, @MyDogTom
I have been using this functionality in rxjava-extras using .to(TestingHelper.test()) for about five weeks. It just seems too useful to miss out on natively (don't have to add extra library to pom.xml, add import, put up with slightly less readable line in code). Discoverability is sorted for new users too.

For anyone writing tests for their observables I think this is very attractive functionality to have natively and I was stoked to use it with rxjava 2.

Note that my motivation for this is for writing tests in my projects (not rxjava 1.x core, though that would be nice too).

I forgot to consider Completable and Single because I don't use them (the majority of our rxjava based code predates their existence). Do they need special treatment (other than addition of a test method there)?

akarnokd · 2016-10-29T15:14:34Z

How about having an interface with the get and assert methods and the 3 test()s return it. In an internal package then you can implement them and have delegates as necessary.

davidmoten · 2016-11-01T22:10:15Z

How about having an interface with the get and assert methods and the 3 test()s return it. In an internal package then you can implement them and have delegates as necessary.

Yep I can do that.

davidmoten · 2016-11-02T01:40:41Z

How about having an interface with the get and assert methods and the 3 test()s return it. In an internal package then you can implement them and have delegates as necessary.

Having extracted an interface the one implementation can serve all test() methods by subscribing to Completable, Single or Observable instances using subscribe(Subscriber). How do you want the implementations to differ?

akarnokd · 2016-11-02T07:30:19Z

src/main/java/rx/Completable.java

@@ -16,19 +16,48 @@

 package rx;

-import java.util.*;
-import java.util.concurrent.*;
+import java.util.Arrays;


Please make sure you don't unroll * imports.

akarnokd · 2016-11-02T07:33:27Z

src/main/java/rx/observers/TestSubscriber2.java

-        ts.assertValue(value);
-        return this;
-    }
+public interface TestSubscriber2<T> extends Observer<T> {


I'd call this TestConsumer because that 2 at the end will make people wonder and question endlessly.

akarnokd · 2016-11-02T07:33:57Z

src/main/java/rx/observers/TestSubscriber2.java


-}
+    boolean isUnsubscribed();


Reinforce with implements Subscription?

MyDogTom · 2016-11-02T08:54:28Z

I think test() should be added also for BlockingObservable.

akarnokd · 2016-11-02T08:56:38Z

Unnecessary. TestConsumer should have awaitTerminalEvent that blocks.

davidmoten · 2016-11-04T20:28:56Z

TestConsumer maybe not ideal because sounds like it's a significantly different beast from a TestSubscriber. I prefer to retain the word Subscriber in the name, what about TestSubscriberFluent?

akarnokd · 2016-11-04T20:35:16Z

AssertableSubscriber

MyDogTom · 2016-11-06T20:28:57Z

+1 for TestSubscriberFluent because it reflects the main difference from TestSubscriber - fluent API
-1 for AssertableSubscriber it doesn't provide any additional info (TestSubscriber is also "assertable").

@akarnokd I still think that BlockingObservable should have test() method. Let's assume that I encounter some legacy code that use BlockingObservable (regardless it's not recommended). I would like to be able write test first and refactor it afterwards (get rid of BlockingObservable).

davidmoten · 2016-11-07T09:45:43Z

@MyDogTom puts it well I think, after all AssertableSubscriber would provoke as many "what's the difference?" questions as TestSubscriber2. TestSubscriberFluent describes exactly what the class is.

davidmoten · 2016-11-07T09:47:20Z

fixed imports, renamed TestSubscriber2 to TestSubscriberFluent (though I'm not assuming the naming discussion has finished).

squashed commits and rebased on latest 1.x

akarnokd · 2016-11-07T10:01:54Z

AssertableSubscriber should be an interface with all the relevant assert methods that any actual internal test class can implement on top of the respective receiver type.

I think, after all AssertableSubscriber would provoke as many "what's the difference?"

If this comes up we will answer that it is the base API interface for asserting a subscriber's state for all 3 reactive types.

I see confusion going the other way around: why TestSubscriber2 or TestSubscriberFluent; its prefix TestSubscriber matches, well, TestSubscriber itself.

davidmoten · 2016-11-08T18:42:06Z

Righto, let's go with AssertableSubscriber. Modified code and squashed commits

akarnokd · 2016-11-08T20:48:32Z

src/main/java/rx/Completable.java

+     * @since 1.2.3
+     */
+    @Experimental
+    public final AssertableSubscriber<Void> test(long initialRequestAmount) {


Completable doesn't need backpressure so this method has no use.

akarnokd · 2016-11-08T20:48:55Z

src/main/java/rx/Single.java

+     * @since 1.2.3
+     */
+    @Experimental
+    public final AssertableSubscriber<T> test(long initialRequestAmount) {


Single doesn't support backpressure either.

…er and support Observable.test()

davidmoten · 2016-11-09T02:47:48Z

Thanks @akarnokd, made fixes and squashed commits.

akarnokd added the Enhancement label Oct 28, 2016

akarnokd added the Feature-Request label Oct 28, 2016

akarnokd requested changes Nov 2, 2016

View reviewed changes

davidmoten force-pushed the chained-test-subscriber branch from e47b1ef to a789ebd Compare November 7, 2016 09:45

davidmoten force-pushed the chained-test-subscriber branch from 00c2467 to 9bed046 Compare November 8, 2016 18:41

akarnokd requested changes Nov 8, 2016

View reviewed changes

davidmoten force-pushed the chained-test-subscriber branch from 9bed046 to 19bbbfa Compare November 9, 2016 02:42

add TestSubscriber2 to provide method chained version of TestSubscrib…

ef91729

…er and support Observable.test()

davidmoten force-pushed the chained-test-subscriber branch from 19bbbfa to ef91729 Compare November 9, 2016 02:46

akarnokd added this to the 1.3 milestone Nov 9, 2016

akarnokd approved these changes Nov 9, 2016

View reviewed changes

akarnokd merged commit d6bc923 into ReactiveX:1.x Nov 9, 2016

akarnokd mentioned this pull request Nov 18, 2016

1.2.3 release preparations #4866

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

1.x: support Observable.test() #4777

1.x: support Observable.test() #4777

davidmoten commented Oct 28, 2016 •

edited

Loading

codecov-io commented Oct 28, 2016 •

edited

Loading

akarnokd commented Oct 28, 2016

MyDogTom commented Oct 28, 2016 •

edited

Loading

davidmoten commented Oct 28, 2016 •

edited

Loading

akarnokd commented Oct 29, 2016

davidmoten commented Nov 1, 2016

davidmoten commented Nov 2, 2016

akarnokd Nov 2, 2016

akarnokd Nov 2, 2016

akarnokd Nov 2, 2016

MyDogTom commented Nov 2, 2016

akarnokd commented Nov 2, 2016

davidmoten commented Nov 4, 2016

akarnokd commented Nov 4, 2016

MyDogTom commented Nov 6, 2016

davidmoten commented Nov 7, 2016

davidmoten commented Nov 7, 2016

akarnokd commented Nov 7, 2016

davidmoten commented Nov 8, 2016

akarnokd Nov 8, 2016

akarnokd Nov 8, 2016

davidmoten commented Nov 9, 2016

1.x: support Observable.test() #4777

1.x: support Observable.test() #4777

Conversation

davidmoten commented Oct 28, 2016 • edited Loading

codecov-io commented Oct 28, 2016 • edited Loading

Current coverage is 84.08% (diff: 100%)

akarnokd commented Oct 28, 2016

MyDogTom commented Oct 28, 2016 • edited Loading

davidmoten commented Oct 28, 2016 • edited Loading

akarnokd commented Oct 29, 2016

davidmoten commented Nov 1, 2016

davidmoten commented Nov 2, 2016

akarnokd Nov 2, 2016

Choose a reason for hiding this comment

akarnokd Nov 2, 2016

Choose a reason for hiding this comment

akarnokd Nov 2, 2016

Choose a reason for hiding this comment

MyDogTom commented Nov 2, 2016

akarnokd commented Nov 2, 2016

davidmoten commented Nov 4, 2016

akarnokd commented Nov 4, 2016

MyDogTom commented Nov 6, 2016

davidmoten commented Nov 7, 2016

davidmoten commented Nov 7, 2016

akarnokd commented Nov 7, 2016

davidmoten commented Nov 8, 2016

akarnokd Nov 8, 2016

Choose a reason for hiding this comment

akarnokd Nov 8, 2016

Choose a reason for hiding this comment

davidmoten commented Nov 9, 2016

davidmoten commented Oct 28, 2016 •

edited

Loading

codecov-io commented Oct 28, 2016 •

edited

Loading

MyDogTom commented Oct 28, 2016 •

edited

Loading

davidmoten commented Oct 28, 2016 •

edited

Loading