Support Scoverage for Scala 3 #2016
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ckipp01
commented
Sep 3, 2022
| class ScoverageData(ctx0: mill.define.Ctx) extends Module()(ctx0) with ScalaModule { | ||
|
|
||
| /** Coverage in the Scala 3 compilier is only supported in the 3.2.x series and above. */ | ||
| private def isSupportedScala: Task[Boolean] = T.task { |
There was a problem hiding this comment.
One thing I'm having a hard time with is figuring out where to put this check. At first I thought it'd be better to put it up above and in every task that computes something I could use like
private def ensureValidScala3[A](value: A): Task[A] = T.task {
val scalaVersion = outer.scalaVersion()
scalaVersion.split('.') match {
case Array(major, minor, patch) if major == "3" && minor.toIntOption.exists(_ >= 2) => value
case Array(major, minor, patch) if major == "3" =>
val msg =
s"Detected Scala version: ${scalaVersion}. However, to use Coverage with Scala 3 you must be at least on Scala 3.2.0."
Result.Failure(msg)
case _ => value
}
}So then I could just do like:
def scoverageRuntimeDeps: T[Agg[Dep]] = T {
if (isScala3()) {
ensureValidScala3(Agg.empty)()
} else {
Agg(ivy"org.scoverage::scalac-scoverage-runtime:${outer.scoverageVersion()}")
}
}But that fell apart when I hit scoveragteToolsClasspath and couldn't get it to work.
Then I moved this down below into ScoverageData and I thought about adding an assert in here to check for an invalid Scala 3, but can't really access the scalaVersion in the assert. I'm not really sure where to do this check. Any input on where you'd think this would be best and how to do it?
There was a problem hiding this comment.
Moreover this should also check that if this is a valid Scala 3 version then the user must also be using scoverage 2.x. It'd be nice to do this check in one place.
There was a problem hiding this comment.
I think this check is best placed in scoverageToolsClasspath. It has access to all required data and is run before any worker action. Also, as there can be no valid classpath created in case we have an incompatible Scala version or an inappropriate scoverage version, this is the best place for this validation.
| @@ -212,19 +251,26 @@ object HelloWorldTests_2_12 extends HelloWorldTests { | |||
| override def threadCount = Some(1) | |||
| override def testScalaVersion: String = sys.props.getOrElse("MILL_SCALA_2_12_VERSION", ???) | |||
| override def testScoverageVersion = sys.props.getOrElse("MILL_SCOVERAGE_VERSION", ???) | |||
| override def testScalatestVersion = "3.0.8" | |||
| override def testScalatestVersion = "3.2.13" | |||
There was a problem hiding this comment.
I ended up having to bump all these because 3.0.8 doesn't support Scala 3.
|
@ckipp01 Do you mind when I update your PR with some changes? As I needed to check it out for a proper review, I already started to think by changing some code. |
Added more tests.
lefou
changed the title
Hey! Of course, I'm on holiday at the moment so I won't be looking closely at it for another couple weeks probably. |
|
Don't know why the Scala 3.2 test fails. Maybe, something changed in Scala itself? |
|
I still have no idea why the htmlReport test is failing with Scala 3.2. The issue is, that a path for the test setup is not correct. This should be no issue in normal usage though, so I tend to just disable this test for now and merge anyway. If anybody has an idea, please help. |
I don't know why this fails, but htmlReport seems to work in normal usage.
lefou
force-pushed
the
coverageScala3
branch
from
e367bf6 to
ad13acb
Compare
|
@lefou, in my project, I'm getting the My project build is on: https://github.com/carlosedp/riscvassembler/blob/main/build.sc Log: |
|
@carlosedp This is interesting, because the file |
|
Could be, that scoverage detected some inlined macro code and wants to create the Source files listing as HTML. |
That could be the case. I've gotten lots of report of this in scoverage and this is reported in Dotty as well scala/scala3#15791. |
|
On the plus side, this could mean, scoverage is finally recognizing code in macros, which was not the case until now. I use scoverage in most of my Mill plugin projects, and coverage for Mill tasks/targets isn't reported correctly. But "inline" isn't "macro". |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow-up to the work done in #2010 to also add support for Scala 3.