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

fixed zio-mock documentation on mocking collaborators compose layers #7600

Merged
merged 12 commits into from
Dec 6, 2022

Conversation

JoaquinIglesiasTurina
Copy link
Contributor

Updated zio-mock documentation as documented on this issue.

@CLAassistant
Copy link

CLAassistant commented Dec 3, 2022

CLA assistant check
All committers have signed the CLA.

@vigoo
Copy link
Contributor

vigoo commented Dec 3, 2022

Thank you!

Copy link
Contributor

@adamgfraser adamgfraser left a comment

Choose a reason for hiding this comment

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

Can we use mdoc here so we check that these code snippets compile as part of CI?

@@ -916,7 +927,6 @@ test("effectful expectation") {
Random
.nextUUID
.map(id => User(id.toString, s"name-$n"))
.provideLayer(Random.live)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please check this gist when reviewing this change.
https://gist.github.com/JoaquinIglesiasTurina/10d16eed4ab774171525673f3087d8e8

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -67,7 +67,12 @@ case class UserServiceLive(emailService: EmailService, userRepository: UserRepos

object UserServiceLive {
val layer: URLayer[EmailService with UserRepository, UserService] =
(UserServiceLive.apply _).toLayer[UserService]
ZLayer {
Copy link
Contributor

Choose a reason for hiding this comment

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

This can just be ZLayer.fromFunction.

new EmailService {
override def send(to: String, body: String): IO[String, Unit] =
proxy(Send, to, body)
ZLayer.fromZIO(
Copy link
Contributor

Choose a reason for hiding this comment

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

ZLayer.apply and use of a for comprehension would be more idiomatic here.

new UserRepository {
override def save(user: User): IO[String, Unit] =
proxy(Save, user)
ZLayer.fromZIO(
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above.

@@ -980,7 +996,7 @@ There are also `failureF` and `failureZIO` variants like what we described for `

This expectation simulates a never-ending loop:

```scala
```scala mdoc/silent
Copy link
Contributor

Choose a reason for hiding this comment

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

Typo.

Copy link
Contributor

@adamgfraser adamgfraser left a comment

Choose a reason for hiding this comment

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

Great to add mdoc to all of these to make sure they compile. I think this could use one more pass to make sure all the examples are as idiomatic as possible and then will be good to go!

@JoaquinIglesiasTurina
Copy link
Contributor Author

I've reviewed the ZLayer constructors and typos. Is there something else I should be mindful of?
Being new to ZIO and Scala, I cannot be proactive on making code idiomatic.
@adamgfraser, thank you for your review. I'm learning a lot.

ZLayer {
for {
proxy <- ZIO.service[Proxy]
} yield (
Copy link
Contributor

Choose a reason for hiding this comment

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

Very minor but this will be slightly more readable if you delete the parentheses here and define new EmailService on the same line. } yield new EmailService {

ZLayer {
for {
proxy <- ZIO.service[Proxy]
} yield (
new UserRepository {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above.

ZLayer(
for {
proxy <- ZIO.service[Proxy]
} yield {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can delete extra curly braces and move to same line.

import zio.test._
import MockPolyService._
Copy link
Contributor

Choose a reason for hiding this comment

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

Could keep the import and just do PolyInput.of[String]. This would probably be slightly cleaner.

def polyError[E: Tag](input: Int) = proxy(PolyError.of[E], input)
def polyOutput[A: Tag](input: Int) = proxy(PolyOutput.of[A], input)
def polyAll[I: Tag, E: Tag, A: Tag](input: I) = proxy(PolyAll.of[I, E, A], input)
ZLayer(
Copy link
Contributor

Choose a reason for hiding this comment

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

Curly braces instead of parentheses.


override def exampleStream(a: Int): stream.Stream[Throwable, String] =
rts.unsafeRun(proxy(ExampleStream, a))
ZLayer.fromZIO(
Copy link
Contributor

Choose a reason for hiding this comment

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

ZLayer.apply here.

rts.unsafeRun(proxy(ExampleStream, a))
ZLayer.fromZIO(
ZIO.serviceWithZIO[Proxy] { proxy =>
withRuntime[Proxy, ExampleService] { rts =>
Copy link
Contributor

Choose a reason for hiding this comment

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

runtime instead of rts.

proxy(ExampleEffect, i)

override def exampleMethod(i: Int): String =
Unsafe.unsafe { implicit u =>
Copy link
Contributor

Choose a reason for hiding this comment

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

unsafe instead of u.

ZLayer.fromZIO(
ZIO.serviceWithZIO[Proxy] { proxy =>
withRuntime[Proxy, ExampleService] { rts =>
ZIO.succeed(
Copy link
Contributor

Choose a reason for hiding this comment

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

Curly braces instead of parentheses for multiple line statement.

ZLayer {
for {
proxy <- ZIO.service[Proxy]
} yield
Copy link
Contributor

Choose a reason for hiding this comment

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

Can bring up definition of AccountObserver a line.

}
}.toLayer

ZLayer{
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing space.

ZLayer{
for {
proxy <- ZIO.service[mock.Proxy]
} yield
Copy link
Contributor

Choose a reason for hiding this comment

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

Delete extra line.

@adamgfraser
Copy link
Contributor

@JoaquinIglesiasTurina Definitely! Thank you for your work on this! Left detailed comments.

Copy link
Contributor

@adamgfraser adamgfraser left a comment

Choose a reason for hiding this comment

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

Congratulations on your first contribution to ZIO! 🎉

@adamgfraser adamgfraser merged commit 2863644 into zio:series/2.x Dec 6, 2022
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