20110207

Members in unit tests considered harmful

I’ve been reading Domain Specific Languages by Martin Fowler. It is a good book to formalize API design and gives you a new perspective to think about your software. You can buy this book even if you’re not planning to implements DSLs in the future. A DSL is a differently marketed API anyway - but that’s another blog.

I don’t like the style of the tests presented in the book. Martin captures testing of DSLs at the beginning of the book (3.6. Testing DSLs page 53). The tests use members heavily. I’ve seen this style already in Robert C. Martin’s Clean Code. Both are TDD pundits so I thought it’s not too widely known that you can write tests with better readability.


The code that is tested is a controller that uses a state machine to react on events with transitions to the next state.

Use of members in unit tests


The testing style I object to looks like this:

@Test public void event_causes_transition(){
    fire(trigger_a);
    assertCurrentState(a);
}

Sorry, this looks nice but it’s not readable. What is the fixture? What is the class tested and the methods called? What are the parameters of the call and the result? What does the assertion check? You can figure it out, but it’s not easy and you have to navigate a lot in the source file.

No members in unit tests


Now contrast that with this member-less style:

@Test public void event_causes_transition(){
    Event trigger_a = anyEvent();
    State a = anyState();
    Controller controller = controller(stateMachine(transition(trigger_a, a)));

    controller.handle(trigger_a.getId());

    assertEquals(a, controller.getCurrentState());
}

It’s longer than the original version, but everything is in one place. You still setup an initial state but it is completely local. The naming is kept in synch with the original version so you can correlate implementations. The naming is local to the test. It can be specific in member-less style: "a" should be a "state" and "trigger_a" a "trigger".

As you can see in the test, it creates an event and a state. The details of both seem not to be significant for the test. This insignificance is stated in the test explicitly. Then a controller is setup up with a state machine that has one transition. After you have done the setup, you can actually call the method you want to test and check the result.

Every test method following this style has the sequence: setup, execute, assert while keeping everything in the scope of the test: no members!

Code organization and readability


After contrasting the two implementation styles let’s try to explain why the member-less style is more readable.

If tests make heavy use of members, they look nice at the surface: no redundancy, nice method names and helper methods to keep the intend clear. Why is this less readable that the member-less style? The problem is the missing context and necessary navigation in the test code to get the context before you can understand what is actually tested.

If you recorded the navigation of a reader you would see something similar to the following:
  • the setup methods
    • memorize setup members
  • test method: execute statement (fire)
  • execute helper implementation
    • recall setup members
    • memorize result members
  • test method: assert statement (assertCurrentState)
  • assert statement implementation
    • recall result members

As you can see, the reader has to navigate and remember a lot to get the initial setup of the test and keep track of the transition triggered by the execution.

In every place were the reader has to remember some information there’s the potential that an additional navigation is necessary, because he could simply not remember enough context. This becomes even harder when you start to organize the tests into test class hierarchies. The context to remember is the context of all super classes members, execution method implementations and assertion method implementations. Naturally, our ability to store information is limited. Less context information to remember is better for the readability of a test.

So the nice test from above actually looks like this in reality:

@Test a(){
    //a lot of navigation and to remember
    a();
    //dito
    b();
    //dito
}

Ideal organization of code


This is not readable. Ideally, something readable is a linear text laid out exactly in the way you need it to work on your current problem. You build up the context as you go without loosing time navigating. A second ideal of readability is that every information necessary is in one spot. All your context is visible to you at the same time.

The two ideals effect each other. Given a problem complex enough, you cannot have a compact linear representation. Either it’s linear or compact.

Additionally, you make trade-offs to remove redundancy from code. If you are asking yourself whether you reorganize to remove redundancy don’t forget that the most important feature of a test is readability. Introduce some redundancy as long as it helps to understand your test.

Organize to allow effective navigation


You cannot organize code in a way it can be read linearly for all purposes, but you can organize it in a way that the navigation capabilities of IDEs allow effective navigation. Every navigation starts in the test method and is done by looking up the definition of a helper method. The full context is kept in local variables, parameters and return values.

You can start to read a test class from every test method without losing vital information. No navigation to members is necessary:

Event trigger = anyEvent();
State state = anyState();
Controller controller = controller(stateMachine(transition(trigger, state)));

The member-less style makes the setup explicit. Try to name all setup helper methods following the Principle of Least Astonishment. They are nicely named and should state what their individual guarantee is. Accordingly, a method that creates an valid event without further guarantees is named anyEvent().

