wuetender-junger-mann.de

What to cover?

In my previous post I dismissed the notion that it is necessary to test every class of your system in isolation. I argued that this is fixing the protocols at all layers of your application and thus making refactorings that shuffle responsibilities around more expensive. Perryn rightly pointed out that the refactoring I proposed should have been covered by some kind of a regression test. So I assume another layer of tests. It would look something like this, where the red circles mark object that are tested together. While the lower right unit can be fully integration tested, the other units actually need stubbed or mocked out collaborators. I think it’s worthwhile to strive for components that have little external dependencies in some cases - pushing as much functionality towards the leaves as Alex recommended.

What is the problem with this approach? Well you definitely should have an integration test a the hightest possible level. In practice this can be difficult, as these things might run in different processes, on different machines, being implemented in a variety of technolgies (javascript on IE6 anyone?). Ideally however we have something along these lines:

Looking at the graph again we might also go for the following component, as it needs only one stubbed out dependency:

On the other hand if we now look at the whole picture of tests, that I mentioned (adding the naïve unit tests), we end up with this picture:

That is probably an overkill. So what is the point of this post? There is a terribly high number of possible tests. As a developer you have to make a call, which ones to go for. Everything else is to expensive and prevents change (Test Sclerosis!). The most confidence in the system is probably gained from system level integration testing. It ensures the system works and it is a level that can actually drive your design in a meaningful way (breaking a system into several components being the actual design effort). However unit tests are quicker to execute and easier to write even in a messy architecture (do you have an object representing that tool your user works with every day, or is it just a bunch of javascripts, a bit of templating and some domainish code to pull things off a database?).

Another thing that these example reveal is that the terms unit test and integration test are relative. This means we probably have to define implicitly or explicitly, what we refer to, when talking about integration tests and unit tests.

Trivial unit tests

I get increasingly annoyed with what I call trivial unit test. People are obviously writing tests for the sake of unit test coverage. I observed the following patterns, which were what I would call overly isolated from their collaborators:

 
// Production code
public class Decorator Factory {
    public Decorator getDecoratorForRendering() {
        return new CompositeDecorator(
                new HtmlDecorator(),
                new ParagraphDecorator(),
                new ImageDecorator()
        );
    }
}
 
// Test code
public void testShouldCreateDecoratorForRendering() throws Exception {
        DecoratorFactory factory = new DecoratorFactory();
        CompositeDecorator compositeDecorator = factory.getDecoratorForRendering();
        Decorator[] decorators = getDeocratorsFromComposite(compositeDecorator);
        int i = 0;
        assertEquals(HtmlDecorator.class, decorators[i++].getClass());
        assertEquals(ParagraphDecorator.class, decorators[i++].getClass());
        assertEquals(ImageDecorator.class, decorators[i++].getClass());
}

 
// Production code
public void reprice(List<Product> productList, PricingPolicy policy) {
        for (Product product : productList) {
                productRepricingService.reprice(product, pricingPolicy);
        }
}
 
// Test code
public void testShouldRepriceAllProducts() {
        for (Product product : productList) {
                productRepricingServiceMock.reprice(product, pricingPolicy);
        }
        finishedMockSetup();
        batchRepricingService.reprice(productList, pricingPolicy);
}

All these tests have a negative net value. There is absolutely nothing gained from them, but on the other hand there is an additional burden in terms of maintenance, build times, and clarity.

Why to test in isolation?

Some reasons I could come up with for testing things in isolation:

In all cases the unit should be complex enough, that the test case actually tells me something I can’t spot immediately by looking at the code.

Bottom line: I have come to the conclusion that in entreprise applications the complexity arises from the interplay of all components. Also the single responsibility principle is sometimes difficult to achieve, as there are some concepts that are central to the business and tend to be overstreched (especially when persisted to RDBMS, which helps beeing reluctant about fine grain objects). Hence I think integration test automation is more important than unit testing. However, if complex functionality arises within a subsystem, it should be nicely isolated and unit tested.

Your typical system, written in a nice object-oriented way performs its task using a number of collaborating objects that send methods to each other. Ignoring some of the complexities like objects actually creating new objects in response to message, we can draw a diagram like the one below:

Each arrow represents a method call. Now for mocking and unit testing, usually you want to test one particular part of the system. In a lot of cases I have seen so far this would be a single object. Somewhat like this:

As we are all aware of dependencies by now and know how we inject them, the constructor for this object probably takes its two collaborators as parameters. So we can easily pass in mock objects, that are used to mimic the actual collaborators behaviour and/ or verify that the object under test actually does trigger certain actions in its collaborators:

So far so good. What is the problem with this approach? Another thing, that the well-adjusted programmer does quite often, is refactoring. While a lot of people think of rename class/ method/ variable, when refactoring is mentioned, these are obviously the trivial cases. Let’s consider a serious change to a subsystem of our hypothetical application:

We might actually introduce some new objects to get a better factored system (contrived example). So we end up with the following:

If we actually went down the hard-core mocking road with that system the place we start from actually looks like this:

