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.

Sign up for GitHub

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

Jump to bottom

2.x: Add assertValuesOnly to BaseTestConsumer. #5568

Merged
merged 2 commits into from
Aug 25, 2017
Merged

2.x: Add assertValuesOnly to BaseTestConsumer. #5568

merged 2 commits into from
Aug 25, 2017

Conversation

vanniktech
Copy link
Collaborator

Adds assertValuesOnly that asserts that the TestObserver/TestSubscriber received only the specified values in the specified order without terminating.

Fixes #5555

@codecov
Copy link

codecov bot commented Aug 24, 2017 •
edited
Loading

Codecov Report

Merging #5568 into 2.x will decrease coverage by 0.03%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##                2.x   #5568      +/-   ##
===========================================
- Coverage     96.23%   96.2%   -0.04%     
- Complexity     5819    5824       +5     
===========================================
  Files           631     631              
  Lines         41417   41421       +4     
  Branches       5739    5739              
===========================================
- Hits          39859   39849      -10     
- Misses          618     623       +5     
- Partials        940     949       +9
Impacted Files Coverage Δ Complexity Δ
.../java/io/reactivex/observers/BaseTestConsumer.java 99.65% <100%> (ø) 113 <1> (+1) ⬆️
.../operators/observable/ObservableFlatMapSingle.java 90.29% <0%> (-5.23%) 2% <0%> (ø)
...nternal/operators/parallel/ParallelReduceFull.java 91.17% <0%> (-3.93%) 2% <0%> (ø)
...ernal/operators/flowable/FlowableFlatMapMaybe.java 94.2% <0%> (-2.9%) 2% <0%> (ø)
...l/operators/observable/ObservableFlatMapMaybe.java 89.54% <0%> (-1.97%) 2% <0%> (ø)
...ternal/operators/flowable/FlowableSubscribeOn.java 96.61% <0%> (-1.7%) 2% <0%> (ø)
...a/io/reactivex/internal/util/QueueDrainHelper.java 58.86% <0%> (-1.42%) 31% <0%> (-1%)
...nternal/operators/observable/ObservableCreate.java 97.43% <0%> (-0.86%) 2% <0%> (ø)
.../operators/observable/ObservableSequenceEqual.java 98.31% <0%> (-0.85%) 2% <0%> (ø)
...nternal/operators/parallel/ParallelSortedJoin.java 94.92% <0%> (-0.73%) 2% <0%> (ø)
... and 16 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 ea7ca2c...dc261cd. Read the comment docs.

artem-zinnatullin
artem-zinnatullin reviewed Aug 24, 2017
View reviewed changes
src/test/java/io/reactivex/observers/TestObserverTest.java Outdated
@@ -1439,4 +1439,47 @@ public void withTag() {
assertTrue(ex.toString(), ex.toString().contains("testing with item=2"));
}
}

@Test
public void assertValuesOnlyWantsOnlyValues() {
Copy link
Contributor

Choose a reason for hiding this comment

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

wants?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could just completely rename it to assertValuesOnly

Copy link
Contributor

Choose a reason for hiding this comment

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

sgtm

src/test/java/io/reactivex/observers/TestObserverTest.java
} catch (AssertionError ex) {
// expected
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Not super important, but if you have time let's add test that verifies that method fails if expected value was emitted :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You mean was not emitted right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant "unexpected", sorry :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@akarnokd akarnokd added this to the 2.2 milestone Aug 24, 2017
akarnokd
akarnokd reviewed Aug 24, 2017
View reviewed changes
src/main/java/io/reactivex/observers/BaseTestConsumer.java Outdated
* @param values the values expected
* @return this;
*/
@SuppressWarnings("unchecked")
Copy link
Member

Choose a reason for hiding this comment

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

@Experimental & @since 2.1.4 - experimental missing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right. Forgot that.

artem-zinnatullin
artem-zinnatullin approved these changes Aug 25, 2017
View reviewed changes
Copy link
Contributor

@artem-zinnatullin artem-zinnatullin left a comment

Choose a reason for hiding this comment

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

👍

@akarnokd akarnokd merged commit 994c65d into ReactiveX:2.x Aug 25, 2017
@vanniktech vanniktech deleted the assertValuesOnly branch August 25, 2017 09:56
@akarnokd akarnokd mentioned this pull request Sep 18, 2017
Closed
@jared-bot jared-bot mentioned this pull request Sep 27, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artem-zinnatullin artem-zinnatullin artem-zinnatullin approved these changes

@akarnokd akarnokd akarnokd approved these changes

Assignees
No one assigned
Labels
2.x Enhancement
Projects
None yet
Milestone
2.2
Development

Successfully merging this pull request may close these issues.
3 participants
@vanniktech @artem-zinnatullin @akarnokd