Hi all,
I've been looking at some tests we've been writing here today, and I think I've spotted a bit of an anti-pattern that I'd like to quickly draw out. In my experience, when I pick up existing unit tests there are three things I look at - what code is being exercised, do the tests pass when I run them, and crucially what is being asserted. Your assert is the one line of code that justifies te existene of the entire test. Getting this wrong can lead to a situation where even if you have 100% code coverage, you have no assurance that your code actually does anything useful at all.
The problem test in question:
public void MakeTea_WithMilk_TeaBagIsDunked()
{
var mockTeabag = new Mock<ITeabag>();
mockTeabag.Setup(x => x.Dunk()).Verifiable();
var teaProvider = new TeaProvider(mockTeabag.Object);
teaProvider.Provide();
mockTeabag.VerifyAll();
}
The first thing I'd recommend is the addition of the "Arrange", "Act" and "Assert" comments to break up the test (though I get that's probably a very personal preference!). But more importantly, what is being asserted against? What does "VerifyAll" tell us? Since our setup isn't setting up a response, and our mock behavious isn't set as strict, the same test can be written as follows and should be a lot clearer:
public void MakeTea_WithMilk_TeaBagIsDunked()
{
// Arrange
var mockTeabag = new Mock<ITeabag>();
// Act
var teaProvider = new TeaProvider(mockTeabag.Object);
teaProvider.Provide();
// Assert
mockTeabag.Verify(x => x.Dunk());
}
It's now obvious that the one thing this test is asserting is that the mock teabag has been "Dunk"ed. Under the hood, Moq is apparently listening to all interactions and simply storing them away, so that we can dig in after the event.
Caveat - I haven't tested the code above, so copy-and-pasting into your own beverage provision products is at your own risk! But the general principle is tested.
Russ
If you enjoyed this post, make sure you subscribe to my RSS feed!