The red lines indicate interactions that are now asserted and mocked out throughout the tests. To do a refactoring like the one proposed above you have to change a whole lot of test cases. Even worse I don’t have the confidence, that the behaviour is still the same, because I had to fiddle with the tests during the refactoring, even though the “outer” interface remained the same. I call this situation test sclerosis as the tissue of your application is hardened by all the tests and mocks and thereby loses a lot of flexibility.

How to avoid this situation?

One important question that helps avoiding getting into the above mentioned situation is asking about the value of writing a particular test. Generally we write tests for the following reasons (not a complete list):

It seems to me that naive massively stubbed, or mocked unit tests usually don’t help much with these goals.

The general advice seems to be to choose your units carefully. Single objects are probably too fine grain. Look for natural subsystems. You might for example stub out the file system or an external system that has a very clearly defined interface, e.g a mail server. Keeping the number of mocks and stubs needed per test-case low (i.e. zero or one) is a good tactic. Try to test things that are interesting and non-trivial.

A few days back I wrote a test that used the filesystem. In order to keep my test independant from any particular file layout I usually do something like this:

 
public class SomeFileSystemTest {
    @Test
    public void performTest() throws IOException {
         doStuff(tempDir);
         ...
    }
 
    private File tempDir;
 
 
    @After
    public void deleteTempDir() throws IOException {
        FileUtils.deleteDirectory(tempDir);
    }
 
    @Before
    public  void createTempDir() throws Exception {
        tempDir = File.createTempFile("temp", "SomeFileSystemTest");
        tempDir.delete();
        tempDir.mkdirs();
    }
}

This seems to be the JUnit 4 way of doing things. So today I got back to this test, because I wanted to reuse the temp directory bit in a different test. So I am wondering if there is any way to reuse this bit other than pulling it into a shared super class (how a testing framework is promoting bad design).

But how would I like to do this (Christmas-driven-development)?

The functionality for providing a resource and tearing it down again should be encapsulated! So how about this:

 
public class TempDirFixture implements Fixture {
    final private File tempDir;
 
    public TempDirFixture() throws Exception {
        tempDir = File.createTempFile("temp", "SomeFileSystemTest");
        tempDir.delete();
        tempDir.mkdirs();
    }
 
 
    @Override
    public void tearDown() throws IOException {
        FileUtils.deleteDirectory(tempDir);
    }
 
    public File getTempDir() {
        return tempDir;
    }
}

All I want to do in my test case is to specify, that I want to use a TempDirFixture. The well adjusted java developers way of doing this is by using a constructor parameter. So the test case would become:

 
public class SomeFileSystemTest {
    final TempDirFixuture fixture;
 
    public SomeFileSystemTest(TempDirFixuture fixture){
        this.fixture = fixture;
    }
    
    @Test
    public void performTest() throws IOException {
         doStuff(fixture.getTempDir());
         ...
    }
}

Then the test runner would be responsible for newing a TempDirFixture up and tearing it down later. So the test runner becomes a bit of a mini DI-framework. The dependencies might be injected on the method rather than the object level. Like this:

 
public class SomeFileSystemTest {
  
    @Test
    public void performTest(TempDirFixuture fixture) throws IOException {
         doStuff(fixture.getTempDir());
         ...
    }
}

While I am calling for a new feature I am willing to give up the old @before and @after annotations, because they are so imperative and not what I want to express. Obviously one might also think about the scope of these Fixtures, but that’s the topic of another blogpost.

Writing the new ant I stumbled across the problem of test cases leaving threads behind, after returning control to the runner. My trusted colleague and JUnit Runner expert Mark Burnett and I, knocked up a quick and dirty junit runner that actually allows you to spot such a condition.

Consider the following testcode:

 
@Test
public void losingThreads() throws InterruptedException {
  Thread newThread = new Thread() {
    public void run() {
      for(;;);
    }
  };
  newThread.start();
}

This thing happily spawns a thread and then returns. You get a green test case, but your IDE might behave somewhat funny as there are still threads open. To help diagnose these problems we came up with this threadpolicerunner.

If you now annotate your test case like this, to use it:

 
@RunWith(ThreadPoliceRunner.class)
public class ThreadTest {
    @Test
    public void losingThreads() throws InterruptedException {
        Thread newThread = new Thread() {
            public void run() {
                for(;;);
            }
        };
        newThread.start();
    }
 
 
    @Test
    public void notLosingThreads() throws InterruptedException {
        Thread newThread = new Thread() {
            public void run() {
            }
        };
        newThread.start();
        newThread.join();
    }
 
}

Your runner will give you the following feedback:

Nice, isn’t?

I had a quite a few chats these days with people fro a more QAish background. Originally I intended just to look at selenium and webdriver. I showed some of the stuff, which I wrote in Java, to my QA colleagues. I used a demo webapp that implements a phonebook. My testcase just added a new contact and verified, that the user got a success message.

After they saw a bit of it, they asked me how they could separate out the testdata. I was a bit stunned, because, well it was all there - at least to me as a java developer. I had separated out everything into a method somewhat like this:

 
 createNewContact("Felix", "Leipold");

So I could test for other inputs as well:

 
 createNewContact("", "Invalid");
 createNewContact("Invalid", "");
 createNewContact("Holden", "Caulfield");

