Remove mocking from tests; use http4s directly #669
Conversation
This is needed for scala3
Codecov Report
@@ Coverage Diff @@
## main #669 +/- ##
==========================================
+ Coverage 90.54% 94.84% +4.30%
==========================================
Files 25 25
Lines 645 815 +170
Branches 1 2 +1
==========================================
+ Hits 584 773 +189
+ Misses 61 42 -19
Continue to review full report at Codecov.
|
|
|
||
| import scala.reflect.ClassTag | ||
|
|
||
| class EncoderDecoderSpec extends AnyFlatSpec with ScalaCheckPropertyChecks { |
There was a problem hiding this comment.
Why is this test in the scala-2 directory? You've included ArbitraryDerivation for both Scala 2 or 3 - is there some other dependency that's missing for Scala 3?
There was a problem hiding this comment.
scalacheck-shapeless doesn't exist for scala3, since on scala3, shapeless isn't out.
I put the spec in scala2 because I ran into what I think might be a compiler bug, preventing me from using the (otherwise working) scala-3 ArbitraryDerivation.
| implicit val decoderCommentData: Decoder[CommentData] = deriveDecoder[CommentData] | ||
| implicit val decoderPullRequestReviewEvent: Decoder[PullRequestReviewEvent] = | ||
| Decoder[String].emap { | ||
| case s if s == PRREventApprove.value => Right(PRREventApprove) |
There was a problem hiding this comment.
| case s if s == PRREventApprove.value => Right(PRREventApprove) | |
| case PRREventApprove.value => Right(PRREventApprove) |
wouldn't work?
There was a problem hiding this comment.
It would work. I think this was existing code that got shuffled around, and I left it as-is
|
|
||
| object auto { | ||
| given deriveArb[A](using gen: DerivedGen[A]): Arbitrary[A] = gen.arb | ||
| } |
There was a problem hiding this comment.
should we harmonize brackets / no brackets?
There was a problem hiding this comment.
Can you elaborate on what you mean by that? Do you mean using a set of empty parens? Typically you wouldn't do that.
There was a problem hiding this comment.
I think Ben is referring to the inconsistent use of significant whitespace vs braces in this file. e.g. object semiauto: vs object auto { ... }
There was a problem hiding this comment.
Ah gotcha, that makes sense.
I don't have a strong opinion on it. The indentation-based parts were mostly what I copied from some folks helping me out in discord. I don't have a sense for what is best to move forward with.
Honestly, I hope this file gets deleted before it matters. The behavior shouldn't be defined here at all, but rather upstream
| val scalamock: String = "5.1.0" | ||
| val scalatest: String = "3.2.9" | ||
| val silencer: String = "1.7.1" | ||
| val bm4 = "0.3.1" |
There was a problem hiding this comment.
Yes, it's better-monadic-for. I forget where exactly, but it's compile-time-only, so it doesn't hurt to have in the project. It just makes things work better regardless
|
didn't check the whole pr |
This is needed for scala3
This PR was extracted from #667
This adds explicit circe encoders and decoders; no more auto-derivation.
This is needed for the new tests, which have to be able to round trip responses through http entities, rather than mocking at the object level.