Dealing with Code Un-Coverable by Tests

Code UnCoverable by Tests

Even for test-coverage addict (as me), there is some code that simply cannot be covered by tests. An example?

The call to MessageBox.Show() cannot be tested automatically since it is blocking. Of course we could mock calls to MessageBox.Show(), but at the end of the day, there will be at least one call to this method in the code base, one call that cannot be covered by tests. 

This example is not isolated. There are a multitude of cases where code reacts to things that cannot be automatized, such as exception that cannot be reproduced…

…but also some environment settings, folder or file browsing by user, showing a modal custom dialog and more.

The Problem

As we explained above, while using mocking can help reduce the surface of code not-coverable by tests, it can’t help to get a code base 100% covered. When exploring test coverage results, with NCoverExplorer, Visual Studio Team System or NDepend, this problem will lead to classes 98% or so covered. It is taking time and it is error prone to figured out each time you are exploring coverage results, that these are false alert. This is clearly anti-agile! What we want is a clean 100% coverage result!

The Solution

NCover and NDepend are tackling this problem the same way: you can exclude some methods from coverage results through a dedicated (and eventually custom) attribute.

NCover knows about this attribute through the /ea command line option as describe here by Jamie Cansdale. Code tagged with this attribute will be excluded from coverage statistics and won’t disturb while exploring test coverage results. For example, if all methods of the class Foo are 100% covered except the method MessageBoxShow(), if the method MessageBoxShow() is tagged with the coverage exclude attribute then the class Foo will be shown as 100% covered. A great side-effect is that developers reviewing the code will know about this coverage exclusion.

With NDepend, the coverage excluding attribute can be provided through the import coverage file dialog as shown below. This will have the same effect as with NCover. For convenience, the assembly NDepend.CQL.dll can be linked from your project. This assembly contains the dedicated attribute: NDepend.CQL.UncoverableByTestAttribute.

If you prefer you can provide your own attribute.

As far as I know, Visual Studio team System Coverage doesn’t support yet this feature but it might be considered for future releases, as explained here.

The Bonus

Actually the burden of exploring coverage results to make sure that some classes or namespaces should be 100% covered is not really agile. It is manual, time consuming, error-prone and cannot be capitalized for future iterations. 

NDepend can help automate this task with just 2 CQL rules:

// <Name>Types 100% covered by tests</Name>
WARN IF Count > 0 IN SELECT TYPES WHERE 
HasAttribute "NDepend.CQL.FullCoveredAttribute" AND PercentageCoverage < 100
// <Name>Methods 100% covered by tests</Name>
WARN IF Count > 0 IN SELECT METHODS WHERE 
HasAttribute "NDepend.CQL.FullCoveredAttribute" AND PercentageCoverage < 100

Basically, these rules will check automatically that all types and methods tagged with the attribute FullCoveredAttribute are indeed 100% covered. Insert these rules in your build process and you’ll be informed as soon as a lack of test coverage is luring. Here also, as a bonus side-effect developer reviewing or refactoring the code will know instantly which part of the code is supposed to be 100% covered.  For applying this trick to the code base of NDepend since the code coverage feature is available, I can testify that it is a really, really time saving trick.

For convenience, the assembly NDepend.CQL.dll can be linked from your project and it contains the dedicated attribute: NDepend.CQL.FullCoveredAttribute (and also the attribute NDepend.CQL.MoreThan95PercentCoveredAttribute).

Of course, CQL rules can be readily tweaked to work with a custom attribute.

