Add an Arbitrary[Random[F]] instance #364
Conversation
…o ScalaCheck's seed
| def genRandom[F[_]: Sync]: Gen[Random[F]] = | ||
| Gen.long | ||
| .map(new scala.util.Random(_).pure[F]) | ||
| .map(new ScalaRandom[F](_) {}) |
There was a problem hiding this comment.
To be honest, it looks a bit "hacky" to me: ArbitraryRandom is placed into cats.effect.std to let it access the ScalaRandom's private constructor. Whereas the other scalacheck-effect code lives in org.scalacheck.effect.
There was a problem hiding this comment.
Yes, that's true, but I did ask about this implementation in Discord and got an initial thumbs up from @djspiewak. (Although to be fair, I’m not sure he was thinking about it from this specific angle.) I suppose if Cats Effect could be persuaded to open up one of those constructors, we could move this somewhere else.
Really the problem is that the Random.scalaUtilRandomSeedLong returns F[Random[F]] and not just Random[F]. I don’t think that’s necessary for this particular case, but I’m not sure whether that’s true in general. If introducing a new constructor in Random is a better option, then we could use that here instead.
There was a problem hiding this comment.
In regards to constructors safety, there are a couple of classes in Cats-Effect that got "unsafe" constructor counterparts: Ref and Deferred. Both are in "kernel" though. However, I don't think that it would hurt much if Random could get a couple of unsafe constructors too. At least to me it seems like a more viable solution comparing to tuning package names in order to access private constructors.
For example, Random.javaUtilRandom[F[_]](random: java.util.Random): F[Random[F]] is not fully referentially transparent, apparently. So, it wouldn't seem too bad if there was unsafeJavaUtilRandom[F[_]](random: java.util.Random): Random[F] counterpart.
|
Perhaps, it would make sense to add generators/arbitraries for Personally I would appreciate such an addition, because it can come in handy for many non-effect driven property-based tests as well. Then, having those arbitraries, obtaining a |
|
Oh, that's an interesting idea! I'm not at a computer at the moment but I'll play with that when I get a chance and see how it works. |
This will link the randomness used by the code under test to ScalaCheck's seed, which should improve test repeatability.
I didn't see a good place to introduce this instance; previously users would have to use both
scalacheck-effect-munitandmunit-cats-effectto runIO-based property tests. This introduces a new artifact that depends on both ScalaCheck and Cats Effect in order to make the instance available.