jacoco Which branch was missed?

Which branch was missed?

Open pjlewis opened this issue 11 years ago • 20 comments

I'm switching a project from Cobertura to JaCoCo - as part of an eventual upgrade to Java 1.7. I've noticed that JaCoCo indicates missed branches but it doesn't tell you which branch was missed. So in code like the following...

if(a==b || c==d) { ... }

JaCoCo might say '1 of 4 branches' missed. But it doesn't tell me which branch (or even which term in the condition). Cobertura used to give an indication in the form '%50/%100' (I think) which gave some idea of where to start. Could something like this be added?

Mar 12 '13 16:03 pjlewis

It is very unlikely that JaCoCo can provide useful information here as it is fully based on Java byte code and we can't tell how a particular source expression in compiled into bytecode.

Mar 12 '13 16:03 marchof

Wasn't Cobertura 'fully based on Java byte code' too? How did they do it?

Mar 12 '13 16:03 pjlewis

They parse the source code in addition.

Mar 12 '13 16:03 marchof

Can you plan to have this feature too ? Because this is a killer feature...

Apr 02 '13 03:04 jvmvik

@marchof Maybe we can cooperate on this? SonarSource Language Recognizer is pretty lightweight and easy to use, and parser for Java is pretty stable and fast and of course will evolve with evolution of Java language (Java 8, ...). All this is to say that it can be embedded into JaCoCo to cover such feature. WDYT?

Apr 02 '13 08:04 Godin

Worth keeping non-Java JVM languages in mind when thinking about this feature. Groovy, Clojure, Scala, JRuby, Jython, Rhino, etc, etc, ...

Apr 02 '13 08:04 pjlewis

@pjlewis Ah, indeed, you right.

Apr 02 '13 08:04 Godin

@Godin thanks for offering your support with the Sonar Language Recognizer! But for now I don't want to consider source code for JaCoCo analysis to keep things simple.

Just from looking at the class files it is probably possible to show some hints. All boolean expressions get their input from local variables, fields or method return values. All of them can be presented without parsing the source.

Apr 02 '13 15:04 marchof

Let's consider for a moment a single branch instruction. JaCoCo should tell us one of 4 possibilities: not executed, taken, not taken or both. This information would be very helpful.

Let's consider for a moment two branch instructions. JaCoCo should tell us the possibility for each branch instruction. Most of the time (if not all) the first branch corresponds to the first Java condition and the second branch corresponds to the second Java condition. This is because Java has to evaluate the first condition before the second condition. If it is possible that the branch instructions can be in reverse order, then JaCoCo should add the branch instruction type (i.e. <, <=, >, >=, ==, !=, etc) and if possible where the values came from (e.g. i, m_member, System.nanoTime, etc). This will give some information to the user so they can possibly figure out which branch corresponds to which condition.

This information will cover the majority of use cases. For the cases that don't work, then file bugs and deal with them on a case by case basis. Some cases may not be possible to fix. Fine. At least, JaCoCo will have a solution that works most of the time instead of not at all.

Aug 28 '13 17:08 numeralnathan

I think it's good idea but we need to parse the source to achieve this, right ?

Sep 11 '13 09:09 jvmvik

My previous comment doesn't require parsing the source. It requires looking at the bytecode. In some cases, the information will be crude. But even crude information will be a lot more helpful than no information.

Sep 11 '13 14:09 numeralnathan

@nathanila Any suggestions are welcome! To get started I recommend to write down some simple multi-condition expressions, compile them and take a look at the byte code (e.g. with javap). My guess is that this "crude information" will not be helpful at all to a typical Java developer who is not into the JVM bytecode details.

Sep 11 '13 22:09 marchof

I looked for some code where I have 2 conditions on 1 line. I found this.

  if ((m_fullGCs > 0) && (!isFullGC(matcher)))
     return;

I then used javap to dump the byte code for the line.

    33: aload_0       
    34: getfield      #57                 // Field m_fullGCs:I
    37: ifle          48
    40: aload_1       
    41: invokestatic  #78                 // Method isFullGC:(Ljava/util/regex/Matcher;)Z
    44: ifne          48
    47: return

The branch coverage could say the following:

Branches: 1st branch (<=): taken covered 2nd branch (!=): both covered

The <= might be confusing. However with 1st and 2nd branch the user will quickly realize the compiler changed the operator for the first branch.

The next enhancement is to analyze what is on the stack for ifle and ifne and where it came from. Then add that information to the branch coverage. The branch coverage could say the following:

Branches: 1st branch: m_fullGCs <= 0: taken covered 2nd branch: isFullGC() != true: both covered

Although, the information doesn't match exactly to the Java code, the user should be able to figure it out. Without this information, the user has a lot more work to do to figure out the coverage... if even possible.

Sep 12 '13 01:09 numeralnathan

This is definitely an interesting feature to have in a coverage tool. I have it working reasonably well in my own tool (JMockit Coverage), so it's doable (and yes, it does require parsing the source code). Based on my experience, I can say it's quite difficult to get right for all possible cases; it will probably take me several years still.

Oct 14 '15 19:10 rliesenfeld

@rliesenfeld Thanks for sharing your insights here! That's why I don't think we will ever do source code parsing for JaCoCo. Rather we use compiler features like the CharacterRangeTable attribute.

Oct 14 '15 20:10 marchof

@C-Otto This issue is about creating a mapping back to the source code to give the user meaningful information about individual branches in from the source point of view. I do not understand how your example relates to this.

Nov 18 '19 18:11 marchof

@marchof you're right, I'm sorry. I misread the issue. Furthermore, I think the issue I described is already fixed in master. Thanks!