Unit Test a class that contains an IDictionary.

Question

I am creating a class that determines which of number of registered WCF client callbacks to call. When a client registers with the server, it supplies the token it is interested in. The class then stores a mapping of token to IClientCallback interface for that token, in a Dictionary.

The method and it's class under test looks like the following

public class Router
{
    private IDictionary<int, IClientCallBack> clients;

    public Router()
    {
        clients = new Dictionary<int, IClientCallBack>();
    }

    public Router(IDictionary<int, IClientCallBack> clients)
    {
        this.clients = clients;
    }

    public bool Register(int token, IClientCallBack client)
    {
        if (!clients.ContainsKey(token))
        {
            clients.Add(token, client);
            return true;
        }
        return false;
    }
}

How do I test the clients are successfully registered with the Router? I figured I can either assume that if the function returned true, it is successful (but what is to stop the function body being only "return true;"?) or I could inject the clients Dictionary into the constructor of the class and in my test I could check that clients.Count equals 1, as per the following.

[TestMethod]
public void RegisterTest()
{
    IDictionary<int, IClientCallBack> clients = new Dictionary<int, IClientCallBack>();
    var router = new Router(clients);
    var client = new Mock<IClientCallBack>().Object;
    var success = router.Register(4, client);
    Assert.IsTrue(success);
    Assert.AreEqual(1, clients.Count);
    Assert.AreEqual(clients[4], client);           
}

While the test above seems good, it seems like overkill to use dependency injection to insert the collection in, just so I can test it. However it does make testing a lot easier and more accurate (when testing other methods of the class too).

Is this the recommended way to test this method, or is it overkill?

Answer 1

Well, why do you care what the contents of the dictionary is? If you've injected it then you may well care - but at that point it's part of the API. If you can do without the constructor taking a dictionary, then just test the actual effects of the dictionary's contents.

So, if you call Register twice for the same token, it should return false, right? Make that a test. Presumably there will be other things to do with tokens - so test when the tokens have been registered and when they haven't.

Put it this way: if someone wanted to reimplement the class using a linked list of token/client pairs, would that break the useful functionality of the class? I suspect not - so your tests probably shouldn't care.

Now, before I start sounding too academic, I'm not one of those people who believe that unit tests should always, always touch just public methods and not care about implementation. They're not black box tests. Sometimes knowing the implementation can make it a lot easier to test just one complex bit of logic, even if it's only normally exposed via something which actually does more work around it. But in the case you've presented, I really would just stick to the "does it do what it should as far as the outside world can tell" approach.

Answer 2

Essentially, if your are looking at the inner data structure your test is checking a specific implementation rather than asserting the expected behaviour of the class.

If this was my test I'd rather set some expectations on the mock object and then invoke some methods on the Router class that validate these expectations.

Answer 3

Good point Jon. The class above is simplified slightly from what I was trying at work this afternoon. In the actual code under test, I supplied 1 or more tokens to register the IClientCallBack against. There should be a dictionary entry for each token. The idea being that Resolve(int token) would return the mapped IClientCallBack...

What's throwing me is if my method is

public bool Register(int token, IClientCallBack client)
{
    return true;
}

My test will incorrectly succeed. Now, I could include a call to another method in the class that would verify the IClientCallBack was correctly mapped, such as a call to Resolve, but I'd then be testing to different methods in the one test, and I thought that was bad karma?

Previously I have just tested that the method would return true, but there's that voice telling me that if I code the method to pass, it may not necessarily do what I want it too.

Perhaps I'm just messing the TDD point :)