Fixing JacocoRunner building script for Jacoco (#1485)

- Latest Bazel uses Jacoco 0.8.7
- Ensure that for building JacocoRunner we use a compatible Bazel version
- Created Bazel branch that adds interface extensions (interfaceNames) to
  Bazel's Jacoco implementation
- Java 8 needs to be used for building Jacoco

Updated documentation.
Updated manual test to build for Bazel 5.0+.
Updated expected coverage for manual test.

Author: gergelyfabian

docs/coverage.md

@@ -62,12 +62,16 @@ Coverage support has been only tested with [ScalaTest](http://www.scalatest.org/
 ### Working around missing lambda coverage with Scala 2.12+
 
 The current Jacoco version in Bazel (0.8.3) has missing coverage for lambdas
-(including for comprehensions; see issue https://github.com/bazelbuild/rules_scala/issues/1056).
+(including for comprehensions; see issue https://github.com/bazelbuild/rules_scala/issues/1056). Also, the support for
+filtering out code generated by the Scala compiler is quite reduced in Jacoco.
+
 This can be worked around by building a fixed version of Jacoco yourselves (including backported fixes from 0.8.5) and reconfiguring your
 build to use that one instead of the default `jacocorunner`.
 
 You can build jacocorunner with a script in `scripts/build_jacocorunner/build_jacocorunner.sh` (see comments there for more explanation and options).
 
+A new version of this script (for Bazel 5.0+) can be found in `scripts/build_jacocorunner/build_jacocorunner_bazel_5.0+.sh`.
+
 Then, you can use the `jacocorunner` property of `scala_toolchain` to provide the jacocorunner you have built:
 
 ```
@@ -94,10 +98,19 @@ filegroup(
 In this example `jacocorunner` is provided as a local file, but you could also upload your version to an artifactory and then use `http_file` (to avoid
 keeping binaries in your repository).
 
-Finally provide the `scala_toolchain` in your `.bazelrc` or as an option to `bazel coverage`:
+Finally, provide the `scala_toolchain` in your `.bazelrc` or as an option to `bazel coverage`:
 
 ```
 coverage --extra_toolchains="//coverage_local_jacocorunner:local_jacocorunner_scala_toolchain"
 ```
 
+You could also register the toolchain in your `WORKSPACE`.
+
 You can verify that the locally built `jacocorunner` works with `manual_test/coverage_local_jacocorunner/test.sh`.
+
+#### Notes
+
+Please ensure these scripts use Java 8.
+
+This should be done in the script itself, as e.g. the manual test requires a higher Java version, so you could add some
+code at the header of the build script to select Java 8 (appropriate for your Java installation).

manual_test/coverage_local_jacocorunner/expected-coverage.dat

@@ -2,16 +2,15 @@ SF:test/coverage_scalatest/A1.scala
 FN:-1,coverage_scalatest/A1$::<clinit> ()V
 FN:3,coverage_scalatest/A1$::<init> ()V
 FN:5,coverage_scalatest/A1$::a1 (Z)Lcoverage_scalatest/B1$;
-FN:-1,coverage_scalatest/A1::a1 (Z)Lcoverage_scalatest/B1$;
 FNDA:1,coverage_scalatest/A1$::<clinit> ()V
 FNDA:1,coverage_scalatest/A1$::<init> ()V
 FNDA:1,coverage_scalatest/A1$::a1 (Z)Lcoverage_scalatest/B1$;
-FNDA:0,coverage_scalatest/A1::a1 (Z)Lcoverage_scalatest/B1$;
-FNF:4
+FNF:3
 FNH:3
-BRDA:5,0,0,-
-BRF:1
-BRH:0
+BRDA:5,0,0,0
+BRDA:5,0,1,1
+BRF:2
+BRH:1
 DA:3,1
 DA:5,1
 DA:6,1
@@ -23,12 +22,10 @@ SF:test/coverage_scalatest/A2.scala
 FN:-1,coverage_scalatest/A2$::<clinit> ()V
 FN:3,coverage_scalatest/A2$::<init> ()V
 FN:5,coverage_scalatest/A2$::a2 ()V
-FN:-1,coverage_scalatest/A2::a2 ()V
 FNDA:1,coverage_scalatest/A2$::<clinit> ()V
 FNDA:1,coverage_scalatest/A2$::<init> ()V
 FNDA:1,coverage_scalatest/A2$::a2 ()V
-FNDA:0,coverage_scalatest/A2::a2 ()V
-FNF:4
+FNF:3
 FNH:3
 DA:3,1
 DA:5,1
@@ -40,12 +37,10 @@ SF:test/coverage_scalatest/B1.scala
 FN:-1,coverage_scalatest/B1$::<clinit> ()V
 FN:3,coverage_scalatest/B1$::<init> ()V
 FN:6,coverage_scalatest/B1$::not_called ()V
-FN:-1,coverage_scalatest/B1::not_called ()V
 FNDA:1,coverage_scalatest/B1$::<clinit> ()V
 FNDA:1,coverage_scalatest/B1$::<init> ()V
 FNDA:0,coverage_scalatest/B1$::not_called ()V
-FNDA:0,coverage_scalatest/B1::not_called ()V
-FNF:4
+FNF:3
 FNH:2
 DA:3,1
 DA:6,0
@@ -54,32 +49,30 @@ LH:2
 LF:3
 end_of_record
 SF:test/coverage_scalatest/B2.java
-FN:3,coverage_scalatest/B2::<init> ()V
-FN:5,coverage_scalatest/B2::b2_a ()Ljava/lang/String;
-FN:9,coverage_scalatest/B2::b2_b ()V
+FN:4,coverage_scalatest/B2::<init> ()V
+FN:6,coverage_scalatest/B2::b2_a ()Ljava/lang/String;
+FN:10,coverage_scalatest/B2::b2_b ()V
 FNDA:0,coverage_scalatest/B2::<init> ()V
 FNDA:1,coverage_scalatest/B2::b2_a ()Ljava/lang/String;
 FNDA:0,coverage_scalatest/B2::b2_b ()V
 FNF:3
 FNH:1
-DA:3,0
-DA:5,1
-DA:9,0
+DA:4,0
+DA:6,1
 DA:10,0
+DA:11,0
 LH:1
 LF:4
 end_of_record
 SF:test/coverage_scalatest/C2.scala
 FN:-1,coverage_scalatest/C2$::<clinit> ()V
 FN:3,coverage_scalatest/C2$::<init> ()V
 FN:5,coverage_scalatest/C2$::c2 (Ljava/lang/String;)Ljava/lang/String;
-FN:-1,coverage_scalatest/C2::c2 (Ljava/lang/String;)Ljava/lang/String;
 FNDA:1,coverage_scalatest/C2$::<clinit> ()V
 FNDA:1,coverage_scalatest/C2$::<init> ()V
 FNDA:1,coverage_scalatest/C2$::c2 (Ljava/lang/String;)Ljava/lang/String;
-FNDA:1,coverage_scalatest/C2::c2 (Ljava/lang/String;)Ljava/lang/String;
-FNF:4
-FNH:4
+FNF:3
+FNH:3
 DA:3,1
 DA:5,1
 DA:7,1
@@ -119,7 +112,6 @@ FN:18,coverage_scalatest/D1$::$anonfun$veryLongFunctionNameIsHereAaaaaaaaa$9 (Ls
 FN:-1,coverage_scalatest/D1$::<clinit> ()V
 FN:3,coverage_scalatest/D1$::<init> ()V
 FN:5,coverage_scalatest/D1$::veryLongFunctionNameIsHereAaaaaaaaa ()V
-FN:-1,coverage_scalatest/D1::veryLongFunctionNameIsHereAaaaaaaaa ()V
 FNDA:1,coverage_scalatest/D1$::$anonfun$veryLongFunctionNameIsHereAaaaaaaaa$1 (Lscala/collection/immutable/List;Ljava/lang/String;)Lscala/collection/immutable/List;
 FNDA:1,coverage_scalatest/D1$::$anonfun$veryLongFunctionNameIsHereAaaaaaaaa$10 (Lscala/collection/immutable/List;Ljava/lang/String;)Lscala/collection/immutable/List;
 FNDA:1,coverage_scalatest/D1$::$anonfun$veryLongFunctionNameIsHereAaaaaaaaa$11 (Lscala/collection/immutable/List;Ljava/lang/String;)Lscala/collection/immutable/List;
@@ -152,8 +144,7 @@ FNDA:1,coverage_scalatest/D1$::$anonfun$veryLongFunctionNameIsHereAaaaaaaaa$9 (L
 FNDA:1,coverage_scalatest/D1$::<clinit> ()V
 FNDA:1,coverage_scalatest/D1$::<init> ()V
 FNDA:1,coverage_scalatest/D1$::veryLongFunctionNameIsHereAaaaaaaaa ()V
-FNDA:0,coverage_scalatest/D1::veryLongFunctionNameIsHereAaaaaaaaa ()V
-FNF:33
+FNF:32
 FNH:32
 DA:3,1
 DA:5,1

manual_test/coverage_local_jacocorunner/test.sh

@@ -24,7 +24,7 @@ test_coverage_with_local_jacocorunner() {
 }
 
 build_local_jacocorunner() {
-    $root_dir/scripts/build_jacocorunner/build_jacocorunner.sh
+    $root_dir/scripts/build_jacocorunner/build_jacocorunner_bazel_5.0+.sh
     cp /tmp/bazel_jacocorunner_build/JacocoCoverage_jarjar_deploy.jar $root_dir/manual_test/coverage_local_jacocorunner
 }
 

scripts/build_jacocorunner/build_jacocorunner.sh

@@ -66,6 +66,17 @@ else
   exit 1
 fi
 
+JAVA_VERSION=$(java -version 2>&1 | head -1 \
+                                  | cut -d'"' -f2 \
+                                  | sed 's/^1\.//' \
+                                  | cut -d'.' -f1)
+
+if [ "$JAVA_VERSION" != "8" ]; then
+  echo "Unexpected java version: $JAVA_VERSION"
+  echo "Please ensure this script is run with Java 8"
+  exit 1
+fi
+
 source_path=$($readlink_cmd -f $(dirname ${BASH_SOURCE[0]}))
 
 build_dir=/tmp/bazel_jacocorunner_build

scripts/build_jacocorunner/build_jacocorunner_bazel_5.0+.sh

@@ -4,20 +4,26 @@
 #
 # The default `JacocoCoverage_jarjar_deploy.jar` has some issues:
 #
-# 1. Scala support on newer Jacoco versions (including 0.8.6) is still lacking some functionality
+# 1. Scala support on newer Jacoco versions (including 0.8.7) is still lacking some functionality
 #
 #    E.g. a lot of generated methods for case classes, lazy vals or other Scala features are causing falsely missed branches in branch coverage.
 #
 #    Proposed changes in:
 #    https://github.com/gergelyfabian/jacoco/tree/scala
 #
-#    Backported to 0.8.6 (to be usable with current Bazel):
-#    https://github.com/gergelyfabian/jacoco/tree/0.8.6-scala
+#    Backported to 0.8.7 (to be usable with current Bazel):
+#    https://github.com/gergelyfabian/jacoco/tree/0.8.7-scala
+#
+# 2. Bazel's code for generating `JacocoCoverage_jarjar_deploy.jar` needs changes after our Jacoco changes
+#
+#    It implements an interface that we have extended, so that implementation also needs to be extended.
+#
+#    This has been added on https://github.com/gergelyfabian/bazel/tree/jacoco_0.8.7_scala.
 #
 # You can use this script to build a custom version of `JacocoCoverage_jarjar_deploy.jar`, including any fixes from the above list you wish
 # and then provide the built jar as a parameter of `java_toolchain` and/or `scala_toolchain` to use the changed behavior for coverage.
 #
-# Choose the fixes from the above list by configuring the used branch for Jacoco repo below.
+# Choose the fixes from the above list by configuring the used branch for Jacoco/Bazel repos below.
 #
 # Patches:
 #
@@ -36,7 +42,23 @@
 
 set -e
 
-
+# Note!!
+# Ensure Java 8 is used for building Jacoco (experienced issue when using e.g. Java 17).
+# You may need to change this on your system.
+# If this matches your system, you could uncomment these lines:
+#export JAVA_HOME=/usr/lib/jvm/java-8-openjdk-amd64
+#export PATH=$JAVA_HOME/bin:$PATH
+
+JAVA_VERSION=$(java -version 2>&1 | head -1 \
+                                  | cut -d'"' -f2 \
+                                  | sed 's/^1\.//' \
+                                  | cut -d'.' -f1)
+
+if [ "$JAVA_VERSION" != "8" ]; then
+  echo "Unexpected java version: $JAVA_VERSION"
+  echo "Please ensure this script is run with Java 8"
+  exit 1
+fi
 
 if [[ "$OSTYPE" == "linux-gnu"* ]]; then
   readlink_cmd="readlink"
@@ -57,7 +79,7 @@ jacoco_repo=$build_dir/jacoco
 # Take a fork for Jacoco that contains Scala fixes.
 jacoco_remote=https://github.com/gergelyfabian/jacoco
 # Take further fixes for Scala (2.11, 2.12 and 2.13) - branch in development:
-jacoco_branch=0.8.6-scala
+jacoco_branch=0.8.7-scala
 
 # Choose the patches that you'd like to use:
 jacoco_patches=""
@@ -67,12 +89,14 @@ jacoco_patches="$jacoco_patches 0001-Build-Jacoco-for-Bazel-5.0+.patch"
 #jacoco_patches="$jacoco_patches 0002-Build-Jacoco-behind-proxy.patch"
 
 
-# Jacoco version should be 0.8.6 in any case as Bazel is only compatible with that at this moment.
-jacoco_version=0.8.6
+# Jacoco version should be 0.8.7 in any case as Bazel is only compatible with that at this moment.
+jacoco_version=0.8.7
 
 bazel_repo=$build_dir/bazel
-bazel_remote=https://github.com/bazelbuild/bazel
-bazel_tag=6.0.0-pre.20220520.1
+bazel_remote=https://github.com/gergelyfabian/bazel
+bazel_version=6.0.0-pre.20220520.1
+# Version of Bazel with extending Bazel's Jacoco interface implementation for our 0.8.7-scala jacoco branch.
+bazel_branch=jacoco_0.8.7_scala
 
 bazel_build_target=JacocoCoverage_jarjar_deploy.jar
 
@@ -108,7 +132,9 @@ mvn clean install
 (
 cd $bazel_repo
 git remote update
-git checkout tags/$bazel_tag
+git checkout $bazel_branch
+
+echo "$bazel_version" > .bazelversion
 
 # Prepare Jacoco version.
 cd third_party/java/jacoco