tags:

views:

81

answers:

4

Hi guys,

I'm really getting to TDD recently, and after reading Kent Beck's book on Test Driven Development, I've still got many questions around test design in my mind.

One of the issues I'm currently having is the use of Mock objects. Take below a very simple that generates a report:

public string MakeFinancialReport()
    {
        return sys1.GetData() + sys2.GetData() + sys3.GetData();
    }

The report must contain a header, a body, and a footer. So a quick test to see if those titles exist in the report:

public void TestReport()
    {
        string report = MakeFinancialReport();
        Assert.IsTrue(report.Contains("[Title]") && report.Contains("[Body]") && report.Contains("[Footer]"));
 }

To isolate the method, I guess I would be mocking away the sys1, sys2 and sys3 calls. Now, if they are all mocks, what have I got left to test? Also, when I do mock them, why must I have tell the mock objects that they are going to be expected to be called once and return X etc. Should it not just be a black box test and the MakeFinancialReport can make as many calls as it wants to build the report?

I'm getting confused with such a little problem, I'm not sure what I'm missing. I see Mocking as taking away testable code, and for most simple methods, what is left to test isn't helpful at all.

+1  A: 

You should only use mock objects when they're helpful. If MakeFinancialReport, sys1, sys2, and sys3 all have complicated logic in them, then you'd want to test each of them independently. By giving mock versions of the three sysX objects to MakeFinancialReport you remove their complexity and just test MakeFinancialReport.

Mock objects are particularly helpful when you want to test error conditions and exception handling, because it might be hard to force an exception from the real object.

When you talk about not having to set explicit expectations and return values, that's a related concept called stubs. You might find Martin Fowler's "Mocks Aren't Stubs" helpful.

Kent Beck's book is a great introduction, but if you're looking for more detail, I highly recommend the xUnit Patterns book. For example, it has a section on mock objects, as well as the more general category of test doubles.

Don Kirkby
Hmm, then I think I'm missing test design principals somewhere. I thought a unit test was to test that one method, that small piece of discrete code, not any of its collaborating objects. The collaborating objects in this case, sys1, sys2 and sys3, each have their own set of unit tests are covered in seperate units tests.The Martin Fowler article I've read many times and from what I can take from the differences of Mocks and Stubs are the differences between state and behaviour style testing.
Martin
I still need to understand that why we use them is because we are trying to move away the complexity of calling code outside of a method/unit so we can focus on the behaviour of the one method. So in this case, what exactly is left in th eMakeFinancialReport() method if we have taken away the 3 system calls via mocks?
Martin
In your example, @Martin, all that's left is string concatenation. That's not much to test. If there's nothing to test, then that's a code smell that would nudge you toward the "Inline Method" refactoring. Perhaps you should look for a better example.
Don Kirkby
Here's a related question: http://stackoverflow.com/questions/1414032/why-create-mock-objects
Don Kirkby
A poor example I agree, but the best I could come up with for now ;) Are you suggesting that the code smell is that the method is too simple, and not worthy of testing?
Martin
Yes, @Martin, the method is too simple. You could just decide it's not worthy of testing, but I'm going one step further to say, "If it's not worthy of testing, then why have it?" The "Inline Method" refactoring cuts that trivial code out of the method and pastes it in all places that call the method. Then delete the method. Sometimes there are reasons to have a trivial method, like encapsulation, but it should be a deliberate choice, not an accident. This has probably strayed from your original question, sorry about that.
Don Kirkby
Typically, you use a mock when the behavior of the class you are testing can be defined in terms of calls it will make to an appropriate interface. This is *generally* only possible if you're using appropriate abstractions, in other words, if the semantics of the interface match your domain closely enough that you can strongly assert correct behavior.
kyoryu
+2  A: 

As it stands, MakeFinancialReport barely does anything apart from interacting with more interesting collaborators and probably isn't worth unit testing.

If I wrote any unit tests for that method, I'd probably just verify that the method does what I expect when its collaborators return null, mainly to document the expected behavior (or to help me decide on the expected behavior if I do it in advance). Currently the method would just fail. That might be fine, but it's worth considering whether you want to treat nulls as empty strings - and unit tests prove that whatever behavior you decide on is intentional.

"Is specifying mock object behaviour white box testing?" Absolutely - if your class has a dependency that you're mocking, you're tying your test to that dependency. But white-box testing has its advantages. Not all collaborator interactions are as trivial as the ones in your example.

Jeff Sternal
A: 

I think one of the issues is that your test mixes the responsibilities from sys1, sys2 and sys3 with that of the TestReport method. It seems to me that what you should separate your tests into 2 parts:

1) MakeFinancialReport() returns the concatenation of the sys1, sys2, sys3. There you can stub sys1, etc..., with something along the lines of

var sys1 =MockRepository.GenerateStub<ISys>();
sys1.Expect(s=>s.GetData()).Return("Part 1");
// etc... for sys2, sys3 var
reportMaker = new ReportMaker(sys1,sys2, sys3); 
Assert.AreEqual("Part 1" + "Part 2" + "Part 3", reportMaker.MakeFinancialReport();

The class that owns the MakeFinancialReport() method shouldn't care or know what the sys classes are doing. They could return any class - the MakeFinancialReport() just concatenates, that's what you should test (if you deem it worth it).

2) Test the GetData() method from the interface sys1, sys2, sys3 implement. This is likely where you would check in what circumstances you would expect to see "Body", "Title", etc...
Stubbing might be overkill here, but what this buys you is a cheap instantiation of a potentially heavy dependency (the 3 sys instances), and a clear separation of what sys does, and what MakeFinancialReport does.

As an aside, it might be because of the language you are using, but it's surprising that your test doesn't begin with the instantiation of the class that owns MakeFinancialReport().

Mathias
+2  A: 

Martin, I think you should use mocks for sys1-3, but they only need to be simple enough to return a single character string each.

This means your test should look like:

public void TestReport()
{
    // Setup mocks for sys1-3
    string report = MakeFinancialReport();
    Assert.IsTrue(report.equals("abc"));
}

This shows that MakeFinancialReport has the properties that it calls GetData() from sys1-3 and it concatenates the results in this particular order.

quamrana
+1 Great answer. If everyone used mocks this clearly, they would be much more widely accepted.
Carl Manaster