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

2.x: Test static from methods and add Maybe.fromSingle & fromCompletable #4685

Merged
merged 1 commit into from
Oct 11, 2016
Merged

2.x: Test static from methods and add Maybe.fromSingle & fromCompletable #4685

merged 1 commit into from
Oct 11, 2016

Conversation

vanniktech
Copy link
Collaborator

  • add Maybe.fromSingle by reusing MaybeFromSingle
  • add Maybe.fromCompletable by resuing MaybeFromCompletable
  • remove anonymous classes in various operators
  • add tests for
    • Completable.fromAction()
    • Completable.fromCallable()
    • Completable.fromObservable()
    • Completable.fromPublisher()
    • Completable.fromRunnable()
    • Completable.fromSingle()
    • Maybe.fromAction()
    • Maybe.fromCallable()
    • Maybe.fromCompletable()
    • Maybe.fromRunnable()
    • Maybe.fromSingle()
    • Single.fromCallable()

observable.subscribe(new CompletableFromObservableObserver<T>(s));
}

static class CompletableFromObservableObserver<T> implements Observer<T> {
Copy link
Member

Choose a reason for hiding this comment

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

final

Copy link
Member

Choose a reason for hiding this comment

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

sorry, didn't see the enclosing package.

public void onError(Throwable e) {
s.onError(e);
}
static class CompletableFromSingleObserver<T> implements SingleObserver<T> {
Copy link
Member

Choose a reason for hiding this comment

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

final

Completable.fromRunnable(new Runnable() {
@Override
public void run() {

Copy link
Member

Choose a reason for hiding this comment

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

I'd increment a counter and assert its 1.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can also test calling twice increments twice. And that it calls lazily (not on creation).

@codecov-io
Copy link

codecov-io commented Oct 9, 2016

Current coverage is 82.10% (diff: 100%)

Merging #4685 into 2.x will increase coverage by 0.06%

@@                2.x      #4685   diff @@
==========================================
  Files           565        565          
  Lines         37426      37436    +10   
  Methods           0          0          
  Messages          0          0          
  Branches       5746       5746          
==========================================
+ Hits          30702      30736    +34   
+ Misses         4644       4614    -30   
- Partials       2080       2086     +6   

Powered by Codecov. Last update e654e9b...aa3217c

* @throws NullPointerException if completable is null
*/
@SchedulerSupport(SchedulerSupport.NONE)
public static <T> Maybe<T> fromCompletable(final Completable completable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

final isn't needed. This should be CompletableSource otherwise it's redundant with toMaybe().

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I copied the signature from the method above and that one is using final. Not everything is consistent though. I'll change it though

* @throws NullPointerException if single is null
*/
@SchedulerSupport(SchedulerSupport.NONE)
public static <T> Maybe<T> fromSingle(final Single single) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto to both here

Completable.fromAction(new Action() {
@Override
public void run() throws Exception {

Copy link
Contributor

Choose a reason for hiding this comment

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

Assert with counter

Copy link
Contributor

Choose a reason for hiding this comment

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

And make sure second subscription increments again

Completable.fromCallable(new Callable<Object>() {
@Override
public Object call() throws Exception {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Assert with counter. You could return it's value and assert you only get 1 and that a subsequent call gets 2

Completable.fromRunnable(new Runnable() {
@Override
public void run() {

Copy link
Contributor

Choose a reason for hiding this comment

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

Can also test calling twice increments twice. And that it calls lazily (not on creation).

Maybe.fromAction(new Action() {
@Override
public void run() throws Exception {

Copy link
Contributor

Choose a reason for hiding this comment

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

Same

Completable.fromCallable(new Callable<Object>() {
@Override
public Object call() throws Exception {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same

Maybe.fromRunnable(new Runnable() {
@Override
public void run() {

Copy link
Contributor

Choose a reason for hiding this comment

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

Same

@akarnokd akarnokd added this to the 2.0 RC5 milestone Oct 10, 2016
@vanniktech
Copy link
Collaborator Author

Anything missing here? I updated this PR on Sunday with the requested changes.

@akarnokd akarnokd merged commit 497f35f into ReactiveX:2.x Oct 11, 2016
@vanniktech vanniktech deleted the test_coverage branch October 11, 2016 08:42
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.

4 participants