31

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.

2
  • nitpicking: shouldn't your IDataAccess be termed IPersistResults, or something similiar ??? Commented Oct 22, 2009 at 13:52
  • IDataAccess will also do Saves() and Selects()... the term Persist to mean means just 'Save' but no Select... ?
    – dferraro
    Commented Oct 23, 2009 at 21:04

7 Answers 7

24

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();
4
  • 1
    This is AWESOME! After realizing that my Mock objects were begining to be more complicated then their actual non-mocked counter parts, I downloaded NMock and used this post as a guide. This is so much easier & simpler & more bullet proof as well. Might be much for the group to wrap their heads around on a first-go, but I like a challenge =). Thanks again everyone..
    – dferraro
    Commented Oct 22, 2009 at 21:34
  • BTW, I do have a question - what does mockery.VerifyallExpectationsHaveBeenMet() do??? I saw in my code, exceptions were thrown mid-unit test when the expected results or parameters were off. So then what's that final call at the end doing?
    – dferraro
    Commented Oct 22, 2009 at 21:35
  • 1
    Glad to hear you like it! What's more, I think some of the new mock frameworks (Rhino, Moq, etc.) are even better, but they're only usable for the 3.5 framework and up. As for configuring expectations on your mocks and verifying them, that's a pretty big topic. This answer to an earlier question of mine was very helpful to me when I was trying to figure it out: stackoverflow.com/questions/1107217/…. With NMock, when you use the Verify... method, you have to make sure you've told the Mockery about every call to every mock object it manages. Commented Oct 23, 2009 at 13:49
  • Doh! I figured it out =). NMock will verify the expects/arguments at 'run time', and VeryifyAllExpectationsHaveBeenMet() will make sure that anything you DID specify to the mock framework really WAS called. Since I was doing 'reverse' TDD that Verify() call was never failing, until I commented out one of my calls and retained the Expect() method then watched it fail =)
    – dferraro
    Commented Oct 23, 2009 at 15:09
5

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.

1
  • I added "unit-testing" tag to be more precise to what you say. Commented Oct 22, 2009 at 13:59
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)));
    }
}
7
  • Thanks Nate and everyone else. This seems like the most simple and obvious answer - but again, my IDataAccess has just a void() function which saves the result to the database. Would I have to add extra methods to my IDataAccess class (like returnRecentInsertCount()), then make an accessor for the instance of my mocked IDataAccess object, to check that value? OR should I add functionality (have IDataAccess.Insert() return count, and have job.Run propogate this count to a return value?) Again - void return / private member problem..
    – dferraro
    Commented Oct 22, 2009 at 15:23
  • @dferraro - you could do that (add extra methods yourself), but you'll be far better off using a Mocking framework that does this for you, if there are any available for your development environment. (And there are many free ones for most popular languages.) Commented Oct 22, 2009 at 15:35
  • I'm still struggling with having the mocks 'talk' to each other properly without doing actual logic. OK, so I have the IConnection Mock just return a 'fake' flat file, by returning a string like ",,,,,".. My mock parser then accepts this string... but.. now what? he accepts a string and returns a data table. If I actually put logic in place to turn this fake string into a datable, I am now re-writing my actual parsing function and it's no longer a mock =(...
    – dferraro
    Commented Oct 22, 2009 at 17:45
  • Please note - while I completely see the advantages to using the mocking frameworks and am not 'scared' of them - I'm avoiding using them on purpose, as I'm using this project for R&D & training purposes and the team needs to take baby steps...
    – dferraro
    Commented Oct 22, 2009 at 17:46
  • I think an important thing to keep in mind is that - based on your description - Job.Run won't require many interesting unit tests. You would probably write tests (for example) that verify it does what you expect when your IConnection returns proper results (which is to say, you would test that it calls IParser with those results) and a test that verifies it does what you expect when it returns null (the correct behavior isn't obvious from what you've written). It's not very exciting, but the purpose of Job.Run is simply to coordinate the behavior of collaborators, not perform logic itself. Commented Oct 22, 2009 at 18:32
2

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.

1

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.

1

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).

2
  • Most decent mock frameworks allow you to assert the flow of calls is correct through the mock objects passed in by providing mechanisms to fail the test if mock functions aren't called in the correct order.
    – workmad3
    Commented Oct 22, 2009 at 14:00
  • @workmad3: That's right. I changed my answer a bit to make this clear in case you understood I thought they don't... Commented Oct 22, 2009 at 14:06
1

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 )

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.