Search code examples
javacode-coveragecomparatorjacoco

Code coverage for compare method not 100%

I have written custom comparator and test method to test it. But Whatever I do, I don't get 100% code coverage. It is not a big issue as I can live without 100% code coverage, but curious to know where I went wrong. I tried suggestion from some other question on stackoverflow of breaking comparator code on multiple lines like this but it didn't work. Also used & instead of && but no luck.

if (e1.getLastUpdateDateTime() == null 
&& e2.getLastUpdateDateTime() == null) 
return 0;

Code below for test method and comparator.

@Test
public void testCompare() {
    SomeObject m1 = new SomeObject();
    m1.setSomeVariable("a");
    m1.setLastUpdateDateTime(null);
    SomeObject m2 = new SomeObject();
    m2.setSomeVariable("b");
    m2.setLastUpdateDateTime(null);
    assertEquals(0, SomeUtil.compare(m1, m2));

    m1 = new SomeObject();
    m1.setSomeVariable("a");
    m1.setLastUpdateDateTime(null);
    m2 = new SomeObject();
    m2.setSomeVariable("b");
    m2.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:02"));
    assertEquals(1, SomeUtil.compare(m1, m2));

    m1 = new SomeObject();
    m1.setSomeVariable("a");
    m1.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:02"));
    m2 = new SomeObject();
    m2.setSomeVariable("b");
    m2.setLastUpdateDateTime(null);
    assertEquals(-1, SomeUtil.compare(m1, m2));

    m1 = new SomeObject();
    m1.setSomeVariable("a");
    m1.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:02"));
    m2 = new SomeObject();
    m2.setSomeVariable("b");
    m2.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:02"));
    assertEquals(0, SomeUtil.compare(m1, m2));

    m1 = new SomeObject();
    m1.setSomeVariable("a");
    m1.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:02"));
    m2 = new SomeObject();
    m2.setSomeVariable("b");
    m2.setLastUpdateDateTime(getXMLGregorianCalendar("2022-01-01 00:00:01"));
    assertEquals(-1, SomeUtil.compare(m1, m2));

    List<SomeObject> SomeObjectList = TestUtil.getSomeObjectListForComparator();
    SomeObject SomeObject = SomeObjectList.stream().sorted(SomeUtil::compare).findFirst().get();
    assertEquals("b", SomeObject.getSomeVariable());
}



public static int compare(SomeObject e1, SomeObject e2) {
    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() == null) return 0;
    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() != null) return 1;
    if (e1.getLastUpdateDateTime() != null && e2.getLastUpdateDateTime() == null) return -1;
    return e2.getLastUpdateDateTime().compare(e1.getLastUpdateDateTime());
}

Jacoco Code Coverage

enter image description here

Thanks @Godin. I have now reimplemented using below code.

public static int compare(SomeObject e1, SomeObject e2) {
    return e1.getLastUpdateDateTime() == null
            ? (e2.getLastUpdateDateTime() == null ? 0 : 1)
            : (e2.getLastUpdateDateTime() == null ? -1 : e2.getLastUpdateDateTime().compare(e1.getLastUpdateDateTime()));
}
Solution

  • For

        if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() == null) return 0;
    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() != null) return 1;
    if (e1.getLastUpdateDateTime() != null && e2.getLastUpdateDateTime() == null) return -1;

    in the last if-statement

        if (e1.getLastUpdateDateTime() != null && e2.getLastUpdateDateTime() == null) return -1;

    to reach part after &&

    e2.getLastUpdateDateTime() == null

    preceding part before &&

    e1.getLastUpdateDateTime() != null

    should evaluate to true, but it never evaluates to false (i.e. e1.getLastUpdateDateTime() == null), because all possible cases of e1.getLastUpdateDateTime() == null are already in the two previous if-statements

        if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() == null) return 0;
    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() != null) return 1;

    so there is no way to write a test that will not reach part after && in the last if-statement. This is so-called dead code - code that can never be executed and this is exactly what the code coverage tool tries to tell you, because the code coverage tool shows what was executed and what was not. To change the result of the code coverage tool you can rewrite your code in a way that avoids dead code, for example

        if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() == null) return 0;
    if (e1.getLastUpdateDateTime() == null && e2.getLastUpdateDateTime() != null) return 1;
    if (/* e1.getLastUpdateDateTime() != null && */ e2.getLastUpdateDateTime() == null) return -1;