Skip to content

bugfix: Keep coverage data in the class directory on sbt 2 - #697

Open
phdoerfler wants to merge 1 commit into
scoverage:mainfrom
phdoerfler:sbt2-coverage-data-in-class-dir
Open

phdoerfler wants to merge 1 commit into
scoverage:mainfrom
phdoerfler:sbt2-coverage-data-in-class-dir

Conversation

@phdoerfler

Copy link
Copy Markdown

Keep coverage data in the class directory on sbt 2

Fixes #696.

On sbt 2, compile is cached. A cache hit restores only the declared outputs, i.e. the class directory. The scoverage-data directory under crossTarget (scoverage.coverage plus the directory the instrumented classes write measurements into) is a side effect of compilation, so it is not restored. After clean, a fresh checkout, or on a CI agent with a warm cache:

  • forked tests fail with FileNotFoundException on scoverage.measurements.*, or
  • coverageReport logs No coverage data, skipping reports and skips the minimum check.

This changes the sbt 2 default of coverageDataDir to Compile / classDirectory, the workaround from #696. The sbt 1 default stays crossTarget. The switch lives in ScoverageSbtPluginCompat, next to the existing Def.uncached.

Trade-offs:

  • On sbt 2, reports now go to <classes>/scoverage-report and <classes>/coverage-report.
  • A jar packaged in the same coverage-enabled invocation contains scoverage-data. A normal compile rewrites the class directory, so regular packaging is unaffected.
  • Users who set coverageDataDir themselves keep their value. The README notes it should stay inside the class directory on sbt 2.

An alternative would be to move only scoverage-data into the class directory and keep reports under crossTarget. That needs a separate path for data vs. reports, including in coverageAggregate. I decided against that since both variants have their downsides and the one presented in this PR is at least a very minimal change.

The largest change is the addition of a new scripted test. cached-compile runs clean; coverage; testOnly; coverageReport twice with Test / fork := true and a 100% minimum. On sbt 2 (sbt "++ 3.x" "scripted scoverage/cached-compile"), it fails without the change (FileNotFoundException in round 2) and passes with it. It also passes on sbt 1. Note that CI currently runs scripted on sbt 1 only.

fwiw: with Test / fork := false, round 2 reports 0% (noted in #696). That looks like a separate in-JVM Invoker issue (#522).

@tgodzik tgodzik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! Looks like the tests are failing when Scala 3.8.x is used on JDK 8. Not sure if there is a way to run specific test only on sbt 2, so maybe we can just run it on 2.13 (we are not really testing Scala versions itself here)

@@ -0,0 +1,16 @@
version := "0.1"
scalaVersion := "3.8.4"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we can't run it on JDK 8, but since we are actually testing sbt 2, we can just use Scala 2.13 here.

Suggested change
scalaVersion := "3.8.4"
scalaVersion := "2.13.18"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, we can check the java version and switch between Scala versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sbt 2: compile served from the local cache leaves instrumented classes without scoverage-data (FileNotFoundException in tests)

2 participants