What targetting 100% code coverage taught me

Arseni Mourzenko
Founder and lead developer
October 14, 2022
Tags: testing 7 unit-testing 6 quality 35

I was re­cent­ly in charge of draft­ing a new sys­tem for a cus­tomer. I took it as an op­por­tu­ni­ty to demon­strate some of the ap­proach­es that should lead to high­er qual­i­ty soft­ware, com­pared to what the cus­tomer was ac­cus­tomed to. Among them was my tar­get to stay at 100% cov­er­age for both lines and branch­es dur­ing the en­tire de­vel­op­ment of the sys­tem.

I of­ten heard a mantra that 100% code cov­er­age is un­reach­able in prac­tice. That is, the time you spend on tests is as­ymp­tot­ic—the more you close on the tar­get, the more time you will waste. Es­sen­tial­ly, if you have to spend as much time to go from 99 to 99.9% as you spent cov­er­ing the first 99%, the cost be­comes pro­hib­i­tive, es­pe­cial­ly giv­en that the ben­e­fit is close to zero. I also heard—and reg­u­lar­ly told to oth­ers, as well—that there is code that doesn't de­serve tests, such as the get­ters and set­ters of a data ob­ject.

On the oth­er hand, TDD tells us that 100% cov­er­age not only makes sense, but is pret­ty much the only vi­able val­ue. Af­ter all, if you can't even test the code you write, why would you write it in the first place?

I did use TDD, but sparse­ly. While there were parts of the code which were pure log­ic, and so ide­al can­di­dates for TDD, most of the code was just glu­ing things around and not do­ing any­thing in­ter­est­ing to de­serve TDD. More­over, I wasn't al­ways sure how I would de­sign my code, and had found my­self quite of­ten throw­ing away code and re­do­ing it. When I wasn't con­fi­dent that I will keep the source code, I would wait be­fore adding tests.


First of all, the claim that the time one spends on tests is as­ymp­tot­ic is false. There are, in­deed, tricky cas­es. Cas­es where you have to scratch your head for sev­er­al hours to find a way to test that one re­main­ing branch. One ex­am­ple was a method that was re­ly­ing on the fol­low­ing prop­er­ty to be test­ed: public string CurrentUser => httpContext.User?.Identity?.Name. In ASP.NET Core, this is what gets the name of the cur­rent user, and there are three branch­es here, as the pre­vi­ous line of code is equiv­a­lent to:

public string CurrentUser
        var user = httpContext.User;
        if (user == null)
            return null;

        var identity = user.Identity;
        if (identity == null)
            return null;

        return identity.Name;

Test­ing that the cor­rect name is used is pret­ty much straight­for­ward. Sim­i­lar­ly, by cre­at­ing an emp­ty ClaimsPrincipal, one could test the branch where the iden­ti­ty is null. How­ev­er, the very first branch—the one where the user it­self is emp­ty—is tricky. In fact, the im­ple­men­ta­tion of DefaultHttpContext.User au­to­mat­i­cal­ly cre­ates a user if it wasn't spec­i­fied. This makes it nec­es­sary to cre­ate a cus­tom mock based on an ab­stract HttpContext, which would re­turn null when asked for a user, giv­en that it also re­quires to set­up a bunch of oth­er things that hap­pen to be used by the code be­ing test­ed—in my case the re­quest and re­sponse head­ers, and the body of the re­sponse.

While some of those cas­es are rather bor­ing to han­dle, oth­ers help un­der­stand­ing the in­ter­nals of .NET. One such thing is the can­cel­la­tion of an IAsyncEnumerable. I had a case which drove me crazy for a while, where the un­cov­ered line was the clos­ing curly brace:

if (token.IsCancellationRequested)
    if (ShouldIndicateCancellation)
        yield return -1;
    } // This line was marked as uncovered.

    yield break; // This line, however, was covered.

It ap­peared that the cov­er­age was giv­ing me a hint that there is some­thing I don't know, or rather don't un­der­stand, about how enu­mer­ables be­have. What hap­pened, in fact, is that this code was called through an­oth­er block, which was loop­ing through the el­e­ments of the se­quence, and check­ing whether the can­cel­la­tion was re­quest­ed. When this was the case, the code be­low would yield return -1. The caller would sim­ply stop the it­er­a­tion, which would mean that, in­deed, the clos­ing brace would nev­er be reached. As for the yield break, it was an­oth­er test—the one where ShouldIndicateCancellation was set to false, that would pass through the line, specif­i­cal­ly be­cause the yield return -1 would not ex­e­cute.

Far from be­ing an an­noy­ance, such edge case in­creas­es the un­der­stand­ing of .NET. In my case, it helped down the row to avoid a bug that would oth­er­wise be found only in pro­duc­tion, where can­cel­la­tions would not be logged prop­er­ly. It's not just the test­ing it­self, but the at­tempt to have a 100% cov­er­age that helped catch­ing the bug.

The as­ser­tion that there ex­ists some log­ic which is “too ba­sic” to de­serve some tests is equal­ly wrong. In fact, all data struc­tures are cov­ered au­to­mat­i­cal­ly as soon as you test the log­ic which re­lies on those data struc­tures. I mean, yes, it's not in­ter­est­ing to have ded­i­cat­ed tests for a struct Coordinate which con­tains two prop­er­ties, int X and int Y. But if you do have such struc­ture in your code, it means that, some­how, some­where, it is used. For in­stance, there may be code which com­putes a dis­tance be­tween two co­or­di­nates. As soon as you test this code—and you de­fin­i­tive­ly should test it—the get­ters and set­ters of the two prop­er­ties of the struc­ture will be test­ed as well.


100% code cov­er­age may be a tricky tar­get for lega­cy code, es­pe­cial­ly code that was nev­er test­ed, and there­fore prob­a­bly not de­signed to be test­ed in the first place. In this case, it could in­deed be smarter to tar­get some low­er tar­get: some­times 95%, some­times 90% or 80%. In some cas­es, even 20% would be dif­fi­cult to reach.

For new code, how­ev­er, it makes sense to tar­get 100% line and branch cov­er­age. The ad­di­tion­al cost to cov­er the re­main­ing 1% won't be the same as what was spent cov­er­ing the first 99%, and the ben­e­fit would be a bet­ter un­der­stand­ing of the sys­tem or the un­der­ly­ing lan­guage or frame­work by the de­vel­op­ers, and few­er bugs.

One have to be care­ful, how­ev­er, to have on the team one or few de­vel­op­ers who are both mo­ti­vat­ed and skill­ful enough that they will at­tempt to un­der­stand what's go­ing on when a few lines are un­cov­ered. Sim­i­lar­ly, one should strong­ly dis­cour­age de­vel­op­ers from mark­ing pieces of code as ig­nored for the code cov­er­age. While this is a valu­able tool in good hands, it could quick­ly be­come a “quick fix” tool for some­body who is not in­ter­est­ed in un­der­stand­ing what's go­ing on, and who just wants to fix that an­noy­ing er­ror that blocks the build un­til the giv­en cov­er­age is at­tained.

This also means that it doesn't seem rea­son­able to en­force 100% code cov­er­age dur­ing the build. Here, I'm pret­ty much un­sure what should be done. On one hand, I'm ab­solute­ly con­vinced that those things do be­long in the build, and that if the thresh­olds are not de­fined, any team can quick­ly drift away from the tar­get. On the oth­er hand, block­ing a build for hours or days be­cause of an ar­ti­fi­cial—yes, ar­ti­fi­cial—prob­lem doesn't seem to me as a good idea ei­ther. Pos­si­bly, an ac­tu­al so­lu­tion would vary from team to team.