Daniel said:
Test-driven development is all well and good, but I would argue that
you shouldn't compromise on implementation choices simply for added
testing convenience.
Its certainly one of those areas of TDDing which has been debated a lot.
Should we deliberately mess with a class to aid testability - if we do
is it a valid messing, or is it indicative of a design problem?
Personal, it depends...
There are times when working with legacy code (i.e. code written without
unittests) that we employ techniques to aid testability, so that we can
add the new functionality or fix the bug. However, the testability aids
often mean we compromise the original design. Sometimes it can even mean
we end up with a design that is 'bad'. However, it is at least now
testable and surrounded with tests. Therefore its open to refactoring
with the safety of those tests, to make the design good again.
Then there are other times when in order to unit test the immutable
class, we'd have to fake/mock a layer far away from the class, in order
too naturally test it. Unless we decide its worth bending the class to
be testable.
A simple example....
We associate an immutable Action object with a JMenu. The immutable
Action object has a simple task of creating a thread to execute a
runnable which does something (doesn't matter what it does for this
example).
class ImmutableAction extends AbstractAction implements Runnable{
public void actionPerformed(...) {
Thread t = new Thread(this);
t.start();
}
public void run() {
//....does something in this other thread.
}
}
In order to test the immutable Action, we could decide that we just
test the run() method directly, as its not easy to unit test something
that starts a separate thread.
public void testRun() {
ImmutableAction action = new ImmutableAction();
action.run();
assertTrue(did action's run() do its job?);
}
However, as we are TDDing, we write the unit tests before the code. so
how do we write a test for the actionPerformed method?
we can't do...
public void testActionPerformed() {
ImmutableAction action = new ImmutableAction();
action.actionPerformed();
// run method is doing its work in a separate thread
// so we can't test whether it worked.
}
Unless....we write the ImmutableAction to be testable from the start...
lets start with the test first....Lets create a Testable derived
version. Then call its actionPerformed() method.
public void testActionPerformed() {
ImmutableAction action = new TestableImmutableAction();
action.actionPerformed();
assertTrue(did action's run() do its job?);
}
class TestableImmutableAction extends ImmutableAction {
protected void runInSeparateThread(Runnable runnable) {
// as we are testing the action,
//we actually want to run it in this thread so...
runnable.run();
}
}
class ImmutableAction extends AbstractAction implements Runnable{
public void actionPerformed(...) {
runInSeparateThread(this);
}
protected void runInSeparateThread(Runnable runnable) {
Thread t = new Thread(runnable);
t.start();
}
public void run() {
//....does something in this other thread.
}
}
It might appear that we have gone to a lot of trouble to test the
actionPerformed() method, seeing as its not really any different, we
still are not executing the thread code in our unittests. However, we
are calling the actionPerformed() method, which in turn is asking for
the run() method to be invoked. This gives us more safety coverage,
when adding new abilities to the class's actionPerformed() method.
Any code in there would not be tested with the first approach where we
only tested the class via the run() method.
This technique has been called 'extract and over ride factory method'
and there's a similar 'extract and over ride getter'
Sometimes difficulty in unit-testing may be
indicative of bad design/implementation decisions,
I strongly agree!
but I don't think
that this is one of those situations.
Good, we might be on the same wave length then...
Would you, for example, advocate
making a private method (one that makes sense to be private from a
design/implementation point of view) public so that you can call it
from your test code and make your unit-testing a bit more
fine-grained?
If I could easily test the private method via the classes public
methods, then no. And seeing as we are TDDing, the private method only
appeared as a result of a test which forced the creation of code to
satisfy the test, followed by the necessary refactorings to remove
duplication. These refactoring may have resulted in the private method
being created. Therefore the private method is usually fully covered by
tests.....
I would not and I see the issue of allowing inheritence
with the immutable class as being the same kind of situation. I would
settle for (and prefer) having my unit tests merely work with the
observable behaviour of the class.
....Agreed, however, if I was truly worried by the private method, and
knew that I should test it further, one approach which I'd used, is
extractClass rather than changing of the private methods access.
I'd move the private method into a class of its own, making it a normal
public method at the same time. We'd have two classes then that we can
test together or separately. If the private method is so in need of
stand alone testing, then its an indication of a design smell and
extracting the method into its own class is usually a good cure.
A classic example is where the private method has an IF or a switch.
These refactor nicely into either the command pattern or the strategy
pattern.
