Unit testing a class with no return value?

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 h开发者_运维百科eck 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.

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();

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.

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)));
    }
}

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.

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.

I've asked a similar question.

Though (sense over theory) I do think that some methods don't need unit tests as long as (and until) they:

• don't return any values
• don't change internal state of the class or system that can be checked
• don't rely on anything else (as input or output) than your mock

If their functionality (ie. sequence of calls) is vital you will have to verify that internal functionality is met though. Meaning that you have to verify (using your mocks) that those methods have been called with correct parameters and correct sequence (if it matters).

First of all, as your run() method is kind of a workflow launcher, and you have several steps that need to be executed inside the workflow, I think you need more than one unit test, maybe even to split the existing class into several smaller ones, each corresponding to a step in the workflow.

This way you'll be also testing in isolation each step of the workflow, if at any moment the workflow fails, these smaller unit tests will allow you to identify easier the faulty part ( the step that fails )

But maybe this is already the case, I don't know if you don't have already this kind of division.

Anyway, back to your run() method, the answer lies in your question :

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 beginning / end of the job

So you have:

• some input data ( from the excel file )

• some "output" data or rather the result of the wokflow.

In order for your run() to succeed, you need to check that:

a) the request has been sent to the third party and/or a result has been received. I don't know which of those will be easier to check, but at least you could probably log the request/response and check the logs ( in the unit test ) for the operation being executed. This will ensure that the whole workflow is executed ( we can imagine the scenario where the correct data are present in the db at the end of the workflow, but not because the run worked correctly, but because the data was already there or something along those lines - if a purge before the test does not delete some data for instance )

b) check the database for the correct values ( in respect toothe input values ) being inserted/updated in the proper places as a result of the workflow.

c) you could even check the logs you're mentioning ( beginning/end of the job ) for the validity of the delay between the two operations ( if you know that it can't work faster than say 10sec, if your log says job done in 1 sec you'll know something went wrong ... )

Edit: as a first test before a) above you might want to check also the input data, as you might imagine errors there as well ( missing excel file, or content has changed so you're having a wrong input, etc )