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

Marble tests for mergeMap and unsubscription marbles for TestScheduler #423

Closed
wants to merge 3 commits into from

Conversation

staltz
Copy link
Member

@staltz staltz commented Sep 30, 2015

This PR resolves #402 and #374.
All tests from RxJS 4 are present in this PR, except for one test: see #422.

Andre Medeiros added 3 commits September 30, 2015 13:58
Replicates most tests from RxJS 4 flatMap.
Using the new unsubscription marble diagram feature in expectObservable
to cover more unsubscribe-related tests from RxJS 4.
@staltz staltz changed the title Test marbles for mergeMap and unsubscription marbles for TestScheduler Marble tests for mergeMap and unsubscription marbles for TestScheduler Sep 30, 2015
}
destination.next(innerValue);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This above wasn't just a refactor, I recall solving one of the tests failing by including this else, because otherwise there would be two calls to destination.next().

@benlesh
Copy link
Member

benlesh commented Sep 30, 2015

Overall this looks good.

One thing that is interesting, is it doesn't cover the expectations of unsubscription does it? It only covers the unsubscribe.

I say that's interesting, because originally my concern was the inability to examine when an inner observable was unsubscribed from, in say, switch or the like. So I guess we still have an unresolved issue.

@benlesh
Copy link
Member

benlesh commented Sep 30, 2015

I'm going to merge this one and create a new issue.

@staltz
Copy link
Member Author

staltz commented Sep 30, 2015

Yeah that crossed my mind. Gotta think about it.

@benlesh
Copy link
Member

benlesh commented Sep 30, 2015

merged with 4d973a3

@benlesh benlesh closed this Sep 30, 2015
@staltz staltz deleted the test-marbles-mergemap branch October 1, 2015 13:28
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unsubscribe from marble tests
3 participants