Port Boopickle to cats effect 3 and add basic munit infrastructure #3852
Conversation
| } | ||
|
|
||
| checkAll("EntityCodec[IO, Fruit]", EntityCodecTests[IO, Fruit].entityCodec) | ||
| // checkAll("EntityCodec[IO, Fruit]", EntityCodecTests[IO, Fruit].entityCodec) |
There was a problem hiding this comment.
EntityCodecTests needs a rewrite to run on munit-cats-effect, if you think munit is fine I could try to do the rewrite
There was a problem hiding this comment.
Why does it need a rewrite? I think it's based just on Discipline?
I'm for what makes it work. I just haven't studied it enough to understand.
There was a problem hiding this comment.
The particular laws take an Eq[IO[A]] which we don't have right now
|
|
||
| /** Common stack for http4s' munit based tests | ||
| */ | ||
| trait Http4sSuite |
There was a problem hiding this comment.
This is equivalent to Http4sSpec for munit. It will need to grow as needed
build.sbt
Outdated
| .enablePlugins(PrivateProjectPlugin) | ||
| .settings( | ||
| // Root project | ||
| name := "http4s", | ||
| description := "A minimal, Scala-idiomatic library for HTTP", | ||
| description := "A minimal, Scala-idiomatic library for HTTP" |
There was a problem hiding this comment.
You don't like my trailing commas?
There was a problem hiding this comment.
I think they were removed by scalafmt; I can restore them
There was a problem hiding this comment.
I'm maintaining four branches right now, so I'm just a little bit frantic about unnecessary diffs right now, especially when they're on the furthest branch downhill.
| } | ||
|
|
||
| checkAll("EntityCodec[IO, Fruit]", EntityCodecTests[IO, Fruit].entityCodec) | ||
| // checkAll("EntityCodec[IO, Fruit]", EntityCodecTests[IO, Fruit].entityCodec) |
There was a problem hiding this comment.
Why does it need a rewrite? I think it's based just on Discipline?
I'm for what makes it work. I just haven't studied it enough to understand.
| test("decode a class from a boopickle decoder") { | ||
| val result = booOf[IO, Fruit] | ||
| .decode(Request[IO]().withEntity(Banana(10.0): Fruit), strict = true) | ||
| result.value.map(assertEquals(_, Right(Banana(10.0)))) |
There was a problem hiding this comment.
I love the removal of an unafeRunSync().
| with syntax.AllSyntax | ||
| with ArbitraryInstances |
There was a problem hiding this comment.
In Cats, it was discussed to not extend syntax traits or arbitraries in the base test suite, so as to make the tests look more like the kind of code it would be testing. We will have a few more imports in our tests without these two lines, but I think it might be better, and this is our chance at a fresh start. What do you think?
There was a problem hiding this comment.
I'll remove everything we don't need right away.
|
Also, if this works, we should consider backporting the munit infrastructure to |
| ), | ||
| unusedCompileDependenciesFilter -= moduleFilter(organization = "org.typelevel", name = "discipline-munit"), |
There was a problem hiding this comment.
I need this to pass CI but I'm not sure why, we are using classes from both libs
|
Maybe we can merge this with the law check commented while we figure out how to check effectful laws with discipline and munit |
|
I can do it, I'll rebase this and merge it |
cquiroz
force-pushed
the
boopickle
branch
3 times, most recently
from
0529ec9
to
cf4d0a6
Compare
Signed-off-by: Carlos Quiroz <carlos.m.quiroz@gmail.com>
|
Ok I squashed this and I will merge to move on to others, The doc task is not yet building |
Port Boopickle to cats effect 3 and add basic munit infrastructure
Port Boopickle to cats effect 3 and add basic munit infrastructure
This PR ports boopickle to run on CE3 but perhaps more important it lays the basics to use
munit/munit-cats-effect for tests.
There is still some work to do but I wanted to verify if the idea of munit seems workable