The secret for 100% test coverage: remove code

Update note

Based on the interesting feedback I got (can be seen on Tom’s ramblings), I realized this post probably needed some tweaking and scope precision. I did put them at the end.

What is the adequate objective for test coverage?

60%?

80%?

99%?

100%?

Like many others, I have often pondered this question, like many before me, and many after I suppose. Why aiming for 100%? The 80/20 law clearly applies to test coverage: to try to cover every corner cases that lie in the code is going to require a significant investment in time and brain cells. Plus integration point can never really be properly covered.

On the other hand, having 100% coverage provides huge benefits:

  1. Every single line of code is constantly tested
  2. Trust in code is high
  3. Any uncovered line is a regression

What happens if the target is, 80%:

  1. Significant part of the code is never tested
  2. Trust in code is moderate and can degrade
  3. 20% uncovered line codes is significant, 2000 lines for a 10K code base. That means full namespaces can hide in there.

For me, there is no question 100% coverage is the only worthy objective, do not settle for less.

Yes there are exceptions, usually at integration points. Mocks are not a real solution either, they can help you increase your coverage but not by that much. The pragmatic solution is to wrap them into isolated modules (jars/assemblies). Think hexagonal architecture there. You will have specific coverage target for those, you also need to make sure that no other code creeps in and finally, understands those are weak points in your design.

While I am working on nFluent, I constantly make sure unit tests exert every single line of code of the library. It also means that I help contributors reach the target. It is not that difficult, especially if you do TDD!

There is one simple golden rule: to reach and maintain 100% coverage, you do not need to add tests, you have to remove not covered lines!

This is important, so let me restate that: a line of code that is not covered is not maintainable, must be seen as not working and must be removed!

Think about it:

  1. The fact that no automated test exists means that the behavior can be silently changed, not just the implementation!
  2. Any newcomer, including your proverbial future self, will have to guess the expected behavior !
  3. What will happen if the code get executed some day in production?
  4. If you are doing TDD you can safely assume the code is useless!

So, when you discover not covered lines, refactor to remove them or to make sure they are exerted. But do not add tests for the sake of coverage.

Having 100% coverage does not mean the code is bug free

Tom’s comments implied that I was somehow trying to promote this idea. 100% coverage is no bug free proof at all, and do not imply this at all. Quality and relevance of your tests are essential attributes; that is exactly why I promote removing non tested lines. Any specially crafted test will not be driven by an actual need and would be artificial. The net result would be a less agile code base.

On the other hand, if you have 100% coverage and you discover some reproducible bug, either by manual testing or in production, you should be able to add an automated test to prevent any re occurrence.

When coverage is insufficient, there is a high probability that you will not be able to add this test, keeping the door open for future regression!

If you want to build trust based on coverage metrics, you need to look into branch coverage and property based testing at the very least. But I do not think this is a smart objective.

Note

  • This post focuses  on new code! For legacy code, the approach should be to add tests before anything else, and never remove working code 🙂

Enlarge your TDD with NFluent

Full disclosure I am an active contributor to NFluent.

Imagine your first days on a new project that has a decent unit tests base (if not, change this or walk away).

You start coding away a fix or new feature, you extend the test base, you happily commit! Then the factory signals a failed build due to failing tests.

Well, that’s expected actually, unit tests are here as a safety net.

Bu then you realize that the test error message does not help. Neither taking a look at the test code, as the assertion’s syntax does not properly reflect the test intents.
Or maybe the previous developer failed to realize that the expected value comes first in Assert.AreEqual, or the comment has not been updated.

In the end, you have to debug the test to understand what is going wrong.

Image

This hassle fuels the naysayers that claim that unit tests are a cute idea but:

  • do not provided added value to the product
  • are expensive to write
  • are a burden in maintenance

They are basically right. Of course this is a short-sighted vision, but those are actual issues with many code base as of now, with the notable exception of OSS.

This is a serious issue that needs to be addressed.

Part of the problem comes from the fact that unit test tools have not significantly changed in the past decade, a time when the main challenges were being able to implement test runners and build testing infrastructures. Then there was interest in building more efficient test runners with the integration of multiple scenario for a single test, or the generation of variants. But no significant effort regarding the API.

