2.x: Improve coverage, fix operator logic 03/12 #5910
Conversation
| @@ -278,7 +278,7 @@ public int size() { | |||
| * @param e the {@link Throwable} {@code e}. | |||
| * @return The root cause of {@code e}. If {@code e.getCause()} returns {@code null} or {@code e}, just return {@code e} itself. | |||
| */ | |||
| private Throwable getRootCause(Throwable e) { | |||
| /*private */Throwable getRootCause(Throwable e) { | |||
There was a problem hiding this comment.
That cause == root case is borderline impossible, unless this method gets called with this.
| b = buffer; | ||
| if (b == null) { | ||
| return; | ||
| if (DisposableHelper.replace(other, bs)) { |
There was a problem hiding this comment.
Use dedicated helper instead of a CAS, plus the old code could unset a terminal indicator.
| @@ -324,9 +320,10 @@ public void request(long n) { | |||
|
|
|||
| @Override | |||
| public void cancel() { | |||
| clear(); | |||
| cancelled = true; | |||
There was a problem hiding this comment.
cancelled is checked at various places but was never actually set.
| @@ -58,7 +58,7 @@ protected void subscribeActual(Subscriber<? super T> s) { | |||
|
|
|||
| Subscription s; | |||
|
|
|||
| final SequentialDisposable timer = new SequentialDisposable(); | |||
| Disposable timer; | |||
There was a problem hiding this comment.
Timer is only manipulated from onNext, onError or onComplete and doesn't have to be atomically safe. The operator disposes the entire worker to stop any outstanding timer activity anyway.
| b = buffer; | ||
| if (b == null) { | ||
| return; | ||
| if (DisposableHelper.replace(other, bs)) { |
There was a problem hiding this comment.
Use the helper method instead of a CAS that could replace a terminal indicator.
| @@ -51,7 +51,7 @@ public void subscribeActual(Observer<? super T> t) { | |||
|
|
|||
| Disposable s; | |||
|
|
|||
| final AtomicReference<Disposable> timer = new AtomicReference<Disposable>(); | |||
| Disposable timer; | |||
There was a problem hiding this comment.
timer is only manipulated from onNext, onError and onComplete.
| @@ -85,12 +85,9 @@ public void onSubscribe(Disposable d) { | |||
| public void onSuccess(T value) { | |||
| other.dispose(); | |||
|
|
|||
| Disposable a = get(); | |||
| Disposable a = getAndSet(DisposableHelper.DISPOSED); | |||
There was a problem hiding this comment.
Should be called at most once anyway, so biasing towards the a != DisposableHelper.DISPOSED being true.
| @@ -58,8 +58,7 @@ public MergerBiFunction(Comparator<? super T> comparator) { | |||
| while (at.hasNext()) { | |||
| both.add(at.next()); | |||
| } | |||
| } else | |||
| if (s2 != null) { | |||
| } else { | |||
There was a problem hiding this comment.
At this point either s1 or s2 can be null.
|
Done with the explanations. |
Codecov Report
@@ Coverage Diff @@
## 2.x #5910 +/- ##
============================================
+ Coverage 98.03% 98.27% +0.23%
- Complexity 5992 6002 +10
============================================
Files 655 655
Lines 43928 43927 -1
Branches 6086 6084 -2
============================================
+ Hits 43067 43169 +102
+ Misses 258 223 -35
+ Partials 603 535 -68
Continue to review full report at Codecov.
|
Improve coverage of RxJava internal components. See the change explanation as comments below.