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

Jaoco does not count branch for conditionals with early returns. #313

Closed
zhihan opened this issue May 20, 2015 · 6 comments
Closed

Jaoco does not count branch for conditionals with early returns. #313

zhihan opened this issue May 20, 2015 · 6 comments

Comments

@zhihan
Copy link

zhihan commented May 20, 2015

Here is a simple chunk of Java code.

public class MyLib {
    public String returnNullFunction(String s) {
    if (s == null) { // <--- the branch that is not reported.
    return null;
    }
    if (s.equals("yes")) {
      return "yes";
    }
    return "no";
  }
}

Jacoco inserts three probes, each for the return arguments. Then the first if condition is only associated with probe 0. And then the tool does not recognize it as a branch. The instrumented Java bytecode is attached below.

public java.lang.String returnNullFunction(java.lang.String);
Code:
   0: invokestatic  #34                 // Method $jacocoInit:()[Z
   3: astore_2      
   4: aload_1       
   5: ifnonnull     14              <--------- the jump, which is clearly a branch.
   8: aconst_null   
   9: aload_2       
  10: iconst_1      
  11: iconst_1      
  12: bastore       
  13: areturn       
  14: aload_1       
  15: ldc           #2                  // String yes
  17: invokevirtual #3                  // Method java/lang/String.equals:(Ljava/lang/Object;)Z
  20: ifeq          30
  23: ldc           #2                  // String yes
  25: aload_2       
  26: iconst_2      
  27: iconst_1      
  28: bastore       
  29: areturn       
  30: ldc           #4                  // String no
  32: aload_2       
  33: iconst_3      
  34: iconst_1      
  35: bastore       
  36: areturn       
LineNumberTable:
  line 6: 4
  line 7: 8
  line 10: 14
  line 11: 23
  line 13: 30
@marchof
Copy link
Member

marchof commented May 20, 2015

I just tested you snipped with JaCoCo: It does show two branches on the line in question.

  • What version of JaCoCo are you using?
  • What report are you using? Native JaCoCo? Any other tool?

Here some background information can be found how JaCoCo probes work: http://www.eclemma.org/jacoco/trunk/doc/flow.html

@marchof marchof added the feedback pending Further information is requested label May 20, 2015
@zhihan
Copy link
Author

zhihan commented May 20, 2015

Hi, Marc

I am using 0.7.4.201502262128
Could you let me know how you test it? I believe for the example we have
three probes inserted. And the line has only one probe associated with it.
I may have made a mistake in my analysis.

Zhi

On Wed, May 20, 2015 at 3:56 PM, Marc R. Hoffmann notifications@github.com
wrote:

I just tested you snipped with JaCoCo: It does show two branches on the
line in question.

  • What version of JaCoCo are you using?
  • What report are you using? Native JaCoCo? Any other tool?

Here some background information can be found how JaCoCo probes work:
http://www.eclemma.org/jacoco/trunk/doc/flow.html


Reply to this email directly or view it on GitHub
#313 (comment).


Zhi Han

@marchof
Copy link
Member

marchof commented May 20, 2015

I run your code snippet with EclEmma (the JaCoCo Eclipse plugin) and it shows two branches for the line.

How did you test???

@zhihan
Copy link
Author

zhihan commented May 20, 2015

I actually write my own code to use the jacoco core to report. So It might be that I have made a mistake.

But given the instrumented bytecode here, could you explain which probe should be associated with the two branches? In this example, there are three probes in total, all inserted before the return statements. The second branch at :20 has probe 2 (:28) and 3 (:35) associated with it. Probe 1 (:12) needs to be associated with the first branch (:4). But neither probe 2 or 3 should be associated with the branch at :4. I cannot see a right answer unless there is fourth probe inserted at :14.

@marchof
Copy link
Member

marchof commented May 20, 2015

If you use the API this expression should give you 2 for the respective line:

IClassCoverage cc = ...;
int totalBranches = cc.getLine(6).getBranchCounter().getTotalCount();

Probes always needs to be inserted before a return statement. See documentation: http://www.eclemma.org/jacoco/trunk/doc/flow.html

As is is the nature of RETURN and THROW statements to actually leave the method we add the probe right before these statements.

@marchof marchof closed this as completed May 20, 2015
@marchof marchof added declined: invalid ❌ and removed feedback pending Further information is requested labels May 20, 2015
@zhihan
Copy link
Author

zhihan commented May 20, 2015

Thanks. I see the issue now.
On May 20, 2015 6:06 PM, "Marc R. Hoffmann" notifications@github.com
wrote:

Closed #313 #313.


Reply to this email directly or view it on GitHub
#313 (comment).

@jacoco jacoco locked and limited conversation to collaborators Jan 11, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants