-
Notifications
You must be signed in to change notification settings - Fork 162
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
Derive zio.test.Gen #161
Derive zio.test.Gen #161
Conversation
@thinkharderdev Hey Dan! Could you have a look at this PR?! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! One small comment but I think this is ready to merge otherwise.
private def genSequence[Z, A](seq: Schema.Sequence[Z, A]): Gen[Random with Sized, Z] = | ||
Gen.chunkOf1(gen(seq.schemaA)).map(nec => seq.fromChunk(nec.toChunk)) | ||
|
||
private def genMap[K, V](map: Schema.MapSchema[K, V]): Gen[Random with Sized, Map[K, V]] = | ||
Gen.mapOf1(gen(map.ks), gen(map.vs)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For these two I would not restrict to single-element chunks (at the very least we should sometimes generate an empty Chunk). So maybe a chunk bounded by 0 and 2? I know in other places we run into severe performance issues if we don't restrict to small(ish) collection sized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I was concerned about chunk's arbitrary size and that's why I did single-element chunks).
No problem, I will bound the chunk by 0 and 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
Co-authored-by: Dan Harris <1327726+thinkharderdev@users.noreply.github.com>
My second attempt on resolving #56.
I've renewed this pr due to recent changes in zio-schema