Coming back to my earlier example, we, TDD craftsmen, need an API that allows us to be expressive on the assertion/checks we make. We want the IDE to help us, typically through Intellisense, we want to be able to add our own tests, we want to express conjoined test criteria as a single check and we want to ensure that newcomers (including our proverbial ‘future self’) to understand clearly what the test is about.

nFluent has been designed to answer those requirements as well as some others. It is on OSS assertion library that works on top of all unit test frameworks (nUnit, mbUnit, xUnit, even MSTest). You can start using now on any existing code base or new project. It is available through nFluent and it is guided by the brilliant T.Pierrain (@Tpierrain).

Check it: http://n-fluent.net/

Pattern: Immutable State

This an oxymoron: immutable state!

By essence, an object’s state keeps mutating, according to the side effects experienced by the object. On the other side, we have the immutable value object. You just can not change it, you can only get a different instance that represents another value. Mutable objects are the bane of concurrent programming, where value objects thrive. But immutability comes at a great cost: the loss of identity.

Whatever you try to do, the number 2 cannot change to 8. The physical world is the world of mutability, stuff keeps changing: objects move, living things grow, stars explode, even atoms have a life expectancy. But, if change happens slowly we can get a decent sense of immutability: a pictures, even if it had a life of its own, its content is immutable, people on the snapshot will not move!

Same for a book: the paper may go yellow, the story will remain the same, which is important if we plan to read it: how could anyone read a book if its content kept changing. Now leave the physical world and go back to the virtual one: the situation is similar, stuff keep changing but we need them to be stable (and consistent) as long as we need them to execute some tasks. The usual approach to that was mutual access exclusion: if you are the one and only one to access the entity you need, you have full control on what may happen to it. But, as I pointed out numerous times, this approach just fails in the long run, if not in the short. Just to stress one pitfall out, one can assume that reads of the entity will outnumber writes, which leads to useless contention: no protection is required when there is no active writer. The usual solution relies on so-called read-write locks, which allow many readers but just a single writer. Better, yes, but it has its own pitfalls:

  • it can lead to writer starvation (so many readers that you cannot write)
  • it triggers tricky deadlocks due to lock promotion (when a reader needs to write)
  • most implementations have disappointing performances

How to efficiently address read vs. write contentions? What if the readers and writers did not compete? The only remaining contentions would be among writers, as readers happily share! Then use an immutable state:

  • readers can get a valid state whenever they need
  • writers just generate to state instances, without altering the preexisting ones

 

Private and internal classes

This post is not going to be about concurrent programming. It has been triggered by a Twitter initiated chat regarding how to write unit tests for private methods. The general consensus was you just don’t: one must test public methods, private ones are an implementation details.
What about private classes then, did I ask. One of the feedback I got was: are you sure private/internal classes are needed in the general case?

I definitely think so, so let me describe some uses.

Internal classes

As a reminder, internal classes in C# are classes that are accessible only from the assembly they are defined in (and friend assemblies, if any). Declaring a class public is a contract with anyone who may use this library: you must implement a quality service for the long run. It implies you provide documentation of some sorts as well as ensure the class will be published for the versions to come.

For example, in RAFTiNG I have classes that are used to implement the behavior of the node according to its current state. Definitely implementation details, not something I want to publish.

Private classes

As a reminder, private classes can only exists as member of other classes in C#. They are useful to implement specific collaboration scenarios. Going on with RAFTiNG examples, in the ‘leader’ state, a node must track the synchronization point for each of the ‘Follower’ nodes.

Implementing it directly in the ‘LeaderState’ class implies building up an array of follower state and managing them one by one. Definitely not very OO: no encapsulation!

So, the proper way to do it is to implement a dedicated class for that, a very specialized class that is only useful in that very state.

See LogReplicationAgent member class here.

Note also that member classes can access private members from their hosting class, which offer interesting collaboration model.

The danger of microbenchmarking

Performance measurement is a hot topic

in IT, and it has always been so. How fast is the new hardware, which RDBMS has the highest TPS, which implementation has the lowest memory footprint…

Nowadays, pretty much anyone with a decent knowledge of a programming language can write some benchmark and publish his results, so benchmarks flourish everywhere.

Please read them with the colloquial grain of salt: measurement is hard and more often than not, the benchmark may include some bias favoring the author expectations. The bias will probably be accidental and of limited impact, but it will be there nonetheless.

But I wanted to discuss about microbenchmarking, with a focus on concurrency related ones.

Continue reading “The danger of microbenchmarking”