Skip to content

Conversation

@akarnokd
Copy link
Member

This fixes observeOn not completing immediately if the downstream requested exactly the remaining amount. The original version required a new request from downstream to trigger the delivery of the terminal event.

This also fixes the rare failure in testNoMoreRequestsAfterUnsubscribe because the unsubscription is now checked in-sequence and doesn't trigger the unwanted replenishment request.

}

@Test
public void requestExactCompletesImmediately() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if it tests "immediately", this test passes without changes in this PR.

Maybe you can avoid going to another thread and awaitTerminalEvent()?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, should request 0 upfront and wait till the range completes before requesting 10. I'll change this to use the TestScheduler.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated the test which now clearly fails on master.

@artem-zinnatullin
Copy link
Contributor

👍 for changes, not sure about test

@akarnokd akarnokd force-pushed the ObserveOnRequestBoundaryCompletionFix branch from 672fa8e to e98b20d Compare March 15, 2016 10:17
@artem-zinnatullin
Copy link
Contributor

👍

1 similar comment
@stevegury
Copy link
Member

👍

akarnokd added a commit that referenced this pull request Mar 16, 2016
…tionFix

1.x: observeOn - fix in-sequence termination/unsubscription
@akarnokd akarnokd merged commit f7321d2 into ReactiveX:1.x Mar 16, 2016
@akarnokd akarnokd deleted the ObserveOnRequestBoundaryCompletionFix branch March 16, 2016 13:50
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.

3 participants