[7.1.0] Pass the name of the classpath manifest jar to JacocoCoverageRunner

src/java_tools/junitrunner/java/com/google/testing/coverage/JacocoCoverageRunner.java

@@ -20,6 +20,7 @@
 import static java.nio.file.StandardOpenOption.CREATE;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableList.Builder;
 import com.google.common.collect.ImmutableSet;
@@ -360,15 +361,33 @@ private static ImmutableList<File> getFilesFromFileList(File mainFile, String ja
     return convertedMetadataFiles.build();
   }
 
-  private static URL[] getUrls(ClassLoader classLoader, boolean wasWrappedJar) {
+  private static URL[] getUrls(ClassLoader classLoader, boolean jarIsWrapped, String wrappedJar) {
+    // jarIsWrapped is a legacy parameter; it should be removed once we are sure Bazel will no
+    // longer set JACOCO_IS_JAR_WRAPPED in java_stub_template
     URL[] urls = getClassLoaderUrls(classLoader);
+    if (urls == null || urls.length == 0) {
+      return urls;
+    }
     // If the classpath was too long then a temporary top-level jar is created containing nothing
-    // but a manifest with
-    // the original classpath. Those are the URLs we are looking for.
-    if (wasWrappedJar && urls != null && urls.length == 1) {
+    // but a manifest with the original classpath. Those are the URLs we are looking for.
+    URL classPathUrl = null;
+    if (!Strings.isNullOrEmpty(wrappedJar)) {
+      for (URL url : urls) {
+        if (url.getPath().endsWith(wrappedJar)) {
+          classPathUrl = url;
+        }
+      }
+      if (classPathUrl == null) {
+        System.err.println("Classpath JAR " + wrappedJar + " not provided");
+        return null;
+      }
+    } else if (jarIsWrapped && urls.length == 1) {
+      classPathUrl = urls[0];
+    }
+    if (classPathUrl != null) {
       try {
         String jarClassPath =
-            new JarInputStream(urls[0].openStream())
+            new JarInputStream(classPathUrl.openStream())
                 .getManifest()
                 .getMainAttributes()
                 .getValue("Class-Path");
@@ -428,14 +447,15 @@ private static URL[] getClassLoaderUrls(ClassLoader classLoader) {
   public static void main(String[] args) throws Exception {
     String metadataFile = System.getenv("JACOCO_METADATA_JAR");
     String jarWrappedValue = System.getenv("JACOCO_IS_JAR_WRAPPED");
+    String wrappedJarValue = System.getenv("CLASSPATH_JAR");
     boolean wasWrappedJar = jarWrappedValue != null ? !jarWrappedValue.equals("0") : false;
 
     File[] metadataFiles = null;
     int deployJars = 0;
     final HashMap<String, byte[]> uninstrumentedClasses = new HashMap<>();
     ImmutableSet.Builder<String> pathsForCoverageBuilder = new ImmutableSet.Builder<>();
     ClassLoader classLoader = ClassLoader.getSystemClassLoader();
-    URL[] urls = getUrls(classLoader, wasWrappedJar);
+    URL[] urls = getUrls(classLoader, wasWrappedJar, wrappedJarValue);
     if (urls != null) {
       metadataFiles = new File[urls.length];
       for (int i = 0; i < urls.length; i++) {

src/main/java/com/google/devtools/build/lib/bazel/rules/java/java_stub_template.txt

@@ -269,7 +269,11 @@ export CLASSLOADER_PREFIX_PATH="${RUNPATH}"
 %set_jacoco_metadata%
 %set_jacoco_main_class%
 %set_jacoco_java_runfiles_root%
+# export JACOCO_IS_JAR_WRAPPED for compatibility with older versions of
+# JacocoCoverageRunner that check for this and not CLASSPATH_JAR
+# TODO(cmita): Remove when this is no longer required
 export JACOCO_IS_JAR_WRAPPED=0
+export CLASSPATH_JAR=""
 
 if [[ -n "$JVM_DEBUG_PORT" ]]; then
   JVM_DEBUG_SUSPEND=${DEFAULT_JVM_DEBUG_SUSPEND:-"y"}
@@ -295,7 +299,8 @@ ARGS=(
   "${ARGS[@]}")
 
 
-function create_and_run_classpath_jar() {
+# Creates a JAR containing the classpath and put the result to stdout
+function create_classpath_jar() {
   # Build class path.
   MANIFEST_CLASSPATH=()
   if is_windows; then
@@ -365,13 +370,9 @@ function create_and_run_classpath_jar() {
   fi
   $JARBIN cvfm "$MANIFEST_JAR_FILE" "$MANIFEST_FILE" >/dev/null || \
     die "ERROR: $self failed because $JARBIN failed"
-
-  # Execute JAVA command
-  $JAVABIN -classpath "$MANIFEST_JAR_FILE" "${ARGS[@]}"
-  exit_code=$?
   rm -f "$MANIFEST_FILE"
-  rm -f "$MANIFEST_JAR_FILE"
-  exit $exit_code
+
+  echo "$MANIFEST_JAR_FILE"
 }
 
 # If the user didn't specify a --classpath_limit, use the default value.
@@ -393,8 +394,17 @@ if ! is_macos; then
 fi
 
 if (("${#CLASSPATH}" > ${CLASSPATH_LIMIT})); then
+  # TODO(cmtia): Remove JACOCO_IS_JAR_WRAPPED when JacocoCoverageRunner will
+  # never need it anymore.
   export JACOCO_IS_JAR_WRAPPED=1
-  create_and_run_classpath_jar
+  CLASSPATH_MANIFEST_JAR=$(create_classpath_jar)
+  export CLASSPATH_JAR="$(basename $CLASSPATH_MANIFEST_JAR)"
+  $JAVABIN -classpath "$CLASSPATH_MANIFEST_JAR" "${ARGS[@]}"
+  exit_code=$?
+  rm -f "$CLASSPATH_MANIFEST_JAR"
+  exit $exit_code
 else
+  export JACOCO_IS_JAR_WRAPPED=0
+  export CLASSPATH_JAR=""
   exec $JAVABIN -classpath $CLASSPATH "${ARGS[@]}"
 fi

src/test/shell/bazel/bazel_coverage_java_test.sh

@@ -632,6 +632,171 @@ end_of_record"
   assert_coverage_result "$expected_result_random" ${coverage_file_path}
 }
 
+function test_java_coverage_with_classpath_jar() {
+  # Verifies the logic in JacocoCoverageRunner can unpack the classpath jar
+  # created when the classpath is too long.
+  cat <<EOF > BUILD
+java_library(
+    name = "lib",
+    srcs = ["src/main/java/lib/Lib.java"],
+)
+
+java_test(
+    name = "lib_test",
+    srcs = ["src/test/java/lib/TestLib.java"],
+    test_class = "lib.TestLib",
+    deps = [":lib"],
+)
+EOF
+
+  mkdir -p src/main/java/lib
+  cat <<EOF > src/main/java/lib/Lib.java
+package lib;
+public class Lib {
+  public static int calcX(int y) {
+    return y * 2;
+  }
+}
+EOF
+
+  mkdir -p src/test/java/lib
+  cat <<EOF > src/test/java/lib/TestLib.java
+package lib;
+
+import static org.junit.Assert.assertEquals;
+import org.junit.Test;
+
+public class TestLib {
+  @Test
+  public void testCalcX() throws Exception {
+    assertEquals(6, Lib.calcX(3));
+  }
+}
+EOF
+
+  bazel coverage \
+    --test_output=all \
+    --combined_report=lcov \
+    --instrumentation_filter="lib" \
+    --action_env CLASSPATH_LIMIT=1 \
+    //:lib_test &>$TEST_log \
+      || echo "Coverage for //:test failed"
+
+  local coverage_file_path="$(get_coverage_file_path_from_test_log)"
+  local expected_result="SF:src/main/java/lib/Lib.java
+FN:2,lib/Lib::<init> ()V
+FN:4,lib/Lib::calcX (I)I
+FNDA:0,lib/Lib::<init> ()V
+FNDA:1,lib/Lib::calcX (I)I
+FNF:2
+FNH:1
+DA:2,0
+DA:4,1
+LH:1
+LF:2"
+
+  assert_coverage_result "$expected_result" "$coverage_file_path"
+}
+
+function test_java_coverage_with_classpath_and_data_jar() {
+  # Ignore this test when testing the released java tools.
+  # TODO(cmita): Enable the test in this case after a java_tools release
+  if [[ "$JAVA_TOOLS_ZIP" == "released" ]]; then
+    return 0
+  fi
+  cat <<EOF > BUILD
+java_binary(
+    name = "foo",
+    srcs = ["src/main/java/foo/Foo.java"],
+    main_class = "foo.Foo",
+    deploy_manifest_lines = [
+      "Premain-Class: foo.Foo",
+    ],
+    use_launcher = False,
+)
+
+java_library(
+    name = "lib",
+    srcs = ["src/main/java/lib/Lib.java"],
+)
+
+java_test(
+    name = "lib_test",
+    srcs = ["src/test/java/lib/TestLib.java"],
+    test_class = "lib.TestLib",
+    deps = [":lib"],
+    jvm_flags = ["-javaagent:\$(rootpath :foo_deploy.jar)"],
+    data = [":foo_deploy.jar"],
+)
+EOF
+
+  mkdir -p src/main/java/foo
+  cat <<EOF > src/main/java/foo/Foo.java
+package foo;
+public class Foo {
+  public static void main(String[] args) {
+    return;
+  }
+
+  public static void premain(String args) {
+    return;
+  }
+
+  public static void agentmain(String args) {
+    return;
+  }
+}
+EOF
+
+  mkdir -p src/main/java/lib
+  cat <<EOF > src/main/java/lib/Lib.java
+package lib;
+public class Lib {
+  public static int calcX(int y) {
+    return y * 2;
+  }
+}
+EOF
+
+  mkdir -p src/test/java/lib
+  cat <<EOF > src/test/java/lib/TestLib.java
+package lib;
+
+import static org.junit.Assert.assertEquals;
+import org.junit.Test;
+
+public class TestLib {
+  @Test
+  public void testCalcX() throws Exception {
+    assertEquals(6, Lib.calcX(3));
+  }
+}
+EOF
+
+  bazel coverage \
+    --test_output=all \
+    --combined_report=lcov \
+    --instrumentation_filter="lib" \
+    --action_env CLASSPATH_LIMIT=1 \
+    //:lib_test &>$TEST_log \
+      || echo "Coverage for //:test failed"
+
+  local coverage_file_path="$(get_coverage_file_path_from_test_log)"
+  local expected_result="SF:src/main/java/lib/Lib.java
+FN:2,lib/Lib::<init> ()V
+FN:4,lib/Lib::calcX (I)I
+FNDA:0,lib/Lib::<init> ()V
+FNDA:1,lib/Lib::calcX (I)I
+FNF:2
+FNH:1
+DA:2,0
+DA:4,1
+LH:1
+LF:2"
+
+  assert_coverage_result "$expected_result" "$coverage_file_path"
+}
+
 function test_java_string_switch_coverage() {
   # Verify that Jacoco's filtering is being applied.
   # Switches on strings generate over double the number of expected branches