How to keep my unit tests DRY when mocking doesn't work?

Question

Edit:

It seems that by trying to provide some solutions to my own problem I blurred the whole problem. So I'm modifying the question little bit.

Suppose I have this class:

public class ProtocolMessage : IMessage
{
    public IHeader GetProtocolHeader(string name)
    {
        // Do some logic here including returning null 
        // and throw exception in some cases

        return header;
    }

    public string GetProtocolHeaderValue(string name)
    {
        IHeader header = GetProtocolHeader(name);

        // Do some logic here including returning null
        // and throw exception in some cases

        return value;
    }
}

It is actually not important what's going on in these methods. The important is that I have multiple unit tests to cover GetProtocolHeader method covering all situations (returning correct header, null or exception) and now I'm writing unit tests for GetProtocolHeaderValue.

If GetProtocolHeaderValue would be dependent on external dependency I would be able to mock it and inject it (I'm using Moq + NUnit). Then my unit test would just test expectation that external dependency was called and returned expected value. The external dependency would be tested by its own unit test and I would be done but how to correctly proceed in this example where method is not external dependency?

Clarification of the problem:

I believe my test suite for GetProtocolHeaderValue must test situation where GetProtocolHeader returns header, null or exception. So the main question is: Should I write tests where GetProtocolHeader will be really executed (some tests will be duplicated because they will test same code as tests for GetProtocolHeader itself) or should I use mocking approach described by @adrift and @Eric Nicholson where I will not run real GetProtoclHeader but just configure mock to return header, null or exception when this method is called?

Answer 1

It's all a matter of oppinion, pure tdd-ists will say no mocks, mockers will mock it all.

In my honest oppinion there is something wrong with the code you wrote, the GetProtocolHeader seems important enough not to be discarded as an implementation detail, as you defined it public.

The problem here lies within the second method GetProtocolHeaderValue that does not have the possibility to use an existing instance of IHeader

I would suggest a GetValue(string name) on IHeader interface

Answer 2

Try the simplest thing that might work.

• If the real GetProtocolHeader() implementation is quick and easy to control (e.g. to simulate header, null and exception cases), just use it.
• If not (i.e. either the real implementation is time-consuming or you can easily simulate the 3 cases), then look at redesigning such that the constraints are eased.

I refrain from using Mocks unless absolutely required (e.g. file/network/external dependency), but as you may know this is just a personal choice not a rule. Ensure that the choice is worth the extra cognitive overhead (drop in readability) of the test.

Answer 3

In the call to GetProtocolHeaderValue, do you actually need to know whether or not it called GetProtocolHeader?

Surely it is enough to know that it is getting the correct value from the correct header. How it actually got it is irrelevant to the unit test.

You are testing units of functionality, the unit of functionality of GetProtocolHeaderValue is whether it returns the expected value, given a header name.

It is true that you may wish to guard against inappropriate caching or cross-contamination or fetching the value from a different header, but I don't think that testing that it has called GetProtocolHeader is the best way to do this. You can infer that it somehow fetched the right header from the fact that it returned the expected value for the header.

As long as you craft your tests and test data in such a way as to ensure that duplicate headers don't mask errors, then all should be well.

EDIT for updated question:

1. If GetProtocolHeader works quickly, reliably and is idempotent, then I still believe that there is no need to mock it. A shortfall in any of those three aspects is (IMO) the principal reason for mocking.

   If (as I suspect from the question title), the reason you wish to mock it is that the preamble required to set up an appropriate state to return a real value is too verbose, and you'd rather not repeat it across the two tests, why not do it in the setup phase?

2. One of the roles performed by good unit tests is documentation.

   If someone wishes to know how to use your class, they can examine the tests, and possibly copy and alter the test code to fit their purpose. This becomes difficult if the real idiom of usage has been obscured by the creation and injection of mocks.

3. Mocks can obscure potential bugs.

   Let's say that GetProtocolHeader throws an exception if name is empty. You create a mock accordingly, and ensure that GetProtocolHeaderValue handles that exception appropriately. Later, you decide that GetProtocolHeader should return null for an empty name. If you forget to update your mock, GetProtocolHeaderValue("") will now behave differently in real life vs. the test suite.

4. Mocking might present an advantage if the mock is less verbose than the setup, but give the above points due consideration first.

   Though you give three different GetProtocolHeader responses (header, null or exception) that GetProtocolHeaderValue needs to test, I imagine that the first one is likely to be "a range of headers". (e.g. What does it do with a header that is present, but empty? How does it treat leading and trailing whitespace? What about non-ASCII chars? Numbers?). If the setup for all of these is exceptionally verbose, it might be better to mock.

Answer 4

Why not simply change GetProtocolHeaderValue(string name) so that it calls 2 methods, the second one accepting a IHeader? That way, you can test all the // do some logic part in a separate test, via the RetrieveHeaderValue method, without having to worry about Mocks. Something like:

public string GetProtocolHeaderValue(string name)
{
   IHeader header = GetProtocolHeader(name);
   return RetrieveHeaderValue(IHeader header);
}

now you can test both parts of GetProtocolHeaderValue fairly easily. Now you still have the same problem testing that method, but the amount of logic in it has been reduced to a minimum.

Following the same line of thinking, these methods could be extracted in a IHeaderParser interface, and the GetProtocol methods would take in a IHeaderParser, which would be trivial to test/mock.

public string GetProtocolHeaderValue(string name, IHeaderParser parser)
{
    IHeader header = parser.GetProtocolHeader(name);
    return parser.HeaderValue(IHeader header);
}

Answer 5

With Moq, you can use CallBase to do the equivalent of a partial mock in Rhino Mocks.

In your example, change GetProtocolHeader to virtual, then create a mock of ProtocolMessage, setting CallBase = true. You can then setup the GetProtocolHeader method as you wish, and have your base class functionality of GetProtocolHeaderValue called.

See the Customizing Mock Behavior section of the moq quickstart for more details.

Answer 6

I'm probably missing something, but given the code listed it seems to me that you don't need to worry about whether its called or not.

Two possibilities exist:

1. That the GetProtocolHeader() method needs to be public in which case you write the set of tests that tell you whether it works as expected or not.
2. That its an implementation detail and doesn't need to be public except in so far as you want to be able to test it directly but in that case all you really care about is the set of tests that tell you whether GetProtocolHeaderValue() works as required.

In either case you are testing the exposed functionality and at the end of the day that's all that matters. If it were a dependency then yes you might be worrying about whether it was called but if its not the surely its an implemenation detail and not relevant?

Answer 7

I often use a partial mock (in Rhino) or the equivalent (like CallsBaseMethod in FakeItEasy) to mock the actual class I'm testing. Then you can make GetProtocolHeader virtual and mock your calls to it. You could argue that it's violating the single responsibility principal, but that's still clearly very cohesive code.

Alternatively you could make a method like

internal static string GetProtocolHeaderValue(string name, IHeader header )

and test that processing independently. The public GetProtocolHeaderValue method wouldn't have any/many tests.

Edit: In this particular case, I'd also consider adding GetValue() as an extension method to IHeader. That would be very easy to read, and you could still do the null checking.