I want to achieve 100% test coverage, but there are always some unreachable branches. How can I make IDEA exclude these unreachable branches from coverage statistics? For example:
public static Class wrapper(Class cls) {
if (cls.isPrimitive()) {
if (Objects.equals(cls, boolean.class)) {
return Boolean.class;
}
if (Objects.equals(cls, byte.class)) {
return Byte.class;
}
if (Objects.equals(cls, short.class)) {
return Short.class;
}
if (Objects.equals(cls, char.class)) {
return Character.class;
}
if (Objects.equals(cls, int.class)) {
return Integer.class;
}
if (Objects.equals(cls, long.class)) {
return Long.class;
}
if (Objects.equals(cls, float.class)) {
return Float.class;
}
if (Objects.equals(cls, double.class)) {
return Double.class;
}
if (Objects.equals(cls, void.class)) {
return Void.class;
}
// Here is an unreachable branch in test coverage for now.
// But if a new primitive type added in new java version, here will be reachable.
throw new UnsupportedOperationException();
}
return cls;
}
There are 2 ways to attempt to define unreachable.
One is a formal definition; some algorithmic approach that tooling can ascertain.
Java already has this - it is baked into the java spec. For example, this is a compiler error (try it!):
void example() {
if (System.currentTimeMillis() < 0) {
System.out.println("A");
return;
} else {
System.out.println("B");
return;
}
System.out.println("C");
}
The compiler will refuse to compile this; any compiler that compiles this is broken (fails to adhere to the spec). It's a bit odd that javac has this small set of what sure feels like code style rules that nevertheless necessarily result in compilation failure, but, it is what it is. (It fails, because 'print C' is not reachable, in a formal way).
Hence, if you have an actually unreachable situation, you're in luck! There is no need to write that throw fallback statement, in fact, the compiler won't let you write it.
Clearly then, what you have written is not formally unreachable.
Which gets us to that other definition of unreachable. Let's call it Homer's definition: Duh, just.. look at it! Of course that line is unreachable!
But the problem is, you determined that. Your eyeballs, your brains. The compiler can't do the job.
So you'd have to tell the compiler; some way to mark the line as: Eh, don't worry about this. Which even exists depending on which code coverage tool you are using, generally with a magic comment. But that's just lying to yourself. It's marking your own homework. What's the point of 100% code coverage if you got there by marking off a bevy of lines as 'eh, I think this is unreachable so whatever, I'll mark it up, tada, 100% code coverage! I deserve myself a pat on the back'.
No, you don't. If that's how you feel this should go, just ditch your "requirement" for 100% code coverage instead, the end result is the same (namely, that you very much have not covered every line), but at least it's clear that you don't, actually, have that kinda coverage.
This leans into 'caveat D' below, but, here's a funky example of how we can get to formal unreachability so we no longer need that fallback throw statement:
enum Suit {
HEARTS, SPADES, CLUBS, DIAMONDS;
}
class Example {
boolean isRed(Suit suit) {
// A simple ternary operator is probably better;
// Just making a point in regards to switch-on-enum here.
return switch(suit) {
case HEARTS, DIAMONDS -> true;
case CLUBS, SPADES -> false;
};
}
}
Note a few interesting things here:
true or false, that's because here we use the switch as expression, which requires (and the lang spec confirms this) that the switch block must be complete.What's going on here is that the lang spec really does allow this. No need to add a third line with default -> throw new IllegalArgumentException("unknown suite: " + suite); - and now we are getting somewhere, because that line semantically matches what you've done in your code - it has the same property that if you were to add it, you would be incapable of covering that line 1.
This is one easy trick: Simply eliminate that 'we cannot actually get here' situation by using constructs that result in formal unreachability.2
And I don't mean mocks. Looking at the snippet up top, even if we remove C, how do we ensure we get code coverage on 'print B'? Mess with our system clock and set it back to before 1970? That sounds awkward.
No - System.currentTimeMillis() is fundamentally hostile to testing, so that code should not exist. Instead, ensure that a Clock object is injected and ask it for the current time. Normally this code would use the system clock, but during testing you can provide a clock that forces the year back to 1950 or whatnot, so you can test it. But, there are limits to how far you should take it.
"Current time" breaks down nicely into the concept of "... according to some clock, so we shall enshrine that concept of 'what clock are you using to determine current time' into our API". But not everything does; adding a 5th suit to our card game model solely for the purpose of representing an unknown suit is not a good idea. We are now actively fighting against what java wants us to do (if we want to switch on suit, the compiler will now dutifully force us to write either a default or case IMPOSSIBLE_SUIT: to cover it, there is no way to tell the compiler one of the enum options should be treated as non-existent as it is only there for testing). We are now dirtying up our API docs, creating additional questions about interactions (reasoning about all state our app could be in, we now have to reason about IMPOSSIBLE_SUIT if we add that).
But when it is sensible and the API supports it (such as java.time.Clock), use them.
Replace the whole thing with:
return cls.isPrimitive() ?
MethodType.methodType(cls).wrap().returnType() :
cls;
That's.. the only way to do it, really. Use solution A or B and if those don't work, find another way. These aren't automatable, and it cannot be guaranteed that 'another way' exists.
There are situations where it is not readily possible to use either solution A or B to ensure there is no need for a fallback line.
One 'simple' solution is to simply not have one, but that is a horrible idea. Objectively, the 'damage' a bug in your code does is in large part a function of how long it takes to figure out it exists PLUS how long it takes you to reproduce and then fix it.
An exception scores extremely well on this: You know immediately, and the trace is pointing you directly at both what happened and where to fix it.
In contrast, 'hmm, weird. The code silently does.. nothing' scores horribly on this. It'll take longer to realize you have a bug, the user experience is far worse (an error is vastly better than 'bizarre behaviour, as if the button does not work' or what not), and you spend literally 10 to 20x more, sometimes hundreds x more time to find and fix it.
Code coverage is a tool. Not a means unto itself. As with any tool, you use it when it is useful, and when the costs of using the tool outstrip the benefit of using the tool, there is only one available answer: Do not use the tool then.
Hence, if 'I want 100% coverage!' leads you to: "I will replace throw statements that serve as fallback to confirm unreachability with silently doing nothing", you've objectively just made a mistake. The cost of the tool '100% coverage' has now exceeded whatever benefit it gave you.
That leads to the extremely simple solution: Stop caring about 100% if this bothers you. Find ways to apply solutions A and B; usually if you do a good job designing your APIs you will find a way. In the specific code you pasted? There really isn't - this is one of the places where you're just going to fall flat.
The specific problem you've ran into is that designing APIs to enable 100% test coverage is an effort and has an effect on how you design them. The OpenJDK team is not on board. The core APIs aren't designed with that in mind (keep in mind that java is 25 years old and most of its API, including everything you are using in the snippet you posted, is well over 20 years old; test coverage was barely a concept at all. The concept of having tests that score 100% coverage did not exist whatsoever back then).
As a kludge, look into the docs of your coverage tool and mark it off as irrelevant, but, make no mistake, that's you marking your own homework. Such markers should be reviewed in triplicate if you want to continue to enjoy the benefits of 100% coverage.
100% code coverage doesn't prove your code is bug free. In fact, it proves.. nothing.
Goodhart's Law is relentless. If you enforce 100% coverage on a team that is not intrinsically motivated to write tests that cover all intended scenarios, using tooling to enforce 100% coverage will only hurt. There are trivial ways to cause a line to be covered but which doesn't at all test that line. In fact, even if you are trying your best, it is quite easy to get 100% coverage whilst failing to actually test much. Here is a trivial example:
String render(LocalDate date) {
return DateTimeFormatter.ofPattern("YYYY-MM-dd").format(date);
}
@Test void test() {
assertEquals("2024-09-05", render(LocalDate.of(2024, 9, 5)));
assertEquals("2024-01-01", render(LocalDate.of(2024, 1, 1)));
assertEquals("2023-12-31", render(LocalDate.of(2024, 12, 31)));
}
This will cause 100% coverage. And there is a pretty serious bug there. See it? No?
YYYY is the pattern for year-of-week. Not the actual year. The missing test is assertEquals("2022-01-01", render(LocalDate.of(2022, 1, 1))); which would fail (because the render method would incorrectly return 2021-01-01; 2024 so happens to be a lucky (or, should I say unlucky?) year where year-of-week and year-of-day happen to align perfectly, which is why the test cases as written don't expose the bug).
If that strikes you as unrealistically esoteric, let's make it even simpler:
boolean isPositive(int v) {
return v >= 0;
}
If I only test isPositive(20), the line gets 'covered', but did I actually cover all ways that can go? Of course not. We never actually tested a negative number and the coverage tool won't tell us. Yes, coverage tools can be configured to treat every boolean expression as requiring all ways through it must be hit by the test suite, but, what if instead you pass v to some method of java.lang.Math? Should the coverage tool be extended to require that you hit every line and all ways for all boolean expressions in the core code itself? Not realistic.
Thus, we get to a perhaps painful conclusion: 100% code coverage isn't 100%. It's a fine goal to go for high coverage, but, 100% is not magical. There really is nothing cool about 100% that isn't virtually just as cool with 99%. And, forcing high coverage or celebrating it puts you at severe risk of getting Goodharted. After all, simple logic:
It's merely highly suggestive that if you score high on coverage, that you probably did a good job. Unless, of course, you were optimizing for scoring as high as possible on coverage instead of optimizing for writing a test that, actually, you know, checks that the software does what you need it to do!.
[1] There are hacky libraries out there to at runtime kludge another value into an enum for the explicit purposes of actually testing this. This is a hack, it uses tricks that the JDK spec doesn't support, the JDK is heckbent on eliminating any such uses, and regardless of the practical implications of the damage you are doing to the code base with this, the entire exercise is misguided to say the least.
[2] Javac just syntax sugars that thing and adds default -> throw new MatchException();. You can trivially see that in action - write the enum in one source file, the switch in another, compile both. Then edit the enum source file and add another enum field, then compile only that file. Voila. It is now easy to cause that switch to fail to return something. You'll find that a MatchException is thrown. Presumably it's acceptable that this behaviour is not covered by your test cases and yet you still score 100% because the JDK is doing the work for you. But we did just put a requirement on; is someone managing that requirement, ensuring that we adhere to it, someone capable of ringing alarm bells if it fails? The JDK expects that you do not recompile the enum after adding a value without recompiling every source file that switches on it.