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

tests: remove unsafeRunSync usage #232

Merged

Conversation

ahjohannessen
Copy link
Contributor

@ahjohannessen ahjohannessen commented Sep 9, 2018

  • add AsyncFlatSpec from scalatest and introduce
    two variants of JSDomSpec.

  • refactor tests that rely on unsafeRunSync to
    use unsafeToFuture from scalatest. Furthermore,
    there is an implicit conversion from IO[Assertion]
    to Future[Assertion] in JSDomAsyncSpec that reduces
    usage of unsafeToFuture boilerplate.

@ahjohannessen ahjohannessen force-pushed the wip-remove-unsafe-run-sync-usage branch 4 times, most recently from ffca6dd to 54d4431 Compare September 17, 2018 11:44
@LukaJCB LukaJCB self-requested a review September 17, 2018 11:46
Copy link
Member

@LukaJCB LukaJCB left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks a lot! :)

trait EasySubscribe {

implicit class Subscriber[T](obs: Observable[T]) {
def apply(next: T => Unit)(implicit s: Scheduler): Cancelable = obs.subscribe { t =>
def apply(next: T Unit)(implicit s: Scheduler): Cancelable = obs.subscribe { t
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 prefer to stay with ascii arrows instead of unicode ones.

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'll update it to use ascii

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fdietze updated PR to use <- instead of and => instead of in tests

Copy link
Member

Choose a reason for hiding this comment

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

Thank you!

 - add `AsyncFlatSpec` from scalatest and introduce
   two variants of `JSDomSpec`.

 - refactor tests that rely on `unsafeRunSync` to
   use `unsafeToFuture` from scalatest. Furthermore,
   there is an implicit conversion from `IO[Assertion]`
   to `Future[Assertion]` in `JSDomAsyncSpec` that reduces
   usage of `unsafeToFuture` boilerplate.
@fdietze
Copy link
Member

fdietze commented Sep 18, 2018

LGTM! Thank you for your contribution!

@fdietze fdietze merged commit eeb7d8f into outwatch:master Sep 18, 2018
@ahjohannessen ahjohannessen deleted the wip-remove-unsafe-run-sync-usage branch September 18, 2018 11:22
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

3 participants