Skip to content

Add TimeUnit null check test case in Timed #5231

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
Merged
merged 8 commits into from
Mar 26, 2017
Merged

Add TimeUnit null check test case in Timed #5231

merged 8 commits into from
Mar 26, 2017

Conversation

ggikko
Copy link
Contributor

@ggikko ggikko commented Mar 26, 2017 •
edited
Loading

  1. It's about Timed.java test case that is null check TimeUnit parameter.
  2. formatting in BasicIntQueueDisposable
  3. add testcase blockingIterableNegativeBufferSizeFail for blockingIterable
  4. modify BlockingMultiObserver field’s modfier

@codecov
Copy link

codecov bot commented Mar 26, 2017 •
edited
Loading

Codecov Report

Merging #5231 into 2.x will increase coverage by 0.07%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #5231      +/-   ##
============================================
+ Coverage     95.99%   96.06%   +0.07%     
- Complexity     5750     5753       +3     
============================================
  Files           628      628              
  Lines         41074    41074              
  Branches       5697     5697              
============================================
+ Hits          39428    39459      +31     
+ Misses          654      649       -5     
+ Partials        992      966      -26
Impacted Files Coverage Δ Complexity Δ
...al/operators/observable/ObservableSampleTimed.java 88.33% <0%> (-8.34%) 3% <0%> (ø)
...a/io/reactivex/processors/SerializedProcessor.java 91.48% <0%> (-6.39%) 27% <0%> (-1%)
...reactivex/internal/operators/single/SingleAmb.java 96.36% <0%> (-3.64%) 9% <0%> (-1%)
...activex/internal/operators/single/SingleCache.java 97.05% <0%> (-2.95%) 23% <0%> (-1%)
...rnal/operators/flowable/FlowableTakeLastTimed.java 96.29% <0%> (-2.78%) 2% <0%> (ø)
...rnal/operators/flowable/FlowableFlatMapSingle.java 95.05% <0%> (-2.2%) 2% <0%> (ø)
...ternal/operators/observable/ObservablePublish.java 91.22% <0%> (-1.76%) 9% <0%> (ø)
...ternal/operators/flowable/FlowableSubscribeOn.java 96.61% <0%> (-1.7%) 2% <0%> (ø)
...internal/operators/observable/ObservableCache.java 93.7% <0%> (-1.58%) 9% <0%> (ø)
...ivex/internal/operators/maybe/MaybeMergeArray.java 96.62% <0%> (-1.13%) 6% <0%> (ø)
... and 38 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 7748fd5...7436014. Read the comment docs.

JakeWharton
JakeWharton reviewed Mar 26, 2017
View reviewed changes
src/main/java/io/reactivex/internal/observers/BasicIntQueueDisposable.java Outdated
implements QueueDisposable<T> {

public abstract class BasicIntQueueDisposable<T> extends AtomicInteger
implements QueueDisposable<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert style changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I changed :)

src/main/java/io/reactivex/internal/observers/BlockingMultiObserver.java Outdated

Disposable d;
private Disposable d;
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert visibility changes. These are intentional

akarnokd
akarnokd reviewed Mar 26, 2017
View reviewed changes
Copy link
Member

@akarnokd akarnokd left a comment

Choose a reason for hiding this comment

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

Some adjustments are still needed, otherwise 👍

src/test/java/io/reactivex/internal/operators/observable/ObservableBlockingTest.java Outdated
@@ -209,6 +209,11 @@ public void blockingLastEmpty() {
Observable.empty().blockingLast();
}

@Test(expected = IllegalArgumentException.class)
public void blockingIterableNegativeBufferSizeFail() {
Copy link
Member

Choose a reason for hiding this comment

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

This is covered by the ParamValidationCheckerTest class which scans all operators for parameter validation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh.. yeap I got it. thank you 👍

src/test/java/io/reactivex/schedulers/TimedTest.java Outdated
@@ -84,4 +84,10 @@ public void toStringOf() {

assertEquals("Timed[time=5, unit=SECONDS, value=1]", t1.toString());
}

@SuppressWarnings({"unchecked", "rawtypes"})
Copy link
Member

Choose a reason for hiding this comment

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

There is no need to suppress this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeap I got it. thank you 👍 I removed this annotation.

src/test/java/io/reactivex/schedulers/TimedTest.java Outdated
@SuppressWarnings({"unchecked", "rawtypes"})
@Test(expected = NullPointerException.class)
public void timeUnitNullFail() throws Exception {
Timed<Integer> t1 = new Timed<Integer>(1, 5, null);
Copy link
Member

Choose a reason for hiding this comment

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

t1 shows up as unused variable, please remove the assignment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree and removed useless variable.

@akarnokd akarnokd merged commit 851de41 into ReactiveX:2.x Mar 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JakeWharton JakeWharton JakeWharton left review comments

@akarnokd akarnokd akarnokd left review comments

Assignees
No one assigned
Labels
2.x Cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ggikko @JakeWharton @akarnokd