Obviously the expectation of QAs was to have an external file, e.g. a spreadsheet that feeds the data into the test code. I am very critical of this approach, but I felt like playing with junit 4 a bit. Junit 4 allows you to specify a runner four your test and indeed they have got one prepackaged that does parameterized tests. So I thought how about having a runner reads a spreadsheet and feeds the data into the test. As an example I use a very simplistic function:

 
    double add(double a, double b) {
        return a + b;
    }

So to test this I created this table and saved it as mathTest.xls:

Then I went on to create a test case, like this:

 
@RunWith(SpreadsheetDriven.class)
@Spreadsheet(name = "mathTest.xls")
public class MathTest {
    double a;
    double b;
    double expectedSum;
 
    public MathTest(Double a, Double b, Double expectedSum) {
        this.a = a;
        this.b = b;
        this.expectedSum = expectedSum;
    }
 
    @Test
    public void testAdd(){
        assertEquals(expectedSum, add(a,b));
 
    }
 
    double add(double a, double b) {
        return a + b;
 
    }
}

Essentially this thing parses the file specified in the annotation and passes it into the constructor. It’s not doing any fancy type conversions. All numerics are passed in as double, booleans as booleans, and everything else should be a string. Implementation and tests are to be found here and depend on apache poi being on the classpath for reading the excel file.

I am also quite proud of the reporting I get:

I am wondering, if all this is a good idea. But if it helps keeping people from using fit, it’s a start. What I realised by now is that actually table or data driven testing is much more appropriate for unit tests. Funnily enough that people actually do it the other way round. For unit tests I would actually recommend not to use a spreadsheet, but the original JUnit Parameterized runner, which lets you define your data like this:

 
    @Parameterized.Parameters
      public static List getParameters(){
        return Arrays.asList(new Object[][]
                {
                        {2,2,4},
                        {3,4,7},
                        {4,2,6},
                        {2,4,6}}
        );
    }

When writing Swing applications I find myself quite often in the Situation to write action handling code like this (Why I do write this kind of code is a different albeit also interesting story):

 
public class Handler {
    Action doMyThing;
 
    public Handler() {
        doMyThing = new AbstractAction("doMyThing"){
            public void actionPerformed(ActionEvent e) {
                doMyThing();
            }
        };
    }
 
    public void doMyThing(){
        System.out.println("printlning is so my thing.");
    }
}

No this looks ok at first sight. My IDE helps me creating that anonymous inner class and all. But as soon as there are three or more actions on my presentation model things get really hard to read. C# introduced delegates for this kind of problem, but what shall the poor Java programmer do?

The thing I tend to ask myself in such situations is, what would I like to write? What of the above code matters? what is coincidental? So a naïve approach would look like this:

 
public class Handler {
    Action doMyThing;
 
    public void doMyThing(){
        System.out.println("printlning is so my thing.");
    }
}

I just slashed all the lines, that seem to be technicalities. And I like it. I still can exercise the doMyThing method in a unit test. And actually it doesn’t take that much to get the thing up and running. We have to do add in a constructor again. But this one is only O(1) in length:

 
public class Handler {
    Action doMyThing;
 
    public Handler() {
        ActionReflector.reflectActions(this);
    }
 
    public void doMyThing(){
        System.out.println("printlning is so my thing.");
    }
}

I like this quite a lot. But how does it work. The reflector looks for all the fields of type Action. If they are null it looks for a method with a matching name. Then it news up a new Action that calls this method and assigns it to the field. If the method lookup fails an exception is thrown. So as long as the class is instantiated at least once in a unit test - which is what one might hope for - the naming convention will actually be enforced.

Here is a a more complex example which illustrates the benefits of this approach.

 
public class Handler {
    Action doMyThing;
    Action doThisOtherThing;
 
    Action manualAction;
 
    public Handler() {
        manualAction = createManualAction();
        ActionReflector.reflectActions(this);
    }
 
 
    public void doMyThing() {
        System.out.println("printlning is so my thing.");
    }
 
    public void doThisOtherThing() {
        System.out.println("printlning is also this other thing.");
    }
 
 
    private Action createManualAction() {
        return new AbstractAction("") {
            public void actionPerformed(ActionEvent e) {
                System.out.println("Doing a manual thing.");
            }
        };
    }
}

The manualAction field is assigned, before the reflector is run. This is quite handy if you just want to delegate to another handler. So you combine the old and the new way easily. If you forget to initialise it, the reflector will fail and tell you the method is missing.

Of course this approach can be extended. One thing I do is to have a second method, that indicates whether the action is enabled. Generally speaking you could use this to implement multi-method interfaces by having a naming convention for each method on the interface.

This niceness has to be paid with some infrastructure code, which is a bit ugly - mostly due to all the declared exceptions in the java reflection.

In essence this gives you a delegate. It works very well for me. Even though you don’t have compile time security, it will blow up as soon as you new your object up, if you get the spelling wrong.

Parser Exception: templates/myoldtemplate.vm 
    Encountered "}" at line 19, column 101. 
    Was expecting one of: "," ... "}" ...  ...  ...