controller.handle(trigger.getId());

Now the actual method to test can be executed. The member-less style shows exactly what is going on. It presents crystal clear the parameters used and the result value received. There is no navigation overhead. The instances from the setup are used to run the actual test. If there is an execution result it is stored in the local context of the test method.

assertEquals(state, controller.getCurrentState());

After the execution you can do the assertion on the resulting state or result. This again uses only instances from the method scope. Sometimes it is useful to introduce custom assertions to remove redundancy. In this style the assertion method works only on the parameters given, e.g. assertState(state, controller).

If you tracked the navigation for the member-less test style you would get something like this:
  • test
  • setup methods
    • memorize state
  • test
    • recall setup state
    • execute method
    • assert method


The result is not the ideal of a linearly readable test, but the navigation to the definition and back to the test is relatively swift. You can be sure that you did not miss vital information as everything is in one place. Removing uncertainty is the key to a simpler reasoning about the test at hand.

Avoid test class hierarchies


The member-less test setup also helps to avoid test class hierarchies that are are only introduced to allow different fixtures for a sets of tests. Sometimes this is avoided in member style test by doing some setup globally in members and in the test method, but this complicates the reasoning even more. With a member-less test you can have a different setup for every test method. Normally, you organize the setup methods in a way that they build on each other to avoid redundancy.

Avoid isolation from the API


If you put the method execution and assertion into separate methods - like the member heavy style does - you introduce the danger of hiding API problems. One argument for TDD is to experience your API from the point of view of a user. When the actual method calls are in helper methods you do isolate yourself from the API reality your users are facing. It should be nice enough to do the setup, execution and assertion directly with it. Otherwise you have a design problem. The test code should be as much as possible analogous to the code users of an API have to write.

Conclusion


I hope I provided a new perspective to look at the organization of unit tests. If you have objections with this approach feel free to add a comment. If you are interested in this and other aspects of testing software, you can have a look at Test Principles from my co-worker Gaetano Gallo. He covered this idea under "Self containment" in his blog.

3 comments:

  1. I would say it depends on context. I prefer to write some tests as you suggest to test the API and (a lot more) in the style of Martin.

    Take a lexer implementation for example. For the implementation of an lexer I have to write many many tests. I love to write tests the following way:

    @Test public void value() throws Exception {
    assertTokens("test", ID);
    }

    @Test public void valueAndValue() throws Exception {
    assertTokens("test and test2", ID, AND, ID);
    }

    @Test public void valueOrValue() throws Exception {
    assertTokens("test or test2", ID, OR, ID);
    }

    @Test public void valueOrValueAndValue() throws Exception {
    assertTokens("test or test2 and test3", ID, OR, ID, AND, ID);
    }

    private void assertTokens(String input, DependTokenType... types) {
    DependLexer lexer = new DependLexer(input);
    for (int n = 0; n < types.length; n++) {
    assertEquals(types[n], lexer.nextToken().type);
    }
    assertNull(lexer.nextToken());
    }

    Writing the tests this way is much more clear as if I have to repeat the testing code in each test. And the API is exactly the way I want it to be!

    ReplyDelete
  2. You provided another example of the test style I don’t like and it can be done even without members.

    I’d say you’re doing exactly one test in your examples: “Parse IDs separated by operators (AND, OR)”. I would like to test it like this:
    assertEquals(newArrayList(ID, AND, ID), newArrayList(new DependLexer("test and test2")));

    (Actually, I would use Quickcheck to create tokens, serialize the tokens to create the lexer input string and compare the resulting tokens with the input tokens.)

    The test style somewhat forces you to do an API that is usable. Here it’s done by implementing Iterator and using Lists.newArrayList from Google Guava (you can find similar implementations in different places). If your test is redundant user code is probably also redundant.

    ReplyDelete
  3. I think here are two conflicting forces at work. One of the forces is to write the tests down as clear as possible (=> not hiding the intend of the test with technical details like the usage of the API). This tests assert that the program does something but don't test how it is done. Ideal tests from this perspective may look like this:

    "test" => ID
    "test or test2" => ID OR ID
    "!test" => NOT ID

    This is not possible in plain java, my tests try to be as near on this as possible.

    The other force witch you rank higher is to test the API (or the HOW it's done). I think both test types have a reason to exist. You should test that the program does what it should and you should test it does it the way it should.

    ReplyDelete