Skip to content

Commit a2ebdf7

Browse files
c-mitacopybara-github
c-mita
authored and
copybara-github
committed
Pass the name of the classpath manifest jar to JacocoCoverageRunner
When the java classpath exceeds the limit, we create a manifest JAR and pass that on the classpath. JacocoCoverageRunner knows how to extract information from this JAR. But if another JAR ends up on that classpath, it confuses the coverage runner which is expecting only a single jar in this case. This changes the java_stub_template file to export the name of the created manifest jar so the coverage runner can extract it. We also change the template file so the relevant exports don't occur in the middle of a larger function. It's possible this is somewhat overengineered and that we could always rely on the manifest jar always being the first one discovered by the coverage runner, but it is not totally obvious to me that that will always be true. Fixes #21268 Closes #21365. PiperOrigin-RevId: 608333782 Change-Id: I9895689fd9d771c9198e36bef222a9f86ada573e
1 parent 7d3b310 commit a2ebdf7

3 files changed

Lines changed: 209 additions & 14 deletions

File tree

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

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import static java.nio.file.StandardOpenOption.CREATE;
2121

2222
import com.google.common.annotations.VisibleForTesting;
23+
import com.google.common.base.Strings;
2324
import com.google.common.collect.ImmutableList;
2425
import com.google.common.collect.ImmutableList.Builder;
2526
import com.google.common.collect.ImmutableSet;
@@ -359,15 +360,33 @@ private static ImmutableList<File> getFilesFromFileList(File mainFile, String ja
359360
return convertedMetadataFiles.build();
360361
}
361362

