views:

53

answers:

3

In one of our service classes I have a bunch of methods which just return the DAO result with no processing like

public void acceptRequest(User from, User to) {
    rosterDAO.acceptRequest(from, to);
}

The unit test for this method looks like this

private final RosterDAO rosterDAO = context.mock(RosterDAO.class);
...
public void testAcceptRequest() {
context.checking(new Expectations() {{
    oneOf (rosterDAO).acceptRequest(from, to);
    will (returnValue(1));
}
});

Now to me this test looks completely pointless, the only thing it does is test that the method calls another method. The return value is already well covered by the DAO tests.I'm tempted to drop these tests as I just don't think there's enough going on to warrant the effort to maintain them.

So for al you TDD gurus who insist on 100% coverage:

What value do you see this test bringing to the project?

How could I write it better?

+3  A: 

The rule of thumb is test everything which could possibly break. So if you are convinced that the one-liner can't reasonably break under any real circumstances, it may be fine to leave it untested. However, if you already have working unit tests for some of these methods, IMHO there is no reason to drop them - having more unit tests never hurts, and the maintenance costs should be negligible.

As it is, the method looks trivial enough. However, consider also the possibility of future extensions / modifications - any real chance of the method being modified in the foreseeable future is a strong enough case to warrant a unit test now.

You may want to cover the bigger scenario with an integration test though, to ensure that different parts of the system as a whole work together as expected under (close to) real circumstances.

IMHO 100% unit test coverage is almost always unrealistic and unnecessary in real-world projects. There is usually a fair amount of e.g. exception-handling code which is difficult to test and trivial, thus in my experience it may not simply worth the effort. Focus on the most critical parts first, using your limited resources to get the most benefit. If methods like this are the most interesting code parts left untested though, and you still have time and energy to cover them, you could just as well go and round your test suite out :-) However, I am afraid most real-life projects are far from this level of dilemmas :-(

Péter Török
+2  A: 

How much extra complexity would it take to make these tests interesting?

Here there are three things that could be wrong: the choice of rosterDAO from all the other DAOs you may have, and the two parameters you are passing: the from and the to, which could for example be transposed without any compilation error. As it happens you seem to have no Exception handling to do (is that right by the way?), so I do agree that this is a pretty minimal case.

However, if there was a smidge of extra logic here, for example any conditionality in the selection of the DAO or which params to pass then I would certainly want some tests.

So looking at your project in the large, is this method typical? Suppose you had 20 methods, and 19 of them did have conditionality and so were worth testing. In that case, I would just leave things tidy and also test this method - it's hardly any work to do it.

If this is a dominant pattern then I agree with Peter Torak, it's probably not worth the effort. But I would pay more attention to integration testing to cover this area.

djna
A: 

It is probably pointless to create a unit test of this particular method as there is no validation or business logic in it (the validation is probably at the rosterDAO level). However, you should create integration tests of the method using a real rosterDAO rather than a mocked one.

Nick Ryan