diff --git a/src/java_tools/junitrunner/java/com/google/testing/coverage/BranchExp.java b/src/java_tools/junitrunner/java/com/google/testing/coverage/BranchExp.java index 4d7328bf549884..0ed2f27081578e 100644 --- a/src/java_tools/junitrunner/java/com/google/testing/coverage/BranchExp.java +++ b/src/java_tools/junitrunner/java/com/google/testing/coverage/BranchExp.java @@ -15,6 +15,7 @@ package com.google.testing.coverage; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; /** A branch coverage that must be evaluated as a combination of probes. */ @@ -57,11 +58,33 @@ public void update(int idx, CovExp exp) { branches.set(idx, exp); } - /** Make a union of the branches of two BranchExp. */ - public BranchExp merge(BranchExp other) { - List mergedBranches = new ArrayList<>(branches); - mergedBranches.addAll(other.branches); - return new BranchExp(mergedBranches); + /** Make a new BranchExp representing the concatenation of branches in inputs. */ + public static BranchExp concatenate(BranchExp first, BranchExp second) { + List branches = new ArrayList<>(first.branches); + branches.addAll(second.branches); + return new BranchExp(branches); + } + + /** Make a new BranchExp representing the pairwise union of branches in inputs */ + public static BranchExp zip(BranchExp left, BranchExp right) { + List zippedBranches = new ArrayList<>(); + int leftSize = left.branches.size(); + int rightSize = right.branches.size(); + int i; + for (i = 0; i < leftSize && i < rightSize; i++) { + List branches = Arrays.asList(left.branches.get(i), right.branches.get(i)); + zippedBranches.add(new BranchExp(branches)); + } + List remainder = leftSize < rightSize ? right.branches : left.branches; + for (; i < remainder.size(); i++) { + zippedBranches.add(new BranchExp(remainder.get(i))); + } + return new BranchExp(zippedBranches); + } + + /** Wraps a CovExp in a BranchExp if it isn't one already. */ + public static BranchExp ensureIsBranchExp(CovExp exp) { + return exp instanceof BranchExp ? (BranchExp) exp : new BranchExp(exp); } @Override diff --git a/src/java_tools/junitrunner/java/com/google/testing/coverage/MethodProbesMapper.java b/src/java_tools/junitrunner/java/com/google/testing/coverage/MethodProbesMapper.java index 69fcc72b055e1b..f1dd594444d77c 100644 --- a/src/java_tools/junitrunner/java/com/google/testing/coverage/MethodProbesMapper.java +++ b/src/java_tools/junitrunner/java/com/google/testing/coverage/MethodProbesMapper.java @@ -72,6 +72,7 @@ public class MethodProbesMapper extends MethodProbesVisitor implements IFilterOu private IFilter filter; private IFilterContext filterContext; private HashSet ignored = new HashSet<>(); + private Map unioned = new HashMap<>(); // Result private Map lineToBranchExp = new TreeMap(); @@ -349,13 +350,6 @@ private boolean updateBranchPredecessor(Instruction predecessor, Instruction ins /** Finishing the method */ @Override public void visitEnd() { - HashSet ignoredInstructions = new HashSet<>(); - for (Map.Entry entry : instructionMap.entrySet()) { - if (ignored.contains(entry.getKey())) { - ignoredInstructions.add(entry.getValue()); - } - } - for (Jump jump : jumps) { Instruction insn = labelToInsn.get(jump.target); jump.source.addBranch(insn, jump.branch); @@ -366,9 +360,6 @@ public void visitEnd() { for (Map.Entry entry : probeToInsn.entrySet()) { int probeId = entry.getKey(); Instruction ins = entry.getValue(); - if (ignoredInstructions.contains(ins)) { - continue; - } Instruction insn = ins; CovExp exp = new ProbeExp(probeId); @@ -411,6 +402,24 @@ public void visitEnd() { } } + // Handle merged instructions + for (AbstractInsnNode node : unioned.keySet()) { + AbstractInsnNode rep = findRepresentative(node); + Instruction insn = instructionMap.get(node); + Instruction repInsn = instructionMap.get(rep); + BranchExp branch = BranchExp.ensureIsBranchExp(insnToCovExp.get(insn)); + BranchExp repBranch = BranchExp.ensureIsBranchExp(insnToCovExp.get(repInsn)); + insnToCovExp.put(repInsn, BranchExp.zip(repBranch, branch)); + ignored.add(node); + } + + HashSet ignoredInstructions = new HashSet<>(); + for (Map.Entry entry : instructionMap.entrySet()) { + if (ignored.contains(entry.getKey())) { + ignoredInstructions.add(entry.getValue()); + } + } + // Merge branches in the instructions on the same line for (Instruction insn : instructions) { if (ignoredInstructions.contains(insn)) { @@ -424,7 +433,7 @@ public void visitEnd() { if (lineExp == null) { lineToBranchExp.put(insn.getLine(), exp); } else { - lineToBranchExp.put(insn.getLine(), lineExp.merge(exp)); + lineToBranchExp.put(insn.getLine(), BranchExp.concatenate(lineExp, exp)); } } else { // If we reach here, the internal data of the mapping is inconsistent, either @@ -447,7 +456,12 @@ public void ignore(AbstractInsnNode fromInclusive, AbstractInsnNode toInclusive) @Override public void merge(AbstractInsnNode i1, AbstractInsnNode i2) { - // TODO(cmita): Implement as part of https://github.com/bazelbuild/bazel/issues/12696 + // Track nodes to be merged using a union-find algorithm. + i1 = findRepresentative(i1); + i2 = findRepresentative(i2); + if (i1 != i2) { + unioned.put(i1, i2); + } } @Override @@ -455,6 +469,20 @@ public void replaceBranches(AbstractInsnNode source, Set newTa // TODO(cmita): Implement as part of https://github.com/bazelbuild/bazel/issues/12696 } + private AbstractInsnNode findRepresentative(AbstractInsnNode node) { + // The "find" part of union-find. Walk the chain of nodes to find the representative node + // (at the root), flattening the tree a little as we go. + AbstractInsnNode parent; + AbstractInsnNode grandParent; + while ((parent = unioned.get(node)) != null) { + if ((grandParent = unioned.get(parent)) != null) { + unioned.put(node, grandParent); + } + node = parent; + } + return node; + } + /** Jumps between instructions and labels */ class Jump { public final Instruction source; diff --git a/src/test/shell/bazel/bazel_coverage_java_test.sh b/src/test/shell/bazel/bazel_coverage_java_test.sh index db4dddbf1a214a..d8104ced1a5cbf 100755 --- a/src/test/shell/bazel/bazel_coverage_java_test.sh +++ b/src/test/shell/bazel/bazel_coverage_java_test.sh @@ -889,12 +889,8 @@ BRDA:9,0,3,0 BRDA:12,0,0,- BRDA:12,0,1,- BRDA:18,0,0,0 -BRDA:18,0,1,0 -BRDA:18,0,2,0 -BRDA:18,0,3,0 -BRDA:18,0,4,0 -BRDA:18,0,5,1 -BRF:12 +BRDA:18,0,1,1 +BRF:8 BRH:3" assert_coverage_result "$expected_result" "$coverage_file_path" @@ -911,13 +907,9 @@ BRDA:9,0,2,0 BRDA:9,0,3,1 BRDA:12,0,0,1 BRDA:12,0,1,0 -BRDA:18,0,0,0 -BRDA:18,0,1,0 -BRDA:18,0,2,1 -BRDA:18,0,3,1 -BRDA:18,0,4,0 -BRDA:18,0,5,0 -BRF:12 +BRDA:18,0,0,1 +BRDA:18,0,1,1 +BRF:8 BRH:5" assert_coverage_result "$expected_result" "$coverage_file_path" @@ -935,12 +927,8 @@ BRDA:9,0,3,1 BRDA:12,0,0,1 BRDA:12,0,1,1 BRDA:18,0,0,1 -BRDA:18,0,1,0 -BRDA:18,0,2,0 -BRDA:18,0,3,1 -BRDA:18,0,4,0 -BRDA:18,0,5,0 -BRF:12 +BRDA:18,0,1,1 +BRF:8 BRH:6" assert_coverage_result "$expected_result" "$coverage_file_path" @@ -957,13 +945,9 @@ BRDA:9,0,2,0 BRDA:9,0,3,1 BRDA:12,0,0,0 BRDA:12,0,1,1 -BRDA:18,0,0,0 +BRDA:18,0,0,1 BRDA:18,0,1,1 -BRDA:18,0,2,0 -BRDA:18,0,3,0 -BRDA:18,0,4,1 -BRDA:18,0,5,0 -BRF:12 +BRF:8 BRH:6" assert_coverage_result "$expected_result" "$coverage_file_path" @@ -982,12 +966,27 @@ BRDA:12,0,0,1 BRDA:12,0,1,1 BRDA:18,0,0,0 BRDA:18,0,1,1 -BRDA:18,0,2,0 -BRDA:18,0,3,1 -BRDA:18,0,4,0 -BRDA:18,0,5,0 -BRF:12 -BRH:6" +BRF:8 +BRH:5" + assert_coverage_result "$expected_result" "$coverage_file_path" + + bazel coverage --test_output=all //:test \ + --coverage_report_generator=@bazel_tools//tools/test:coverage_report_generator \ + --combined_report=lcov &>$TEST_log \ + --test_filter=".*(testOdd|testException)" \ + || echo "Coverage for //:test failed" + + local coverage_file_path="$( get_coverage_file_path_from_test_log )" + local expected_result="BRDA:9,0,0,1 +BRDA:9,0,1,1 +BRDA:9,0,2,0 +BRDA:9,0,3,1 +BRDA:12,0,0,1 +BRDA:12,0,1,0 +BRDA:18,0,0,1 +BRDA:18,0,1,0 +BRF:8 +BRH:5" assert_coverage_result "$expected_result" "$coverage_file_path" }