Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Unit-testing a simple collection class

Consider the following class:

public class MyIntSet
{
    private List<int> _list = new List<int>();

    public void Add(int num)
    {
        if (!_list.Contains(num))
            _list.Add(num);
    }

    public bool Contains(int num)
    {
        return _list.Contains(num);
    }
}

Following the "only test one thing" principle, suppose I want to test the "Add" function. Consider the following possibility for such a test:

[TestClass]
public class MyIntSetTests
{
    [TestMethod]
    public void Add_AddOneNumber_SetContainsAddedNumber()
    {
        MyIntSet set = new MyIntSet();
        int num = 0;

        set.Add(num);
        Assert.IsTrue(set.Contains(num));
    }
}

My problem with this solution is that it actually tests 2 methods: Add() and Contains(). Theoretically, there could be a bug in both, that only manifests in scenarios where they are not called one after the other. Of course, Contains() now servers as a thin wrapper for List's Contains() which shouldn't be tested in itself, but what if it changes to something more complex in the future? Perhaps a simple "thin wrap" method should always be kept for testing purposes ?

An alternative approach might suggest mocking out or exposing (possibly using InternalsVisibleTo or PrivateObject) the private _list member and have the test inspect it directly, but that could potentially create test maintainability problems if someday the internal list is replaced by some other collection (maybe C5).

Is there a better way to do this? Are any of my arguments against the above implementations flawed?

Thanks in advance, JC

like image 581
Ohad Schneider Avatar asked Jul 28 '09 15:07

Ohad Schneider


2 Answers

Your test seems perfectly OK to me. You may have misunderstood a principle of unit testing.

A single test should (ideally) only test one thing, that is true, but that does not mean that it should test only one method; rather it should only test one behaviour (an invariant, adherence to a certain business rule, etc.) .

Your test tests the behaviour "if you add to a new set, it is no longer empty", which is a single behaviour :-).

To address your other points:

  • Theoretically, there could be a bug in both, that only manifests in scenarios where they are not called one after the other.
    True, but that just means you need more tests :-). For example, add two numbers, then call Contains, or call Contains without Add.

  • An alternative approach might suggest mocking out or exposing (possibly using InternalsVisibleTo) the private _list member and have the test inspect it directly, but that could potentially create test maintainability problems[...]
    Very true, so don't do this. A unit test should always be against the public interface of the unit under test. That's why it's called a unit test, and not a "messing around inside a unit"-test ;-).

like image 107
sleske Avatar answered Nov 08 '22 12:11

sleske


There are two possibilities.

  1. You've exposed a flaw in your design. You should carefully consider if the actions that your Add method is executing is clear to the consumer. If you don't want people adding duplicates to the list, why even have a Contains() method? The user is going to be confused when it's not added to the list and no error is thrown. Even worse, they might duplicate the functionality by writing the exact same code before they call .Add() on their list collection. Perhaps it should be removed, and replaced with an indexer? It's not clear from your list class that it's not meant to hold duplicates.

  2. The design is fine, and your public methods should rely on each other. This is normal, and there is no reason you can't test both methods. The more test cases you have, theoretically the better.

As an example, say you have a functions that just calls down into other layers, which may already be unit tested. That doesn't mean you don't write unit tests for the function even if it's simply a wrapper.

like image 20
womp Avatar answered Nov 08 '22 13:11

womp