Test coverage for if statement with logical or (||) - with Java's short circuiting, what's the forth condition JaCoCo wants me to cover?

Question

This is probably a rather simple question, but I'm at a loss...

I have an if statement like the following:

if(TheEnum.A.equals(myEnum) || TheEnum.B.equals(myEnum))

TheEnum can be A, B, C, ... G (more than just 4 options).

JaCoCo (SONAR) tells me that there are four conditions I can cover here. Which ones are those? Isn't the entire set I can test for in this instance essentially

if(true || not_evaluated) => true
if(false || true) => true
if(false || false) => false

I'm pretty sure I can't specifically test for if(true || true) or if(true || false), as short circuit evaluation won't get that far...?

If so, what is the forth option JaCoCo/Sonar wants me to test for?

Traditionalism

Try the non-shortcircuiting | to see what JaCoCo says to that. – Promptbook

I find this very puzzling behavior. To me it's actually not that short-circuit won't let you test both (true, true) and (true, false) cases - it's that the case (true, true) cannot exist. If myEnum is A, it is not B. I can only see three cases in the given snippet of code. myEnum must be one of (A, B, not-A-or-B). That's three cases. – Multimillionaire

Answer 1

You are right, this code is short-circuiting. It's compiled into bytecode roughly like this (assuming Java has goto):

if(TheEnum.A.equals(myEnum)) goto ok;
if(!TheEnum.B.equals(myEnum)) goto end;
ok:
   // body of if statement
end:

So as JaCoCo analyzes the bytecode, from its point of view you have the two independent checks: first if and second if, which generate four possible branches. You may consider this as a JaCoCo bug, but I guess it's not very easy to fix this robustly and it is not very disturbing, so you can live with it.

Resupine

Well, it reduces coverage. And with a 95% target and a few of these in a short class, it kind of does matter... ;) – Traditionalism

@Traditionalism time to explain your manager that high coverage does not mean high quality, then ;) – Benzedrine

@Christian: there are many cases when you cannot get 100% coverage. For example, asserts inside methods. Or catch(CloneNotSupportedException ex) in clone() method. On the other hand 100% coverage is not a silver bullet. History knows many cool bugs which were unnoticed despite the 100% coverage. E. g. calculating an average with (a+b)/2. It's simply to cover it, it has no branches, but if you don't test it on numbers higher than Integer.MAX_VALUE/2 you won't notice a bug. – Resupine

Answer 2

If the 100% score matters more than anything else... ... use this approach:

if (Boolean.logicalOr(TheEnum.A.equals(myEnum), TheEnum.B.equals(myEnum))) {
    ...

Or use the binary or

if (TheEnum.A.equals(myEnum) | TheEnum.B.equals(myEnum)) {
    ...

You may nest the calls as deep as you like, or use an own or function:

if (or(TheEnum.A.equals(myEnum), TheEnum.B.equals(myEnum))) {
    ...

public static boolean or(Boolean... vals) {
    for (Boolean v : vals) {
        if (Boolean.TRUE.equals(v))
        return true;
    }
    return false;
}

Using a standalone testable function or will test 100% of it.

Note that this may affect the performance... I told you! Also hiding all these decisions may backfire!

Highspirited