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

Provide Observable.timestamp(Scheduler) to be used in the tests. #751

Merged
merged 1 commit into from
Feb 14, 2014

Conversation

vigdorchik
Copy link
Contributor

The test could've used toBlockingObservable, but this hangs, even though timestamped observable is completed.

@cloudbees-pull-request-builder

RxJava-pull-requests #665 SUCCESS
This pull request looks good

@vigdorchik
Copy link
Contributor Author

@samuelgruetter @headinthebox please review. I'd also change 2 overloads to the one

Observable.timestamp(s: Option[Scheduler] = None)

but I'm not sure it's ok to break binary compatibility.

@cloudbees-pull-request-builder

RxJava-pull-requests #671 SUCCESS
This pull request looks good

@samuelgruetter
Copy link
Contributor

LGTM, thanks for contributing!

Regarding

The test could've used toBlockingObservable, but this hangs, even though timestamped observable is completed.

That's not surprising: You can't use toBlockingObservable and TestScheduler in a single threaded test, because you would have to call advanceTimeTo while you're blocking on a method of BlockingObservable.

And regarding

Observable.timestamp(s: Option[Scheduler] = None)

Don't do this, we want to keep it the same way in all operators. But I agree that the way overloads with schedulers are done currently is not good, Scala offers much better ways with implicits. We're still working on this, and will change all operators at once.

@benjchristensen
Copy link
Member

From the Scala folks ... should this be updated and merged considered the changes that have happened in the past month?

@headinthebox @samuelgruetter ?

@samuelgruetter
Copy link
Contributor

This can be merged, but once we decide on #815 , it will change again.

@vigdorchik
Copy link
Contributor Author

I would appreciate a way to have a timestamped stream unit-tested. If this is not with Scheduler parameter, I'm fine.

@benjchristensen benjchristensen merged commit 7c1cbf1 into ReactiveX:master Feb 14, 2014
jihoonson pushed a commit to jihoonson/RxJava that referenced this pull request Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants