Why is JaCoCo not covering my String switch statements?

Question

I have a switch statement that extracts an addressing mode from a String and I've written unit tests to cover, what I thought was every eventuality but JaCoCo seems to skip my switch statements, resulting in lower coverage.

Why, if all my case statements, including a default are being executed in tests, would the switch statement not be counted as hit?

This issue might be relevant: https://github.com/jacoco/jacoco/issues/116. And its 4-year old parent too (https://github.com/jacoco/jacoco/issues/15) — default locale, Mar 07 '17 at 08:08
Another option is that you don't test for NPE if `t[OP_ADD]` is null. Relevant question: https://stackoverflow.com/questions/35288022/jacoco-coverage-for-switch-statement?rq=1 — default locale, Mar 07 '17 at 08:43
I'm not testing for `NullPointerException` though, I'm testing for `ArrayOutOfBoundsIndexException` as some instructions have an addressing mode, e.g. `OP_LDA_I` while some are immediately addressed e.g. `OP_SEC` — Ross Drew, Mar 07 '17 at 11:13
Technically, `t[OP_ADD]` can be null, in this case, NullPointerException will occur. To cover this case you'll need to add tests for null values, as far as I can see. — default locale, Mar 07 '17 at 11:17
Oh, you are saying I should test for null. But that would show up in the `if`, so why would that affect my `switch` coverage? — Ross Drew, Mar 07 '17 at 11:22

Godin · Accepted Answer · 2018-01-13T09:45:56.420

For the switch by String

class Fun  {
  static int fun(String s) {
    switch (s) {
      case "I":
        return 1;
      case "A":
        return 2;
      case "Z":
        return 3;
      case "ABS":
        return 4;
      case "IND":
        return 5;
      default:
        return 6;
    }
  }
}

Oracle Java compiler generates bytecode similar to the following code (Eclipse Compiler for Java generates slightly different bytecode)

    int c = -1;
    switch (s.hashCode()) {
      case 65: // +1 branch
        if (s.equals("I")) // +2 branches
          c = 0;
        break;
      case 73: // +1 branch
        if (s.equals("A")) // +2 branches
          c = 1;
        break;
      case 90: // +1 branch
        if (s.equals("Z")) // +2 branches
          c = 2;
        break;
      case 64594: // +1 branch
        if (s.equals("ABS")) // +2 branches
          c = 3;
        break;
      case 72639: // +1 branch
        if (s.equals("IND")) // +2 branches
          c = 4;
        break;
      default: // +1 branch
    }
    switch (c) {
      case 0: // +1 branch
        return 1;
      case 1: // +1 branch
        return 2;
      case 2: // +1 branch
        return 3;
      case 3: // +1 branch
        return 4;
      case 4: // +1 branch
        return 5;
      default: // +1 branch
        return 6;
    }

So that original switch-statement with 6 cases is represented in bytecode by a switch with 6 cases for hashCode of String plus 5 if-statements plus another switch with 6 cases. To see this bytecode you can use javap -c.

JaCoCo performs analysis of bytecode and in versions lower than 0.8.0 has no filter for switch by string. Your tests cover cases, where conditions in if-statements evaluate to true, but not the cases where they evaluate to false. Personally I would advise to simply ignore missing cases, because the goal is not to test that compiler generates proper code, but to test that your application behaves correctly. But for a sake of completeness of this answer - here is tests that cover all bytecode branches:

import org.junit.Test;
import static org.junit.Assert.*;

public class FunTest {
  @Test
  public void test() {
    // original strings:
    assertEquals(1, Fun.fun("I"));
    assertEquals(2, Fun.fun("A"));
    assertEquals(3, Fun.fun("Z"));
    assertEquals(4, Fun.fun("ABS"));
    assertEquals(5, Fun.fun("IND"));

    // same hash codes, but different strings:
    assertEquals(6, Fun.fun("\0I"));
    assertEquals(6, Fun.fun("\0A"));
    assertEquals(6, Fun.fun("\0Z"));
    assertEquals(6, Fun.fun("\0ABS"));
    assertEquals(6, Fun.fun("\0IND"));

    // distinct hash code to cover default cases of switches
    assertEquals(6, Fun.fun(""));
  }
}

And report generated by JaCoCo 0.7.9 as a proof:

JaCoCo version 0.8.0 provides filters, including filter for bytecode that javac produces for switch by string. And so generates following report even without additional tests:

So, it's find `Strings` that have the same hascode as all my case statement `String`s or grin and bare it? :( Thanks — Ross Drew, Mar 08 '17 at 20:19
@RossDrew personally I would advise to simply ignore missing cases — Godin, Mar 09 '17 at 08:28
I accepted this answer until I wrote test cases to cover it. I added tests that check each of my `switch` cases against cases where the `hashcode()` is the same but `equals()` is different. Which should, check the nested `if` but the code coverage remains the same. https://github.com/rossdrew/emuRox/blob/master/src/test/groovy/com/rox/emu/p6502/op/OpCodeConverterSpec.groovy — Ross Drew, Mar 09 '17 at 10:39
@RossDrew "accepted this answer" - maybe you upvoted, but definitely not accepted. "but the code coverage remains the same" - I guess that you don't test "default" case of switch statements. Answer was updated to be more precise and include tests. But once again - IMO this is overkill. — Godin, Mar 09 '17 at 17:11
I accepted it then took it back again. It is overkill but I like my 100% badge and if it leads to learning more about my language and tools, it's all a good thing :) As for the default test case not being tested. The code coverage says 16 hits in there. — Ross Drew, Mar 09 '17 at 18:35
@RossDrew addition of debug output `if (t[OP_ADD].startsWith("\0")) System.out.println("TESTED");` just before the switch tells me that you don't actually test cases of the same hash code. You can also confirm this with help of debugger. In your tests `\0` should be placed before `I`, `A`, `Z`, `ABS` and `IND` as in my answer, not before `OP`, i.e. `OP_ORA_\0ABS` and etc. — Godin, Mar 09 '17 at 19:29
@Godin, would you know why such code is generated ? Is that an optimization trick? What would be the reason? — Baptiste Pernet, Aug 29 '19 at 19:34
@BaptistePernet see JSR-334 (https://jcp.org/aboutJava/communityprocess/final/jsr334/index.html) "In order to keep strings in switch a small language change, the JVM lookupswitch and tableswitch instructions in Java SE 7 do not support switching on a string. Instead, Java compilers are responsible for translating a switch on strings into some sequence of byte code instructions with the proper semantics. Many valid and efficient translation schemes are possible that have better expected performance than successive comparisons of the string being switched on with each case label constant." — Godin, Aug 29 '19 at 21:01
I got around this by making a map from String to return value. — Ozymandias, Oct 24 '19 at 23:38
This drove me crazy for hours. I had a similar issue with at `when` statement. I was very confused as to why I wasn't getting full coverage, i thought I had written my tests wrong. Updating from 0.8.3 to 0.8.6 fixed it. Thank you @Godin — Nelson Ramirez, Nov 29 '20 at 02:47

Why is JaCoCo not covering my String switch statements?

1 Answers1

Linked