From 555bd09ed71d40926f3bff7b1626be07a3209038 Mon Sep 17 00:00:00 2001 From: David Haxton Date: Sat, 22 Feb 2020 22:29:24 -0800 Subject: [PATCH 01/12] Fix source pathing for code coverage --- scala/private/phases/phase_coverage.bzl | 9 +- .../instrumenter/JacocoInstrumenter.java | 20 ++- test/coverage/A1.scala | 2 + test/coverage/A2.scala | 5 +- test/coverage/B1.scala | 2 + test/coverage/B2.java | 2 + test/coverage/BUILD | 12 +- test/coverage/C2.scala | 2 + test/coverage/TestAll.scala | 1 + test/coverage/TestB2.java | 2 + test/coverage/expected-coverage.dat | 140 +++++++++++------- 11 files changed, 121 insertions(+), 76 deletions(-) diff --git a/scala/private/phases/phase_coverage.bzl b/scala/private/phases/phase_coverage.bzl index 6c008d67f..ec8305c9d 100644 --- a/scala/private/phases/phase_coverage.bzl +++ b/scala/private/phases/phase_coverage.bzl @@ -36,8 +36,11 @@ def _phase_coverage(ctx, p, srcjars): output_jar = ctx.actions.declare_file( "{}-offline.jar".format(input_jar.basename.split(".")[0]), ) + srcs_paths = [] + for src in ctx.files.srcs: + srcs_paths = srcs_paths + [src.path] in_out_pairs = [ - (input_jar, output_jar), + (input_jar, output_jar, srcs_paths), ] args = ctx.actions.args() @@ -54,7 +57,7 @@ def _phase_coverage(ctx, p, srcjars): arguments = [args], ) - replacements = {i: o for (i, o) in in_out_pairs} + replacements = {i: o for (i, o, _) in in_out_pairs} provider = _coverage_replacements_provider.create( replacements = replacements, ) @@ -73,4 +76,4 @@ 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)]) + return (["%s=%s=%s" % (in_out_pair[0].path, in_out_pair[1].path, ",".join(in_out_pair[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..4f472fb94 100644 --- a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java +++ b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java @@ -52,16 +52,17 @@ 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")); ) { - FileVisitor fileVisitor = createInstrumenterVisitor(jacoco, outFS); + FileVisitor fileVisitor = createInstrumenterVisitor(jacoco, outFS, srcs); inFS.getRootDirectories().forEach(root -> { try { Files.walkFileTree(root, fileVisitor); @@ -72,18 +73,18 @@ private void processArg(Instrumenter jacoco, String arg) throws Exception { } } - private SimpleFileVisitor createInstrumenterVisitor(Instrumenter jacoco, FileSystem outFS) { + private SimpleFileVisitor createInstrumenterVisitor(Instrumenter jacoco, FileSystem outFS, String srcs) { return new SimpleFileVisitor () { @Override public FileVisitResult visitFile(Path inPath, BasicFileAttributes attrs) { try { - return actuallyVisitFile(inPath, attrs); + return actuallyVisitFile(inPath, attrs, srcs); } catch (final Exception e) { throw new RuntimeException(e); } } - private FileVisitResult actuallyVisitFile(Path inPath, BasicFileAttributes attrs) throws Exception { + private FileVisitResult actuallyVisitFile(Path inPath, BasicFileAttributes attrs, String srcs) throws Exception { Path outPath = outFS.getPath(inPath.toString()); Files.createDirectories(outPath.getParent()); if (inPath.toString().endsWith(".class")) { @@ -98,6 +99,13 @@ private FileVisitResult actuallyVisitFile(Path inPath, BasicFileAttributes attrs } else { Files.copy(inPath, outPath); } + + if(outPath.toString().endsWith(".class")) { + Files.write( + outFS.getPath((outPath.toString() + "-paths-for-coverage.txt")), + srcs.replace(",", "\n").getBytes(java.nio.charset.StandardCharsets.UTF_8) + ); + } return FileVisitResult.CONTINUE; } }; 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 From 4eae66835df4fa30ca13d6b3dc60ed3a222cffd3 Mon Sep 17 00:00:00 2001 From: David Haxton Date: Sat, 22 Feb 2020 23:16:08 -0800 Subject: [PATCH 02/12] lint fixes hopefully? --- scala/private/phases/phase_coverage.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scala/private/phases/phase_coverage.bzl b/scala/private/phases/phase_coverage.bzl index ec8305c9d..050fa0f89 100644 --- a/scala/private/phases/phase_coverage.bzl +++ b/scala/private/phases/phase_coverage.bzl @@ -36,9 +36,11 @@ def _phase_coverage(ctx, p, srcjars): output_jar = ctx.actions.declare_file( "{}-offline.jar".format(input_jar.basename.split(".")[0]), ) + srcs_paths = [] - for src in ctx.files.srcs: + for src in ctx.files.srcs: srcs_paths = srcs_paths + [src.path] + in_out_pairs = [ (input_jar, output_jar, srcs_paths), ] From 77f2181cb7b6b3e9ae2fa7c68c57384bf183ebc6 Mon Sep 17 00:00:00 2001 From: David Haxton Date: Sat, 22 Feb 2020 23:53:11 -0800 Subject: [PATCH 03/12] lint? idk what's going on here --- scala/private/phases/phase_coverage.bzl | 2 -- 1 file changed, 2 deletions(-) diff --git a/scala/private/phases/phase_coverage.bzl b/scala/private/phases/phase_coverage.bzl index 050fa0f89..9d83e961c 100644 --- a/scala/private/phases/phase_coverage.bzl +++ b/scala/private/phases/phase_coverage.bzl @@ -36,11 +36,9 @@ def _phase_coverage(ctx, p, srcjars): output_jar = ctx.actions.declare_file( "{}-offline.jar".format(input_jar.basename.split(".")[0]), ) - srcs_paths = [] for src in ctx.files.srcs: srcs_paths = srcs_paths + [src.path] - in_out_pairs = [ (input_jar, output_jar, srcs_paths), ] From 9e2e2a268a12a8364ecf290423c25973e7201f86 Mon Sep 17 00:00:00 2001 From: David Haxton Date: Mon, 24 Feb 2020 11:35:37 -0800 Subject: [PATCH 04/12] Rename in_out_pair to records --- scala/private/phases/phase_coverage.bzl | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/scala/private/phases/phase_coverage.bzl b/scala/private/phases/phase_coverage.bzl index 9d83e961c..5294c20a0 100644 --- a/scala/private/phases/phase_coverage.bzl +++ b/scala/private/phases/phase_coverage.bzl @@ -38,26 +38,26 @@ def _phase_coverage(ctx, p, srcjars): ) srcs_paths = [] for src in ctx.files.srcs: - srcs_paths = srcs_paths + [src.path] - in_out_pairs = [ + srcs_paths.append(src.path) + 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, ) @@ -75,5 +75,5 @@ def _phase_coverage(ctx, p, srcjars): }, ) -def _jacoco_offline_instrument_format_each(in_out_pair): - return (["%s=%s=%s" % (in_out_pair[0].path, in_out_pair[1].path, ",".join(in_out_pair[2]))]) +def _jacoco_offline_instrument_format_each(records): + return (["%s=%s=%s" % (records[0].path, records[1].path, ",".join(records[2]))]) From acaa15587c47d34b1b660dc74221ff2a978aad96 Mon Sep 17 00:00:00 2001 From: David Haxton Date: Mon, 24 Feb 2020 12:13:17 -0800 Subject: [PATCH 05/12] Remove bad assumptions about how bazel works --- .../coverage/instrumenter/JacocoInstrumenter.java | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java index 4f472fb94..31ec618c2 100644 --- a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java +++ b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java @@ -70,6 +70,11 @@ private void processArg(Instrumenter jacoco, String arg) throws Exception { throw new RuntimeException(e); } }); + + Files.write( + outFS.getPath("-paths-for-coverage.txt"), + srcs.replace(",", "\n").getBytes(java.nio.charset.StandardCharsets.UTF_8) + ); } } @@ -99,13 +104,6 @@ private FileVisitResult actuallyVisitFile(Path inPath, BasicFileAttributes attrs } else { Files.copy(inPath, outPath); } - - if(outPath.toString().endsWith(".class")) { - Files.write( - outFS.getPath((outPath.toString() + "-paths-for-coverage.txt")), - srcs.replace(",", "\n").getBytes(java.nio.charset.StandardCharsets.UTF_8) - ); - } return FileVisitResult.CONTINUE; } }; From 6d51720232b0872b632df416732fd5148e95ded8 Mon Sep 17 00:00:00 2001 From: David Haxton Date: Mon, 24 Feb 2020 12:25:47 -0800 Subject: [PATCH 06/12] Clean up lingering srcs Clean up lingering srcs --- .../coverage/instrumenter/JacocoInstrumenter.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java index 31ec618c2..91a011dcc 100644 --- a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java +++ b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java @@ -62,7 +62,7 @@ private void processArg(Instrumenter jacoco, String arg) throws Exception { FileSystem inFS = FileSystems.newFileSystem(inPath, null); FileSystem outFS = FileSystems.newFileSystem( URI.create("jar:" + outPath.toUri()), Collections.singletonMap("create", "true")); ) { - FileVisitor fileVisitor = createInstrumenterVisitor(jacoco, outFS, srcs); + FileVisitor fileVisitor = createInstrumenterVisitor(jacoco, outFS); inFS.getRootDirectories().forEach(root -> { try { Files.walkFileTree(root, fileVisitor); @@ -78,18 +78,18 @@ private void processArg(Instrumenter jacoco, String arg) throws Exception { } } - private SimpleFileVisitor createInstrumenterVisitor(Instrumenter jacoco, FileSystem outFS, String srcs) { + private SimpleFileVisitor createInstrumenterVisitor(Instrumenter jacoco, FileSystem outFS) { return new SimpleFileVisitor () { @Override public FileVisitResult visitFile(Path inPath, BasicFileAttributes attrs) { try { - return actuallyVisitFile(inPath, attrs, srcs); + return actuallyVisitFile(inPath, attrs); } catch (final Exception e) { throw new RuntimeException(e); } } - private FileVisitResult actuallyVisitFile(Path inPath, BasicFileAttributes attrs, String srcs) throws Exception { + private FileVisitResult actuallyVisitFile(Path inPath, BasicFileAttributes attrs) throws Exception { Path outPath = outFS.getPath(inPath.toString()); Files.createDirectories(outPath.getParent()); if (inPath.toString().endsWith(".class")) { From 576b00f354fbccb381aa0a7f9f4a07f21557c9d6 Mon Sep 17 00:00:00 2001 From: David Haxton Date: Mon, 24 Feb 2020 12:43:06 -0800 Subject: [PATCH 07/12] this commit should fail tests --- .../rulesscala/coverage/instrumenter/JacocoInstrumenter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java index 91a011dcc..54da8cbd2 100644 --- a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java +++ b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java @@ -72,7 +72,7 @@ private void processArg(Instrumenter jacoco, String arg) throws Exception { }); Files.write( - outFS.getPath("-paths-for-coverage.txt"), + outFS.getPath("-paths-for-blah.txt"), srcs.replace(",", "\n").getBytes(java.nio.charset.StandardCharsets.UTF_8) ); } From 4bf94080d3943e04a5f6202408808a43aa2529ab Mon Sep 17 00:00:00 2001 From: David Haxton Date: Mon, 24 Feb 2020 12:43:21 -0800 Subject: [PATCH 08/12] this commit should pass tests --- .../rulesscala/coverage/instrumenter/JacocoInstrumenter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java index 54da8cbd2..91a011dcc 100644 --- a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java +++ b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java @@ -72,7 +72,7 @@ private void processArg(Instrumenter jacoco, String arg) throws Exception { }); Files.write( - outFS.getPath("-paths-for-blah.txt"), + outFS.getPath("-paths-for-coverage.txt"), srcs.replace(",", "\n").getBytes(java.nio.charset.StandardCharsets.UTF_8) ); } From 9a8eaa9e67b0c9932f61e08a9b855051494d849c Mon Sep 17 00:00:00 2001 From: David Haxton Date: Mon, 24 Feb 2020 12:44:54 -0800 Subject: [PATCH 09/12] let this test actually fail the CI --- .../rulesscala/coverage/instrumenter/JacocoInstrumenter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java index 91a011dcc..54da8cbd2 100644 --- a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java +++ b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java @@ -72,7 +72,7 @@ private void processArg(Instrumenter jacoco, String arg) throws Exception { }); Files.write( - outFS.getPath("-paths-for-coverage.txt"), + outFS.getPath("-paths-for-blah.txt"), srcs.replace(",", "\n").getBytes(java.nio.charset.StandardCharsets.UTF_8) ); } From 8d0c3da0b6a246a78427be1ba88d323ad87da585 Mon Sep 17 00:00:00 2001 From: David Haxton Date: Mon, 24 Feb 2020 13:27:27 -0800 Subject: [PATCH 10/12] update comments --- .../coverage/instrumenter/JacocoInstrumenter.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java index 54da8cbd2..d553d7cf5 100644 --- a/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java +++ b/src/java/io/bazel/rulesscala/coverage/instrumenter/JacocoInstrumenter.java @@ -71,8 +71,18 @@ private void processArg(Instrumenter jacoco, String arg) throws Exception { } }); + /* + * 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-blah.txt"), + outFS.getPath("-paths-for-coverage.txt"), srcs.replace(",", "\n").getBytes(java.nio.charset.StandardCharsets.UTF_8) ); } From 0293fadf9a731697c657f6f4bba331419778adaf Mon Sep 17 00:00:00 2001 From: David Haxton Date: Tue, 25 Feb 2020 11:08:39 -0800 Subject: [PATCH 11/12] Refactor src_paths --- scala/private/phases/phase_coverage.bzl | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scala/private/phases/phase_coverage.bzl b/scala/private/phases/phase_coverage.bzl index 5294c20a0..985072562 100644 --- a/scala/private/phases/phase_coverage.bzl +++ b/scala/private/phases/phase_coverage.bzl @@ -36,9 +36,7 @@ def _phase_coverage(ctx, p, srcjars): output_jar = ctx.actions.declare_file( "{}-offline.jar".format(input_jar.basename.split(".")[0]), ) - srcs_paths = [] - for src in ctx.files.srcs: - srcs_paths.append(src.path) + src_paths = [src.path for src in ctx.files.srcs] records = [ (input_jar, output_jar, srcs_paths), ] From d7f49278602b8c7747c454474b516de6632cf3b3 Mon Sep 17 00:00:00 2001 From: David Haxton Date: Tue, 25 Feb 2020 11:12:14 -0800 Subject: [PATCH 12/12] spelling is hard --- scala/private/phases/phase_coverage.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scala/private/phases/phase_coverage.bzl b/scala/private/phases/phase_coverage.bzl index 985072562..f076b5ad4 100644 --- a/scala/private/phases/phase_coverage.bzl +++ b/scala/private/phases/phase_coverage.bzl @@ -36,7 +36,7 @@ def _phase_coverage(ctx, p, srcjars): output_jar = ctx.actions.declare_file( "{}-offline.jar".format(input_jar.basename.split(".")[0]), ) - src_paths = [src.path for src in ctx.files.srcs] + srcs_paths = [src.path for src in ctx.files.srcs] records = [ (input_jar, output_jar, srcs_paths), ]