views:

125

answers:

2

So I have a class with a method as follows:

public class SomeClass
{
    ...

    private SomeDependency m_dependency;

    public int DoStuff()
    {
        int result = 0;
        ...
        int someValue = m_dependency.GrabValue();
        ...
        return result;
    }  
}

And I've decided that rather than to call m_dependency.GrabValue() each time, I really want to cache the value in memory (i.e. in this class) since we're going to get the same value each time anyway (the dependency goes off and grabs some data from a table that hardly ever changes).

I've run into problems however trying to describe this new behaviour in a unit test. I've tried the following (I'm using NUnit with RhinoMocks):

[Test]
public void CacheThatValue()
{
    var depend = MockRepository.GeneraMock<SomeDependency>();

    depend.Expect(d => d.GrabValue()).Repeat.Once().Return(1);

    var sut = new SomeCLass(depend);
    int result = sut.DoStuff();
    result = sut.DoStuff();
    depend.VerifyAllExpectations();
}

This however doesn't work; this test passes even without introducing any changes to the functionality. What am I doing wrong?

+4  A: 

I see caching as orthogonal to Do(ing)Stuff. I would find a way to pull the caching logic outside of the method, either by changing SomeDependency or wrapping it somehow (I now have a cool idea for a caching class based around lambda expressions -- yum).

That way your tests for DoStuff don't need to change, you only need to make sure they work with the new wrapper. Then you can test the caching functionality of SomeDependency, or its wrapper, independently. With well-architected code putting a caching layer in place should be rather easy and neither your dependency nor your implementation should know the difference.

Unit tests shouldn't be testing implementation, they should test behavior. At the same time, the subject under test should have a narrowly-defined set of behavior.

To answer your question, you are using a Dynamic Mock and the default behavior is to allow any call that isn't configured. The additional calls are just returning "0". You need to set up an expectation that no more calls are made on the dependency:

depend.Expect(d => d.GrabValue()).Repeat.Once().Return(1);
depend.Expect(d => d.GrabValue()).Repeat.Never();

You may need to enter record/replay mode to get it to work properly.

Talljoe
Good point: "Unit tests shouldn't be testing implementation, they should test behavior. At the same time, the subject under test should have a narrowly-defined set of behavior." This answers my question (as does Robert's comment to my question)
jpoh
+2  A: 

This seems like a case for "tests drive the design". If caching is an implementation detail of SubDependency - and therefore can't be directly tested - then probably some of its functionality (specifically, its caching behavior) needs to be exposed - and since it's not natural to expose it within SubDependency, needs to be exposed in another class (let's call it "Cache"). In Cache, of course, the behavior is contractual - public, and thereby testable.

So the tests - and the smells - are telling us we need a new class. Test-Driven Design. Ain't it great?

Carl Manaster