362-
private static URL[] getUrls(ClassLoader classLoader, boolean wasWrappedJar) {
363+
private static URL[] getUrls(ClassLoader classLoader, boolean jarIsWrapped, String wrappedJar) {
364+
// jarIsWrapped is a legacy parameter; it should be removed once we are sure Bazel will no
365+
// longer set JACOCO_IS_JAR_WRAPPED in java_stub_template
363366
URL[] urls = getClassLoaderUrls(classLoader);
367+
if (urls == null || urls.length == 0) {
368+
return urls;
369+
}
364370
// If the classpath was too long then a temporary top-level jar is created containing nothing
365-
// but a manifest with
366-
// the original classpath. Those are the URLs we are looking for.
367-
if (wasWrappedJar && urls != null && urls.length == 1) {
371+
// but a manifest with the original classpath. Those are the URLs we are looking for.
372+
URL classPathUrl = null;
373+
if (!Strings.isNullOrEmpty(wrappedJar)) {
374+
for (URL url : urls) {
375+
if (url.getPath().endsWith(wrappedJar)) {
376+
classPathUrl = url;
377+
}
378+
}
379+
if (classPathUrl == null) {
380+
System.err.println("Classpath JAR " + wrappedJar + " not provided");
381+
return null;
382+
}
383+
} else if (jarIsWrapped && urls.length == 1) {
384+
classPathUrl = urls[0];
385+
}
386+
if (classPathUrl != null) {
368387
try {
369388
String jarClassPath =
370-
new JarInputStream(urls[0].openStream())
389+
new JarInputStream(classPathUrl.openStream())
371390
.getManifest()
372391
.getMainAttributes()
373392
.getValue("Class-Path");
@@ -427,14 +446,15 @@ private static URL[] getClassLoaderUrls(ClassLoader classLoader) {
427446
public static void main(String[] args) throws Exception {
428447
String metadataFile = System.getenv("JACOCO_METADATA_JAR");
429448
String jarWrappedValue = System.getenv("JACOCO_IS_JAR_WRAPPED");
449+
String wrappedJarValue = System.getenv("CLASSPATH_JAR");
430450
boolean wasWrappedJar = jarWrappedValue != null ? !jarWrappedValue.equals("0") : false;
431451

432452
File[] metadataFiles = null;
433453
int deployJars = 0;
434454
final HashMap<String, byte[]> uninstrumentedClasses = new HashMap<>();
435455
ImmutableSet.Builder<String> pathsForCoverageBuilder = new ImmutableSet.Builder<>();
436456
ClassLoader classLoader = ClassLoader.getSystemClassLoader();
437-
URL[] urls = getUrls(classLoader, wasWrappedJar);
457+
URL[] urls = getUrls(classLoader, wasWrappedJar, wrappedJarValue);
438458
if (urls != null) {
439459
metadataFiles = new File[urls.length];
440460
for (int i = 0; i < urls.length; i++) {

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

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,11 @@ export CLASSLOADER_PREFIX_PATH="${RUNPATH}"
269269
%set_jacoco_metadata%
270270
%set_jacoco_main_class%
271271
%set_jacoco_java_runfiles_root%
272+
# export JACOCO_IS_JAR_WRAPPED for compatibility with older versions of
273+
# JacocoCoverageRunner that check for this and not CLASSPATH_JAR
274+
# TODO(cmita): Remove when this is no longer required
272275
export JACOCO_IS_JAR_WRAPPED=0
276+
export CLASSPATH_JAR=""
273277

274278
if [[ -n "$JVM_DEBUG_PORT" ]]; then
275279
JVM_DEBUG_SUSPEND=${DEFAULT_JVM_DEBUG_SUSPEND:-"y"}
@@ -295,7 +299,8 @@ ARGS=(
295299
"${ARGS[@]}")
296300

297301

298-
function create_and_run_classpath_jar() {
302+
# Creates a JAR containing the classpath and put the result to stdout
303+
function create_classpath_jar() {
299304
# Build class path.
300305
MANIFEST_CLASSPATH=()
301306
if is_windows; then
@@ -365,13 +370,9 @@ function create_and_run_classpath_jar() {
365370
fi
366371
$JARBIN cvfm "$MANIFEST_JAR_FILE" "$MANIFEST_FILE" >/dev/null || \
367372
die "ERROR: $self failed because $JARBIN failed"
368-
369-
# Execute JAVA command
370-
$JAVABIN -classpath "$MANIFEST_JAR_FILE" "${ARGS[@]}"
371-
exit_code=$?
372373
rm -f "$MANIFEST_FILE"
373-
rm -f "$MANIFEST_JAR_FILE"
374-
exit $exit_code
374+
375+
echo "$MANIFEST_JAR_FILE"
375376
}
376377

377378
# If the user didn't specify a --classpath_limit, use the default value.
@@ -393,8 +394,17 @@ if ! is_macos; then
393394
fi
394395

395396
if (("${#CLASSPATH}" > ${CLASSPATH_LIMIT})); then
397+
# TODO(cmtia): Remove JACOCO_IS_JAR_WRAPPED when JacocoCoverageRunner will
398+
# never need it anymore.
396399
export JACOCO_IS_JAR_WRAPPED=1
397-
create_and_run_classpath_jar
400+
CLASSPATH_MANIFEST_JAR=$(create_classpath_jar)
401+
export CLASSPATH_JAR="$(basename $CLASSPATH_MANIFEST_JAR)"
402+
$JAVABIN -classpath "$CLASSPATH_MANIFEST_JAR" "${ARGS[@]}"
403+
exit_code=$?
404+
rm -f "$CLASSPATH_MANIFEST_JAR"
405+
exit $exit_code
398406
else
407+
export JACOCO_IS_JAR_WRAPPED=0
408+
export CLASSPATH_JAR=""
399409
exec $JAVABIN -classpath $CLASSPATH "${ARGS[@]}"
400410
fi

‎src/test/shell/bazel/bazel_coverage_java_test.sh‎

Lines changed: 165 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -614,6 +614,171 @@ end_of_record"
614614
assert_coverage_result "$expected_result_random" ${coverage_file_path}
615615
}
616616

617+
function test_java_coverage_with_classpath_jar() {
618+
# Verifies the logic in JacocoCoverageRunner can unpack the classpath jar
619+
# created when the classpath is too long.
620+
cat <<EOF > BUILD
621+
java_library(
622+
name = "lib",
623+
srcs = ["src/main/java/lib/Lib.java"],
624+
)
625+
626+
java_test(
627+
name = "lib_test",
628+
srcs = ["src/test/java/lib/TestLib.java"],
629+
test_class = "lib.TestLib",
630+
deps = [":lib"],
631+
)
632+
EOF
633+
634+
mkdir -p src/main/java/lib
635+
cat <<EOF > src/main/java/lib/Lib.java
636+
package lib;
637+
public class Lib {
638+
public static int calcX(int y) {
639+
return y * 2;
640+
}
641+
}
642+
EOF
643+
644+
mkdir -p src/test/java/lib
645+
cat <<EOF > src/test/java/lib/TestLib.java
646+
package lib;
647+
648+
import static org.junit.Assert.assertEquals;
649+
import org.junit.Test;
650+
651+
public class TestLib {
652+
@Test
653+
public void testCalcX() throws Exception {
654+
assertEquals(6, Lib.calcX(3));
655+
}
656+
}
657+
EOF
658+
659+
bazel coverage \
660+
--test_output=all \
661+
--combined_report=lcov \
662+
--instrumentation_filter="lib" \
663+
--action_env CLASSPATH_LIMIT=1 \
664+
//:lib_test &>$TEST_log \
665+
|| echo "Coverage for //:test failed"
666+
667+
local coverage_file_path="$(get_coverage_file_path_from_test_log)"
668+
local expected_result="SF:src/main/java/lib/Lib.java
669+
FN:2,lib/Lib::<init> ()V
670+
FN:4,lib/Lib::calcX (I)I
671+
FNDA:0,lib/Lib::<init> ()V
672+
FNDA:1,lib/Lib::calcX (I)I
673+
FNF:2
674+
FNH:1
675+
DA:2,0
676+
DA:4,1
677+
LH:1
678+
LF:2"
679+
680+
assert_coverage_result "$expected_result" "$coverage_file_path"
681+
}
682+
683+
function test_java_coverage_with_classpath_and_data_jar() {
684+
# Ignore this test when testing the released java tools.
685+
# TODO(cmita): Enable the test in this case after a java_tools release
686+
if [[ "$JAVA_TOOLS_ZIP" == "released" ]]; then
687+
return 0
688+
fi
689+
cat <<EOF > BUILD
690+
java_binary(
691+
name = "foo",
692+
srcs = ["src/main/java/foo/Foo.java"],
693+
main_class = "foo.Foo",
694+
deploy_manifest_lines = [
695+
"Premain-Class: foo.Foo",
696+
],
697+
use_launcher = False,
698+
)
699+
700+
java_library(
701+
name = "lib",
702+
srcs = ["src/main/java/lib/Lib.java"],
703+
)
704+
705+
java_test(
706+
name = "lib_test",
707+
srcs = ["src/test/java/lib/TestLib.java"],
708+
test_class = "lib.TestLib",
709+
deps = [":lib"],
710+
jvm_flags = ["-javaagent:\$(rootpath :foo_deploy.jar)"],
711+
data = [":foo_deploy.jar"],
712+
)
713+
EOF
714+
715+
mkdir -p src/main/java/foo
716+
cat <<EOF > src/main/java/foo/Foo.java
717+
package foo;
718+
public class Foo {
719+
public static void main(String[] args) {
720+
return;
721+
}
722+
723+
public static void premain(String args) {
724+
return;
725+
}
726+
727+
public static void agentmain(String args) {
728+
return;
729+
}
730+
}
731+
EOF
732+
733+
mkdir -p src/main/java/lib
734+
cat <<EOF > src/main/java/lib/Lib.java
735+
package lib;
736+
public class Lib {
737+
public static int calcX(int y) {
738+
return y * 2;
739+
}
740+
}
741+
EOF
742+
743+
mkdir -p src/test/java/lib
744+
cat <<EOF > src/test/java/lib/TestLib.java
745+
package lib;
746+
747+
import static org.junit.Assert.assertEquals;
748+
import org.junit.Test;
749+
750+
public class TestLib {
751+
@Test
752+
public void testCalcX() throws Exception {
753+
assertEquals(6, Lib.calcX(3));
754+
}
755+
}
756+
EOF
757+
758+
bazel coverage \
759+
--test_output=all \
760+
--combined_report=lcov \
761+
--instrumentation_filter="lib" \
762+
--action_env CLASSPATH_LIMIT=1 \
763+
//:lib_test &>$TEST_log \
764+
|| echo "Coverage for //:test failed"
765+
766+
local coverage_file_path="$(get_coverage_file_path_from_test_log)"
767+
local expected_result="SF:src/main/java/lib/Lib.java
768+
FN:2,lib/Lib::<init> ()V
769+
FN:4,lib/Lib::calcX (I)I
770+
FNDA:0,lib/Lib::<init> ()V
771+
FNDA:1,lib/Lib::calcX (I)I
772+
FNF:2
773+
FNH:1
774+
DA:2,0
775+
DA:4,1
776+
LH:1
777+
LF:2"
778+
779+
assert_coverage_result "$expected_result" "$coverage_file_path"
780+
}
781+
617782
function test_java_string_switch_coverage() {
618783
# Verify that Jacoco's filtering is being applied.
619784
# Switches on strings generate over double the number of expected branches

0 commit comments

Comments
 (0)