Over Specification in Tests
I've gotten the chance to visit and consult various companies and I see this all the time: Over specification in tests, especially when doing mocks and stubs.
One main reason for that is that people use their stub as a mock object (stubs are fake objects that we will not assert on so that we can test something else). here is a small example from something posted on the alt.net mailing list:
[Test]
public void Can_Return_All_Plans()
{
IPlanDao mockPlanDao = MockRepository.GenerateMock<IPlanDao>();
mockPlanDao.Expect(x => x.AllPlans(_fakeAdmin)).Return(new List<Plan>{_fakePlan});
PlanController planController = new PlanControllerForTesting(_mockCommonDao, _mockCommonService, mockPlanDao);
ViewResult result = planController.All(false);
mockPlanDao.VerifyAllExpectations();
result.AssertViewResultNameAndViewDataType(typeof(PlanContainer), "Plans");
}
in this test the following line is used to "stub" out a return value into the system under test.
mockPlanDao.Expect(x => x.AllPlans(_fakeAdmin)).Return(new List<Plan>{_fakePlan});
no problem here. The problem lies in the last two lines of the test:
mockPlanDao.VerifyAllExpectations();
result.AssertViewResultNameAndViewDataType(typeof(PlanContainer), "Plans");
the second line (the assert) is probably the thing you really want to test. but the call to "verifyAllExpectations" is the one that makes the test very fragile. It is "asserting" that someone has actually called "GetAllPlans()" to get the results, when it clearly does not matter how one got the results. all that matters is that:
"Given a set of results returned into the application under test, the view in the controller is correct" (the translation of the last line in the test).
If by any chance the application later does several more calls to the stubbed out dao, or uses a different method to get results, the test could break, even though the application would still ultimately work with the same end result.
It's like me ordering a pizza delivery and then asking the delivery guy "did you get here by car or motorcycle?" . I realy don't care, as long as the end result: PIZZA, is correct.
So the general rules for me are:
- If you see both "Verify" and "Assert" in the same test, it is usualy a smell of over specified tests
- if you see "expect" and "Return" on a fake object, make sure that you name it "stubXX" or "mockXX" so that you can distinguish whether you want to call verify on it later or not(most cases should be "not")
- Try to test on the end result or end state rather than verify interactions. The only time you absolutely have not choice but to test an interaction using verify is when calling void methods on external objects. that is clearly a one-way communication and is (or part of)the end result of what you are trying to accomplish.
A lot of people think that adding that extra "verify" just means it is a good thing since they are doing more assert. Well, they sure are testing more things, but they are internal things to the app's behavior and that is usually leading to brittle tests. Try to test on the end result or end state rather than verify interactions.