How do I test a method was called within a class under test?

Question

I'll start by saying I am pretty new to unit testing and I'd like to start using a TDD approach, but for now am writing unit tests for some existing classes to verify their functionality in all cases.

I've been able to test the majority of my code using NUnit and Rhino mocks without much trouble. However, I've been wondering about unit testing functions that end up calling a lot of other methods within the same class. I can't do something like

classUnderTest.AssertWasCalled(cut => cut.SomeMethod(someArgs))

because the class under test isn't a fake. Furthermore, if a method I'm testing calls other methods in the class under test that in turn also call methods in the same class, I'm going to need to fake a ton of values just to test the "top level" method. Since I'm also unit testing all of these "sub methods", I should be able to assume that "SomeMethod" works as expected if it passes the unit test and not need to worry about the details of those lower-level methods.

Here is some example code I've been working with to help illustrate my point (I've written a class to manage import/export of Excel files using NPOI):

    public DataSet ExportExcelDocToDataSet(bool headerRowProvided)
    {
        DataSet ds = new DataSet();

        for (int i = 0; i < currentWorkbook.NumberOfSheets; i++)
        {               
            ISheet tmpSheet = currentWorkbook.GetSheetAt(i);

            if (tmpSheet.PhysicalNumberOfRows == 0) { continue; }
            DataTable dt = GetDataTableFromExcelSheet(headerRowProvided, ds, tmpSheet);

            if (dt.Rows.Count > 0)
            {
                AddNonEmptyTableToDataSet(ds, dt);
            }
        }

        return ds;
    }

    public DataTable GetDataTableFromExcelSheet(bool headerRowProvided, DataSet ds, ISheet tmpSheet)
    {
        DataTable dt = new DataTable();
        for (int sheetRowIndex = 0; sheetRowIndex <= tmpSheet.LastRowNum; sheetRowIndex++)
        {
            DataRow dataRow = GetDataRowFromExcelRow(dt, tmpSheet, headerRowProvided, sheetRowIndex);
            if (dataRow != null && dataRow.ItemArray.Count<object>(obj => obj != DBNull.Value) > 0)
            {
                dt.Rows.Add(dataRow);
            }
        }

        return dt;
    }

...

You can see that ExportExcelDocToDataSet (my "top-level" method in this case) calls GetDataTableFromExcelSheet which calls GetDataRowFromExcelRow, which calls a couple of other methods that are defined within this same class.

So what is the recommended strategy for refactoring this code to make it more unit testable without having to stub values called by submethods? Is there a way to fake method calls within the class under test?

Thanks in advance for any help or advice!

Answer 1

Modify the subject under test (SUT). If something is hard to unit test, then the design might be awkward.

Faking method calls within the class under test leads to over specified tests. The result are very brittle tests: As soon as you modify or refactor the class, then it is very likely that you also need modify the unit tests. This leads too high maintenance costs of unit testing.

To avoid over specified tests, concentrate on public methods. If this method calls other methods within the class, do not test these calls. On the other hand: Method calls on other dependend on component (DOCs) should be tested.

If you stick to that and have the feeling that you miss some important thing in your tests, then it might be a sign for a class or a method which is doing too much. In case of a class: Look for violations of the Single Responsibility Principle (SRP). Extract classes out of it and test them separately. In case of a method: Split it up the method in several public methods and test each of them separately. If this is still too awkward, you definitely have a class which violates the SRP.

In your specific case you can do the following: Extract the methods ExportExcelDocToDataSet and GetDataTableFromExcelSheet into two different classes (maybe call them ExcelToDataSetExporter and ExcelSheetToDataTableExporter). The original class which contained both methods should reference both classes and call those methods, which you previously extracted. Now you are able to test all three classes in isolation. Apply the Extract Class refactoring (book) to achieve the modification of your original class.

Also note that retrofitting tests are always a bit cumbersome to write and maintain. The reason is that the SUTs, which are written without unit tests, tend to have an awkward design and thus are harder to test. This means that the problems with unit tests must be solved by modifying the SUTs and cannot be solved by pimping up the unit tests.

Answer 2

It doesn't really matter what tested method calls under the hood - this is implementation detail and your unit tests shouldn't be much aware of that. Usually (well, most of the time with unit testing) you want to test single unit and focus on that.

You could either write separate, isolated tests for each public method in your class or refactor part of functionality of your tested class outside. Both approaches focus on same thing though - having isolated tests for each unit.

Now, to give you few hints:

• what is the name of your tested class? Basing on methods it exposes, something along the lines of ExcelExporterAndToDataSetConverter ... or ExcelManager? It seems as this class might be doing too many things at once; this asks for bit of refactoring. Exporting data to DataSet can be easily separated from converting excel data to DataSets/DataRows.
• what happens when GetDataTableFromExcelSheet method changes? Gets moved to other class or is replaced by 3rd party code? Should it break your export tests? It shouldn't - this is one of the reasons your export tests shouldn't verify whether it was called or not.

I suggest moving to DataSet/DataRow conversion methods to separate class - it will ease writing of the unit tests and your export tests won't be as fragile.

Answer 3

I guess you are testing the public method GetDataTableFromExcelSheet separately, so for the tests of ExportExcelDocToDataSet you don't need to verify the behaviour of GetDataTableFromExcelSheet (beyond the fact that ExportExcelDocToDataSet works as expected).

A common strategy is to test only public methods, as any private methods supporting your public methods are tested by default if the public methods behave as expected.

Taking this further, you can test only behaviours of a class, rather than focusing on methods as the unit. That helps to prevent your tests becoming brittle -- where changing the internals of the class has a tendency to break some of your tests.

Of course you want all the code to be well tested, but too tight a focus on methods can lead to brittleness; testing class behaviour (does it do what it should at the highest level) also tests the lower levels.

If you want to fake methods from a test you could refactor your code to take an interface for the method you want to fake. See the command pattern.

In this case though the obvious change would be for the ExportExcelDocToDataSet to take a workbook as an argument. In testing you can then send a fake workbook. See inversion of control.