diff --git a/scala/private/phases/phase_coverage.bzl b/scala/private/phases/phase_coverage.bzl index 6c008d67f..f076b5ad4 100644 --- a/scala/private/phases/phase_coverage.bzl +++ b/scala/private/phases/phase_coverage.bzl @@ -36,25 +36,26 @@ def _phase_coverage(ctx, p, srcjars): output_jar = ctx.actions.declare_file( "{}-offline.jar".format(input_jar.basename.split(".")[0]), ) - in_out_pairs = [ - (input_jar, output_jar), + srcs_paths = [src.path for src in ctx.files.srcs] + records = [ + (input_jar, output_jar, srcs_paths), ] args = ctx.actions.args() - args.add_all(in_out_pairs, map_each = _jacoco_offline_instrument_format_each) + args.add_all(records, map_each = _jacoco_offline_instrument_format_each) args.set_param_file_format("multiline") args.use_param_file("@%s", use_always = True) ctx.actions.run( mnemonic = "JacocoInstrumenter", - inputs = [in_out_pair[0] for in_out_pair in in_out_pairs], - outputs = [in_out_pair[1] for in_out_pair in in_out_pairs], + inputs = [record[0] for record in records], + outputs = [record[1] for record in records], executable = ctx.attr._code_coverage_instrumentation_worker.files_to_run, execution_requirements = {"supports-workers": "1"}, arguments = [args], ) - replacements = {i: o for (i, o) in in_out_pairs} + replacements = {i: o for (i, o, _) in records} provider = _coverage_replacements_provider.create( replacements = replacements, ) @@ -72,5 +73,5 @@ def _phase_coverage(ctx, p, srcjars): }, ) -def _jacoco_offline_instrument_format_each(in_out_pair): - return (["%s=%s" % (in_out_pair[0].path, in_out_pair[1].path)]) +def _jacoco_offline_instrument_format_each(records): + return (["%s=%s=%s" % (records[0].path, records[1].path, ",".join(records[2]))]) diff --git a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java index 86c450b63..d553d7cf5 100644 --- a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java +++ b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java @@ -52,11 +52,12 @@ public void processRequest(List < String > args) { private void processArg(Instrumenter jacoco, String arg) throws Exception { String[] parts = arg.split("="); - if (parts.length != 2) { - throw new Exception("expected `in_path=out_path` form for argument: " + arg); + if (parts.length != 3) { + throw new Exception("expected `in_path=out_path=srcs` form for argument: " + arg); } Path inPath = Paths.get(parts[0]); Path outPath = Paths.get(parts[1]); + String srcs = parts[2]; try ( FileSystem inFS = FileSystems.newFileSystem(inPath, null); FileSystem outFS = FileSystems.newFileSystem( URI.create("jar:" + outPath.toUri()), Collections.singletonMap("create", "true")); @@ -69,6 +70,21 @@ private void processArg(Instrumenter jacoco, String arg) throws Exception { throw new RuntimeException(e); } }); + + /* + * https://github.com/bazelbuild/bazel/blob/567ca633d016572f5760bfd027c10616f2b8c2e4/src/java_tools/junitrunner/java/com/google/testing/coverage/JacocoCoverageRunner.java#L411 + * + * Bazel / JacocoCoverageRunner will look for any file that ends with '-paths-for-coverage.txt' within the JAR to be later used for reconstructing the path for source files. + * This is a fairly undocumented feature within bazel at this time, but in essence, it opens all the jars, searches for all files matching '-paths-for-coverage.txt' + * and then adds them to a single in memory set. + * + * https://github.com/bazelbuild/bazel/blob/567ca633d016572f5760bfd027c10616f2b8c2e4/src/java_tools/junitrunner/java/com/google/testing/coverage/JacocoLCOVFormatter.java#L70 + * Which is then used in the formatter to find the corresponding source file from the set of sources we wrote in all the JARs. + */ + Files.write( + outFS.getPath("-paths-for-coverage.txt"), + srcs.replace(",", "\n").getBytes(java.nio.charset.StandardCharsets.UTF_8) + ); } } diff --git a/test/coverage/A1.scala b/test/coverage/A1.scala index 2dcc34f43..be90760e4 100644 --- a/test/coverage/A1.scala +++ b/test/coverage/A1.scala @@ -1,3 +1,5 @@ +package coverage; + object A1 { def a1(flag: Boolean): B1.type = if (flag) B1 diff --git a/test/coverage/A2.scala b/test/coverage/A2.scala index 0e58f455d..ff3cce15c 100644 --- a/test/coverage/A2.scala +++ b/test/coverage/A2.scala @@ -1,7 +1,8 @@ +package coverage; + object A2 { def a2(): Unit = { - println("a2: " + - "" // B2.b2_a() + println("a2: " + B2.b2_a() ) } } diff --git a/test/coverage/B1.scala b/test/coverage/B1.scala index 3b5a9f305..ee8f12665 100644 --- a/test/coverage/B1.scala +++ b/test/coverage/B1.scala @@ -1,3 +1,5 @@ +package coverage; + object B1 { def not_called(): Unit = { diff --git a/test/coverage/B2.java b/test/coverage/B2.java index 30b97c543..629bb28d0 100644 --- a/test/coverage/B2.java +++ b/test/coverage/B2.java @@ -1,3 +1,5 @@ +package coverage; + class B2 { public static String b2_a() { return C2.c2("hello from b2_a"); diff --git a/test/coverage/BUILD b/test/coverage/BUILD index 66bb48a8e..483d98313 100644 --- a/test/coverage/BUILD +++ b/test/coverage/BUILD @@ -62,20 +62,10 @@ scala_library( "A2.scala", ], deps = [ - # TODO :: Understand why referencing a local java library breaks coverage - # ":b2", + ":b2", ], ) -# -# As it stands I can't seem to generate coverage for Java libraries pulled into -# a scala_test target. -# -# The java_library is instrumented, but doesn't have the .uninstrumented files -# that Bazel seems to expect. There are a few code paths for code coverage, so -# down the road we can explore how to fix this... -# - java_library( name = "b2", srcs = [ diff --git a/test/coverage/C2.scala b/test/coverage/C2.scala index 7e405b1cf..40daebda1 100644 --- a/test/coverage/C2.scala +++ b/test/coverage/C2.scala @@ -1,3 +1,5 @@ +package coverage; + object C2 { def c2(input: String): String = input.reverse diff --git a/test/coverage/TestAll.scala b/test/coverage/TestAll.scala index cbe8f9e4e..aae034261 100644 --- a/test/coverage/TestAll.scala +++ b/test/coverage/TestAll.scala @@ -1,3 +1,4 @@ +package coverage; import org.scalatest._ class TestAll extends FlatSpec { diff --git a/test/coverage/TestB2.java b/test/coverage/TestB2.java index ea5275e64..bffa8898f 100644 --- a/test/coverage/TestB2.java +++ b/test/coverage/TestB2.java @@ -1,3 +1,5 @@ +package coverage; + import org.junit.Test; import org.junit.Assert.*; diff --git a/test/coverage/expected-coverage.dat b/test/coverage/expected-coverage.dat index d142c915f..d92769254 100755 --- a/test/coverage/expected-coverage.dat +++ b/test/coverage/expected-coverage.dat @@ -1,78 +1,110 @@ -SF:/A1.scala -FN:-1,A1$:: ()V -FN:5,A1$:: ()V -FN:3,A1$::a1 (Z)LB1$; -FN:-1,A1::a1 (Z)LB1$; -FNDA:1,A1$:: ()V -FNDA:1,A1$:: ()V -FNDA:1,A1$::a1 (Z)LB1$; -FNDA:0,A1::a1 (Z)LB1$; +SF:test/coverage/A1.scala +FN:-1,coverage/A1$:: ()V +FN:7,coverage/A1$:: ()V +FN:5,coverage/A1$::a1 (Z)Lcoverage/B1$; +FN:-1,coverage/A1::a1 (Z)Lcoverage/B1$; +FNDA:1,coverage/A1$:: ()V +FNDA:1,coverage/A1$:: ()V +FNDA:1,coverage/A1$::a1 (Z)Lcoverage/B1$; +FNDA:0,coverage/A1::a1 (Z)Lcoverage/B1$; FNF:4 FNH:3 -BA:3,2 +BA:5,2 BRF:1 BRH:1 -DA:3,4 -DA:4,0 -DA:5,5 +DA:5,4 +DA:6,0 +DA:7,5 LH:2 LF:3 end_of_record -SF:/A2.scala -FN:-1,A2$:: ()V -FN:7,A2$:: ()V -FN:3,A2$::a2 ()V -FN:-1,A2::a2 ()V -FNDA:1,A2$:: ()V -FNDA:1,A2$:: ()V -FNDA:1,A2$::a2 ()V -FNDA:0,A2::a2 ()V +SF:test/coverage/A2.scala +FN:-1,coverage/A2$:: ()V +FN:8,coverage/A2$:: ()V +FN:5,coverage/A2$::a2 ()V +FN:-1,coverage/A2::a2 ()V +FNDA:1,coverage/A2$:: ()V +FNDA:1,coverage/A2$:: ()V +FNDA:1,coverage/A2$::a2 ()V +FNDA:0,coverage/A2::a2 ()V FNF:4 FNH:3 -DA:3,4 -DA:7,5 +DA:5,11 +DA:8,5 LH:2 LF:2 end_of_record -SF:/B1.scala -FN:-1,B1$:: ()V -FN:7,B1$:: ()V -FN:4,B1$::not_called ()V -FN:-1,B1::not_called ()V -FNDA:1,B1$:: ()V -FNDA:1,B1$:: ()V -FNDA:0,B1$::not_called ()V -FNDA:0,B1::not_called ()V +SF:test/coverage/B1.scala +FN:-1,coverage/B1$:: ()V +FN:9,coverage/B1$:: ()V +FN:6,coverage/B1$::not_called ()V +FN:-1,coverage/B1::not_called ()V +FNDA:1,coverage/B1$:: ()V +FNDA:1,coverage/B1$:: ()V +FNDA:0,coverage/B1$::not_called ()V +FNDA:0,coverage/B1::not_called ()V FNF:4 FNH:2 -DA:4,0 -DA:7,5 +DA:6,0 +DA:9,5 +LH:1 +LF:2 +end_of_record +SF:test/coverage/B2.java +FN:3,coverage/B2:: ()V +FN:5,coverage/B2::b2_a ()Ljava/lang/String; +FN:9,coverage/B2::b2_b ()V +FNDA:0,coverage/B2:: ()V +FNDA:1,coverage/B2::b2_a ()Ljava/lang/String; +FNDA:0,coverage/B2::b2_b ()V +FNF:3 +FNH:1 +DA:3,0 +DA:5,3 +DA:9,0 +DA:10,0 LH:1 +LF:4 +end_of_record +SF:test/coverage/C2.scala +FN:-1,coverage/C2$:: ()V +FN:7,coverage/C2$:: ()V +FN:5,coverage/C2$::c2 (Ljava/lang/String;)Ljava/lang/String; +FN:-1,coverage/C2::c2 (Ljava/lang/String;)Ljava/lang/String; +FNDA:1,coverage/C2$:: ()V +FNDA:1,coverage/C2$:: ()V +FNDA:1,coverage/C2$::c2 (Ljava/lang/String;)Ljava/lang/String; +FNDA:1,coverage/C2::c2 (Ljava/lang/String;)Ljava/lang/String; +FNF:4 +FNH:4 +DA:5,9 +DA:7,5 +LH:2 LF:2 end_of_record -SF:/TestAll.scala -FN:10,TestAll$$anonfun$1:: (LTestAll;)V -FN:10,TestAll$$anonfun$1::apply ()V -FN:10,TestAll$$anonfun$1::apply$mcV$sp ()V -FN:6,TestAll$$anonfun$2:: (LTestAll;)V -FN:6,TestAll$$anonfun$2::apply ()Lorg/scalatest/compatible/Assertion; -FN:3,TestAll:: ()V -FNDA:1,TestAll$$anonfun$1:: (LTestAll;)V -FNDA:1,TestAll$$anonfun$1::apply ()V -FNDA:1,TestAll$$anonfun$1::apply$mcV$sp ()V -FNDA:1,TestAll$$anonfun$2:: (LTestAll;)V -FNDA:1,TestAll$$anonfun$2::apply ()Lorg/scalatest/compatible/Assertion; -FNDA:1,TestAll:: ()V +SF:test/coverage/TestAll.scala +FN:11,coverage/TestAll$$anonfun$1:: (Lcoverage/TestAll;)V +FN:11,coverage/TestAll$$anonfun$1::apply ()V +FN:11,coverage/TestAll$$anonfun$1::apply$mcV$sp ()V +FN:7,coverage/TestAll$$anonfun$2:: (Lcoverage/TestAll;)V +FN:7,coverage/TestAll$$anonfun$2::apply ()Lorg/scalatest/compatible/Assertion; +FN:4,coverage/TestAll:: ()V +FNDA:1,coverage/TestAll$$anonfun$1:: (Lcoverage/TestAll;)V +FNDA:1,coverage/TestAll$$anonfun$1::apply ()V +FNDA:1,coverage/TestAll$$anonfun$1::apply$mcV$sp ()V +FNDA:1,coverage/TestAll$$anonfun$2:: (Lcoverage/TestAll;)V +FNDA:1,coverage/TestAll$$anonfun$2::apply ()Lorg/scalatest/compatible/Assertion; +FNDA:1,coverage/TestAll:: ()V FNF:6 FNH:6 -BA:6,2 +BA:7,2 BRF:1 BRH:1 -DA:3,2 -DA:5,22 -DA:6,51 -DA:9,23 -DA:10,13 +DA:4,2 +DA:6,22 +DA:7,51 +DA:10,23 +DA:11,13 LH:5 LF:5 end_of_record