This entry was posted in Code Coverage, Code metrics, CQL, CQLinq, LoC, Maintainability, measurement. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://runefs.wordpress.com Rune FS

    I must admit it always make me wonder when I see the term “untestable code”. There is no such thing. How ever the expected cost of testing should always be compared against the expected cost of not testing.

    It’s possible to write a unit test that “clicks” the ok button of the MessageBox or you could have the unit test “press” enter, so no mucking needed to test that method. I’m not saying I would do it but just stating that it’s testable.

    So the decision I think should not be based on “is this code testable?” (the answer is alwyas “yes”) but instead “When concidering cost benifits does it make sense to test this code?” if the answer to that is no, the I like the idea of being able to remove it from the coverage reports.

    Concidering when and what to test should be something we do conciously that goes for the above examples, for when we choose our test data and when we choose which execution paths to run while testing

  • Brian Johnston

    Writing mocks for message boxes and copying to clipboards is just insane. That’s building in a lot of extra moving parts just for the purpose of testing.

    I’ve read several books, but one of my favorites is the Pragmatic Programmer, and Pragmatic Unit Testing – the ‘spirit’ of these two books is to do what make sense. If it makes sense for your company to spend thousands of dollars to initially generate, build upon, and maintain all those silly mocks for a message box or a paste command, then go for it – and thousands is accurate…how much do you think you cost the company per hour [it's more than just cash], and then those other people who in the future who have to build upon and maintain those mocks/tests?

    I think any *good* manager/team-lead/whatever is going to look at the cost/risk analysis and say 2% is acceptable *if* that 2% can be accurately traced to inert code such as showing message boxes – and this, I think, is were the attributes come in. One could quickly find these ‘hiding’ bugs and manually code review them to make sure there’s nothing ‘standing out’ at them rather quickly by search for the attributes.

    My point is, is mocking is nice, it’s great ‘theoritically’, and it has it’s place in *parts* of the real world, but it’s not a silver bullet like some out there might pitch – it’s okay to *not mock* something – seriously, people lived without them for years. Like all people who try to see everything as a nail – you’re eventually going to get bit by it…you have to be ‘pragmatic’ about what you test, use mocks where they make sense, use something else, like these attributes where they make sense; right tool-right job.

  • http://www.NDepend.com Patrick Smacchia

    James, I don’t agree.

    First, we do the opposote of hidding these uncoverable cases. We properly tag them with some dedicated attributes. Thanks to these attributes, they will be very easy to find in the mass of the code base in the future.

    Second, faked 100% coverage is a real convenience when it comes to analyze what is well tested and what is not. At a point in the past, we decided for example that call MessageBox.Show are untestable or must be mocked. We wrote proper CQL rules and we can move forward without being bothered by this special case again and again.

  • http://blog.jagregory.com James Gregory

    I think hiding these last few percent is just a way to make us feel better. Those areas are untested, regardless of whether they’re untested because we didn’t write tests for them, or because our tools are incapable of testing it.

    By hiding these bits away, they’re liable to be forgotten when our tooling eventually becomes capable of testing these areas. I’d rather have them there nagging at me.

    I don’t see a faked 100% the same as a real 100%. Even if you hide those last two percent, they’re still there, un-tested.

  • http://www.NDepend.com Patrick Smacchia

    Steve and Joshka,
    as said in the post, you can always use a mock and isolate un-coverable code into a special class namespace or assembly.

    But first it takes energy to mock all this + to mock also in the test code, second it is a bit awkard to group code with different concern, just because it is uncoverable.

    Mock or not to mock is a matter of judgment.

    > I can happily accept 98% as coverage because it is implicit to me that there is always untestable code.

    The point is how can you differenciate at a glance that the 2% doesn’t come from a developer that forget to test cover its changes? And also, why don’t prefer checking for the 100% coverage automatically?

  • http://bitfed.com joshka

    In the message box example, the problem is that your code is tightly coupled to the MessageBox implementation. If instead you provide the following:

    class MyDecoupledClass
    {
    public IMessageBox messageBox = new MessageBoxWrapper();

    public bool MessageBoxToDemandQueryDeletion(ICQLQuery query) {
    string unused;
    Debug.Assert(CanDeleteQuery(query, out unused));
    return messageBox.Show(
    “Are you sure that you want to delete the query: {” +
    query.QueryString + “} ?”,
    “NDepend”, MessageBoxButtons.YesNo) == DialogResult.Yes;
    }

    }

    interface IMessageBox
    {
    void Show(string msg, string title, MessagBoxButtons buttons);
    }

    class MessageBoxWrapper : IMessageBox
    {
    void Show(string msg, string title, MessagBoxButtons buttons)
    {
    MessageBox.Show(msg, title, buttons);
    }
    }

    You can now mock / fake the IMessageBox. The clipboard example is much the same.

    This technique of decoupling your code takes just as much time to create as your NDepend option and provides a much better decoupled system.

  • http://www.mostlyclean.com Steve Burman

    Patrick,

    To me it feels a bit dirty slapping an attribute on impossible-to-test code. Is this going to far to ensure the magic 100? I can happily accept 98% as coverage because it is implicit to me that there is always untestable code.

    However I can see some benefit in explicitly ‘calling-out’ the offending code blocks.

    Rather than being attribute based, has any consideration been given to being able to separate out the application of these rules.

    Say, for example, in an infrastructure assembly you could have a class dedicated to maintaining references to these types of code blocks that you want excluded from coverage results. Or maybe something configuration-based? Maintenance could get tricky in that instance though.