Cross-build for Scala.js #272
Conversation
|
This looks reasonable to me, thanks! My only concern is about further ScalaTest lock-in, since we're slowly trying to migrate all of these projects to MUnit, but I can live with that. |
| private implicit def assertionToProp: IO[Assertion] => PropF[IO] = { assertion => | ||
| assertion.as(PropF.Result[IO](Prop.True, Nil, Set.empty, Set.empty): PropF[IO]).handleError { | ||
| case _: DiscardedEvaluationException => PropF.Result[IO](Prop.Undecided, Nil, Set.empty, Set.empty) | ||
| case t => PropF.Result[IO](Prop.Exception(t), Nil, Set.empty, Set.empty) | ||
| } | ||
| } |
|
If the goal is to move to |
|
@armanbilge My vote would be to merge this if it's ready and then migrate. As long as there's a clear route to MUnit and this work is already done, I think there's value in recording it in the history. |
|
Ok, works for me! I'll fix it up for merge. |
Co-authored-by: Ben Fradet <benjamin.fradet@gmail.com>
|
@travisbrown I'm unclear what to do about the CI failure, I don't think it's related to this PR. |
|
@armanbilge I don't think that last commit (moving the Scoverage config to JVM-only) is what you want, since it won't disable Scoverage on Scala.js, where it's causing problems. I don't know why switching to cross-project would mess up these settings. You might look at the other failures (for Scala 2), which seem likely to be more obvious. |
This reverts commit ed43440.
|
I can't reproduce any of these failures locally :( |
|
I see them locally with the same commands CI is using, but haven't looked in detail. |
|
Thanks, I'll give that a try, |
Codecov Report
@@ Coverage Diff @@
## master #272 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 25 25
=========================================
Hits 25 25
Continue to review full report at Codecov.
|
build.sbt
Outdated
| @@ -133,7 +138,7 @@ ThisBuild / githubWorkflowJavaVersions := Seq("adopt@1.8") | |||
| ThisBuild / githubWorkflowPublishTargetBranches := Seq.empty | |||
| ThisBuild / githubWorkflowBuild := Seq( | |||
| WorkflowStep.Sbt( | |||
| List("clean", "coverage", "test", "coverageReport", "scalafmtCheckAll"), | |||
| List("clean", "coverage", "fs2JVM / test", "fs2JS / test", "fs2JVM / coverageReport", "scalafmtCheckAll"), | |||
There was a problem hiding this comment.
Looking at e.g. the circe-optics build, this seems to be done via .jsSettings(coverageEnabled := false), which seems better, since it applies for people running the default commands locally, and not just in CI.
There was a problem hiding this comment.
For whatever reason that doesn't seem to work here.
There was a problem hiding this comment.
Ugh. I get why you're doing this, but this kind of thing is why I hate Scala.js so much.
There was a problem hiding this comment.
I think the problems we are experiencing here are confounded by the fact this is a single-module project.
There was a problem hiding this comment.
@travisbrown I think moving the project into its own folder and having a designated root fixes it.
|
This looks good to me, thanks! |
|
@travisbrown @BenFradet Anything I can do to get this merged? :) thanks! |
|
Works for me, thanks! |
|
I just did a complex merge to keep this PR fresh. Is there any chance of it getting merged? It's okay if the answer is no, just trying to get an idea of where this stands :) thanks! |
|
Thanks, I'll merge when green this time. |
The actual code was a pure cross, but the tests required significant re-working to avoid the use of
unsafeRunSync(not available on JS).Fixes #159.