Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply Jacoco's instruction filtering to Java branch coverage #13896

Closed
wants to merge 3 commits into from

Conversation

c-mita
Copy link
Member

@c-mita c-mita commented Aug 24, 2021

Jacoco applies a set of filters during its coverage analysis to remove
various compiler constructs that could lead to surprising outputs in
coverage results, leaving behind coverage data that closer fits the
code given to the compiler.

A simple java example is the function:

public int foo(String input) {
  switch (input) {
    case "A":
      return 1;
    case "B":
      return 2;
    case "C":
      return 3;
    default:
      return -1
  }
}

This would generate at least double the number of expected branches as
the compiler generates a second set of branches to operate on hashCode.
Jacoco would normally ignore the first set of branches, reducing the
number to the expected 4.

Because Bazel applies its own branch analysis step, Bazel's branch
coverage output does not benefit from this filtering.

This change adapts the custom analyzer to record the necessary
information during the ASM Tree walk in the same manner as Jacoco's
regular analyzer in order to apply the filters.

The filters have three output operations:

  • ignore - ignore a range of instructions
  • merge - merges the coverage information of two instructions
  • replaceBranches - retarget the output branches of an instruction

The first of these, ignore, is by far the simplest to implement and
covers the vast majority of use cases in Java.

By mapping the AbstractInsnNode objects recorded during the ASM Tree
walk to the Instruction objects used during branch analysis, we can
simply skip over Instruction objects that have had their associated
AbstractInsnNode object ignored by a filter.

The remaining operations, merge and replaceBranches, are left
unimplemeted for now; these require more care to handle, are harder
to test, and affect only a minority of cases, specifically:

  • merge is only used to handle duplication of finally blocks
  • replaceBranches is used by Kotlin and Eclipse compiler filters

Part of #12696

@google-cla google-cla bot added the cla: yes label Aug 24, 2021
@c-mita
Copy link
Member Author

c-mita commented Aug 24, 2021

To be clear, I don't think supporting the other filter operations is impossible; the main issue is testing, especially for the replaceBranches call.

@c-mita c-mita requested a review from comius August 24, 2021 18:11
@c-mita c-mita linked an issue Aug 24, 2021 that may be closed by this pull request
@c-mita c-mita force-pushed the jacoco_filter branch 2 times, most recently from 1fbb937 to 0cf44ce Compare August 25, 2021 11:53
c-mita added 3 commits August 26, 2021 13:56
This acts as a preliminary test for the application of Jacoco's filters.

The java compiler will generate a large number of branches on a string
switch statement (one set for testing against String::hashCode and a
second set as backup) - the majority of these should be filtered as
Jacoco normally would.
Jacoco's filters operate on asm.tree.AbstractInsnNode objects. We track
these as we walk through the ASM tree in the same way Jacoco does, apply
the filters to them appropriately, and then map ignored nodes back to
the Instruction objects we already operated on.

We only handle IFilter::ignore for now; this covers the majority of
filters.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bazel branch coverage ignores Jacoco filtering
1 participant