Unit testing a class with no return value?

Question

I didn't find much in tutorials on this specific question..

So I have a class called 'Job' which has public ctors and a single public Run() function. Everything in the class is private and encapsulated in the class. (You may remember an older post here on this Testing only the public method on a mid sized class?, which replies helped me greatly)

This Run() method does a bunch of things - takes an excel file as input, extracts data out of it, sends a request to a third party data vendor, takes the result and puts it in the database and logs the begining / end of the job.

This Job class uses 3 seperate interfaces / classes inside it's run method, (IConnection will connect to the third party vendor and send the request, IParser will parse the results, and IDataAccess will save the results to the database). So now, the only real logic inside my Run() method is extracting out the excel input and sending it down the chain of the other classes. I created 3 mock classes and use DI on the Job class ctor and everything is fine and dandy...

Except - I'm still a little lost on how the heck to test my Run() method - because it's void and doesn't return anything...

In this case, should I add a return value to the Run() method that returns how many records were extracted from the Excel file? Since this is the only logic done in that function now.. this wouldn't be handled in real code, but would be in the unit tests... which seems a bit smelly to me - but i'm a newb as far as true TDD is concerned...

Second question - should I created a fourth class called IExcelExtractor, which does that logic for me? Or is this a bit of class explosion??

Even if I did the latter, how would I test my Run() function if it returns void and all of its work is being carried out by mocked objects which really do nothing? I could understand if my function had a meaningful return value... but in this case I'm a but confused.

Thanks so much for reading through all this if you made it this far.

Solution 1

What you're describing is often called behavior verification (as opposed to state verification). It's got its proponents and detractors, but for several categories of classes it's the only game in town if you want to unit test.

To unit test a class whose behavior is limited to interacting with collaborators, you typically pass mock collaborator objects that are instrumented in a way that allows you to verify their methods have been called in the way you expect.

If you were to do this by hand (yuck!) for the classes you mentioned in your question, you might create a MockParser class that implements IParser and adds properties that record if and how its methods were called.

It's better to use mocking framework that will create the mocks on the fly, specify expections on them, and verify those expectations.

I've been using NMock2 these days, and the tests look something like this:

// 'mockery' is the central framework object and Mock object factory
IParser mockParser   = mockery.NewMock<IParser>();

// Other dependencies omitted
Job     job          = new Job(mockParser);

// This just ensures this method is called so the return value doesn't matter
Expect.Once.On(mockParser).
    .Method("Parse").
    .WithAnyArguments().
    .Will(Return.Value(new object()));

job.Run();
mockery.VerifyAllExpectationsHaveBeenMet();

Solution 2

When you inject a mock, you pass to the Run class's constructor a test class that you will ask if the test passed. For example, you could test that the IParser mock got the correct request given the excel file you passed in the constructor. You can do this via your own class, and collect the results in it and test what it collected, or you could do this via a mocking framework that gives you ways of expressing such testing without constructing a class.

I see that you tagged your question with tdd, but in true tdd you don't really get this question (you do, but asked differently) because you build the test first, which defines the interface, instead of building the class interface and then thinking how are you going to test this thing. The need to test drives the design. You still use the same techniques (and likely end up with the same design in this case), but the question would have come out a bit different.

Solution 3

You mention you have mock implementations of the 3 classes/interfaces being used in in your self-contained class...

Why not create some known values to return from your mock IConnection, just pass all of them through your mock IParser, and store them in your mock IDataAccess - then in the test check to see that the results in the mock IDataAccess match the expected results from the input from the mock IConnection after running through the run() method?

Edited to add an example -

Application interfaces / classes:

public interface IConnection {
    public List<Foo> findFoos();
}

public interface IParser {
    public List<Foo> parse(List<Foo> originalFoos);
}

public interface IDataAccess {
    public void save(List<Foo> toSave);
}

public class Job implements Runnable {
    private IConnection connection;
    private IParser parser;
    private IDataAccess dataAccess;

    public Job(IConnection connection, IParser parser, IDataAccess dataAccess) {
        this.connection = connection;
        this.parser = parser;
        this.dataAccess = dataAccess;
    }

    public void run() {
        List<Foo> allFoos = connection.findFoos();
        List<Foo> someFoos = parser.parse(allFoos);
        dataAccess.save(someFoos);
    }
}

Mocks / Test classes:

public class MockConnection implements IConnection {
    private List<Foo> foos;

    public List<Foo> findFoos() {
        return foos;
    }

    public void setFoos(List<Foo> foos) {
        this.foos = foos;
    }
}

public class MockParser implements IParser {

    private int[] keepIndexes = new int[0];

    public List<Foo> parse(List<Foo> originalFoos) {
        List<Foo> parsedFoos = new ArrayList<Foo>();
        for (int i = 0; i < originalFoos.size(); i++) {
            for (int j = 0; j < keepIndexes.length; j++) {
                if (i == keepIndexes[j]) {
                    parsedFoos.add(originalFoos.get(i));
                }
            }
        }
        return parsedFoos;
    }

    public void setKeepIndexes(int[] keepIndexes) {
        this.keepIndexes = keepIndexes;
    }
}

public class MockDataAccess implements IDataAccess {
    private List<Foo> saved;

    public void save(List<Foo> toSave) {
        saved = toSave;
    }

    public List<Foo> getSaved() {
        return saved;
    }
}

public class JobTestCase extends TestCase {

    public void testJob() {
        List<Foo> foos = new ArrayList<Foo>();
        foos.add(new Foo(0));
        foos.add(new Foo(1));
        foos.add(new Foo(2));
        MockConnection connection = new MockConnection();
        connection.setFoos(foos);
        int[] keepIndexes = new int[] {1, 2};
        MockParser parser = new MockParser();
        parser.setKeepIndexes(keepIndexes);
        MockDataAccess dataAccess = new MockDataAccess();
        Job job = new Job(connection, parser, dataAccess);
        job.run();
        List<Foo> savedFoos = dataAccess.getSaved();
        assertTrue(savedFoos.length == 2);
        assertTrue(savedFoos.contains(foos.get(1)));
        assertTrue(savedFoos.contains(foos.get(2)));
        assertFalse(savedFoos.contains(foos.get(0)));
    }
}

Solution 4

The idea of TDD is basically that by adhering to it, you are going to write code that is easy to test, because you first write the tests against an interface that lacks the implementation, and then write the code to make the tests pass. It seems that you have written the Job class before the tests.

I figured out that you can change the Job.Run implementation, in which case if you want the code to be testable, you should do something to it to be able to read the values you need to test for.

Solution 5

If the only thing that your run() method does is invoke other objects, then you test it but verifying that the mocks were invoked. Exactly how you do this depends on the mock package, but generally you will find an "expect" method of some sort.

Do not write code within your run() method that will track its execution. If you are unable to verify the method's operation based on its interactions with collaborators (the mocks), that indicates a need to rethink those interactions. Doing so also clutters the mainline code, increasing maintenance costs.

View more solutions