views:

80

answers:

2

I've got several unittests of this pattern:

[TestMethod ()]
[ExpectedException (typeof (ArgumentNullException))]
public void DoStuffTest_Exception ()
{
    var foo = new Foo ();
    Foo.DoStuff (null);
}

It turns out that code coverage markes the throwing line as half-run, so I get 1 block of uncovered code each time.

After thinking about this problem for a while, the best solution I could come up with was adding a try/catch. Since this is a repeated pattern, I'm going to create a helper method along the lines of

public static void ExpectException<_T> (Action action) where _T: Exception
{
    try { action(); }
    catch (_T) { return; }
    Assert.Fail ("Expected " + _T);
}

This would have the nice side benefit that I could add all exception tests to the non-throwing tests.

Is this a valid design, or did I miss something?

Edit: Ugs... seems like the above ExpectException method leaves me with 1 uncovered block as well.

A: 

Yep this is pretty standard fare - a lot of our tests do the same. At the same time, you have to wonder if you're not placing too high a value on code coverage if those half-branches weigh in so much for it to be worth the effort.

Epaga
Currently at horrifying 35% coverage, so this does not add much. It's more of a minor design issue (that could happen to save a few hundred lines of test code).
mafutrct
+5  A: 

What you are suggesting is valid. Aside from you code coverage issue, I would argue it is better than using the ExpectedException attribute as it explicitly shows which line of the test is expected to throw the exception. Using ExpectedException means that any line of code in the test can throw the expected exception type and the test will still pass. If the error originates from another call that was not expected to throw, it can disguise the fact that the test should be failing because the line that should be throwing isn't.

What would be a useful modification to what you have proposed would be to return the caught exception:

public static _T ExpectException<_T> (Action action) where _T: Exception
{
    try { action(); }
    catch (_T ex) { return ex; }
    Assert.Fail ("Expected " + typeof(_T));
    return null;
}

This would enable the test code to further assert the exception if it desired (ie. to check a particular message was used).

NUnit (though it doesn't look like you are using it as you have a TestMethod attribute) has a built-in construct similar to what you have proposed:

Assert.Throws<ArgumentNullException>(() => Foo.DoStuff(null))
adrianbanks
+1. Good stuff. This has been nagging me for awhile but never got around to fixing it.
magnus