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

FlowableWithLatestFrom forgets request #5494

Merged
merged 4 commits into from Jul 18, 2017
Merged

Conversation

matgabriel
Copy link
Contributor

The following scenario did not work with FlowableWithLatestFrom:
Flowable<?> result = source.combineWithLatest(other, someCombiner)

  • client subscribe to the result stream
  • source stream emits
  • other stream emits
  • source stream emits

expected result: result stream emits the combined event
actual result: result stream does not emit anything

@akarnokd
Copy link
Member

Nice find and I'm a bit baffled as FlowableWithLatestFromMany does not have this flaw...

@@ -261,7 +264,10 @@ public void testNoDownstreamUnsubscribe() {

@Test
public void testBackpressure() {
Flowable<Integer> source = Flowable.range(1, 10);
TestScheduler testScheduler = new TestScheduler();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't change existing tests but add new ones instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I have added a test, but I still had to modify testBackpressure because it was relying on the fact that the source would not get more request if the other stream does not emit data.

@akarnokd akarnokd added this to the 2.2 milestone Jul 17, 2017
@codecov
Copy link

codecov bot commented Jul 17, 2017

Codecov Report

Merging #5494 into 2.x will decrease coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #5494      +/-   ##
============================================
- Coverage     96.11%   96.03%   -0.08%     
+ Complexity     5798     5797       -1     
============================================
  Files           631      631              
  Lines         41297    41299       +2     
  Branches       5743     5743              
============================================
- Hits          39692    39663      -29     
- Misses          632      654      +22     
- Partials        973      982       +9
Impacted Files Coverage Δ Complexity Δ
...nal/operators/flowable/FlowableWithLatestFrom.java 100% <100%> (ø) 2 <0> (ø) ⬇️
...in/java/io/reactivex/subjects/BehaviorSubject.java 84.89% <0%> (-6.78%) 56% <0%> (ø)
...reactivex/internal/operators/maybe/MaybeUsing.java 93.93% <0%> (-6.07%) 4% <0%> (ø)
.../operators/observable/ObservableFlatMapSingle.java 88.8% <0%> (-5.98%) 2% <0%> (ø)
...ernal/operators/flowable/FlowableFlatMapMaybe.java 90.33% <0%> (-5.8%) 2% <0%> (ø)
...vex/internal/operators/single/SingleTakeUntil.java 86.88% <0%> (-4.92%) 2% <0%> (ø)
...nternal/operators/parallel/ParallelReduceFull.java 91.17% <0%> (-3.93%) 2% <0%> (ø)
...tivex/internal/observers/FutureSingleObserver.java 94.33% <0%> (-3.78%) 24% <0%> (-1%)
...internal/operators/completable/CompletableAmb.java 94.91% <0%> (-3.39%) 10% <0%> (-1%)
...ex/internal/operators/maybe/MaybeTimeoutMaybe.java 95.58% <0%> (-2.95%) 2% <0%> (ø)
... and 42 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cd62d62...5d871b7. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants