Storytelling with Tests #1: test names and granularity

Storytelling This is going to be the first part of a post series dedicated to quality of automated tests. I'm going to share my experience in regard to patterns and tools which you can use to maximize the benefit of your tests.

Very often you can hear that automated tests provide a great documentation of the production code. Such documentation is more fun to write and is always up to date (of course as long as you actually run your tests). A new developer may take a look on the tests to get familiar with the project. Sounds great, doesn't it?

Unfortunately, very often the reality is more or less similar to the following:

@Test
public void testSomethingLol() {

    // 2 kilometers of code

    assertTrue(ok);
}

Not very helpful, right?

There is a large space for improvements in a test like above, but today let's focus on the most fundamental concept: the name.

Test name is crucial. It's an outline of the scenario that the test covers. When chosen carefully, it can be the most valuable information for the reader.

When it's not easy to choose a correct name for a test, then you should consider it a warning light. The most common reason for such difficulty is a bad structure of the test itself.

An example

Let's consider the following fragment of a business domain:

A document can have multiple revisions. At some point a revision can be accepted or rejected by a reviewer. A rejected revision can be amended. Making an amendment creates a new revision prepopulated with the data from the original one. It can be then edited and eventually accepted or rejected.

There are some constraints that I intentionally skipped in the description above. Let's take a look at the following test and check if we can find and understand them.

@Test
public void testAmendDocument() {

  Revision rev1 = documentRevision()
              .inState(REJECTED)
              .createdBy(testUser);

  Revision rev2 = amendmentFor(rev1)
              .inState(ACCEPTED)
              .createdBy(testUser);

  assertFalse(rev1.canBeAmendedBy(testUser));
  assertFalse(rev2.canBeAmendedBy(testUser));
}

Can we learn anything from this test? Well, something we can:

  • it tests part of the "amend a document revision" functionality represented by Revision.canBeAmendedBy method
  • it creates a document with 2 revisions, one in state "Accepted", the other in state "Rejected"
  • both of the revisions can not be amended by the test user

However there is still the most important question to ask: why both of the document revisions can not be amended? Unfortunately, the test name (nor the test itself) does not answer this question.

Let's take a look on the method under test then:

public class Revision {  
    // ...
    public boolean canBeAmendedBy(User user) {
        return isInState(REJECTED)
                && isCreatedBy(user)
                && !hasExistingAmendment();
    }
}

Now it's clear. The first revision from the test can not be amended because its amendment exists already. The second revision is in state ACCEPTED, so it can not be amended neither, as only rejected revisions are allowed to be amended.

It is not the worst unit test ever. It smoothly sets the required data up and covers the important part of the code. But a the same time it fails to document the tested functionality.

Tests should tell stories about the production code. They should be descriptive enough not to force the reader to look at the production code. A failing test should promptly tell what exactly does not work in the system – without strenuous investigation and debugging.

The better way

How can we fix the name of the test from the example? It's not so easy without fixing the test itself.

As I wrote above, the name should describe the test scenario. A descriptive name for the test above would be something like "revision that has an existing amendment or is not in REJECTED state can not be amended". Quite a long name, isn't it? That's because the test covers two distinct business rules. Because of that, we had to use the word "or". It's a common smell, that we're trying to test too much at once.

The solution is to split a large test into multiple small ones. The rule of thumb is that one test covers only one behaviour. That's it. It's very easy to choose test names when following this principle. It is also very easy to comprehend the story that the test tells.

This is actually just applying the Single Responsibility and Open/Close principles to the test codebase. Whenever a tested requirement becomes no longer required, you delete the corresponding test. Whenever a new requirement for your unit emerges, you write a single test. At the end of the project, you end up with large amount of small, very focused tests.

Corrected example

If we apply it to our example, we will end up with tests such as the following:

@Test
public void rejectedRevisionCanBeAmendedByTheOwner() {  
    Revision doc = documentRevision()
              .inState(REJECTED)
              .ownedBy(owner);

    assertTrue(doc.canBeAmendedBy(owner));
}

Dead simple. We can not say much more about the test method than it has been already said in its name. Still, the name is concise and to the point.

Another test could look as follows:

@Test
public void revisionCanNotBeAmendedByUserWhoDoesntOwnIt() {  
    Revision doc = documentRevision()
              .inState(REJECTED)
              .ownedBy(owner);

    assertFalse(doc.canBeAmendedBy(sbElse));
}

As you can see it uses the same data setup as the previous one. It could be tempting to merge them into a single test. In most cases it is better not to do this, though. One could argue that it violates the Don't Repeat Yourself principle, but it rewards us with a very descriptive and self-sufficient test method. It is valuable especially when the tested code evolves.

Another test could look like this:

@Test
public void revisionCanNotBeAmendedIfNotRejected() {  
    Revision doc = documentRevision()
              .inState(ACCEPTED)
              .ownedBy(owner);

    assertFalse(doc.canBeAmendedBy(owner));
}

And another:

@Test
public void revisionCanNotBeAmendedIfAmendmentExistsAlready() {  
    Revision doc1 = documentRevision()
              .inState(REJECTED)
              .ownedBy(owner);

    Revision doc2 = amendmentFor(doc1);

    assertFalse(doc1.canBeAmendedBy(owner));
}

And one more:

@Test
public void rejectedAmendmentCanBeAmended() {  
    Revision doc1 = documentRevision()
              .inState(REJECTED)
              .ownedBy(owner);

    Revision doc2 = amendmentFor(doc1).inState(REJECTED);

    assertTrue(doc2.canBeAmendedBy(owner));
}

As you can see, each test focuses on a single scenario and does not do anything beyond that.

There is more than one test method for a single production method. It's perfectly OK. A method should not be considered the smallest testable amount of code. A single method may have multiple behaviours and each one of them is a candidate for a unit test.

It works great with iterative development – for example with Test Driven Development. You can identify the smallest scenario and write a test. It enables you to define a good API while focusing on just a tiny bit of the requirements at once. Then you add more tests as you implement more features.

Summary

Writing small tests increases their value as documentation. Test method name should tell what requirement is under test. It's extremely helpful for the readers. It makes iterative development easier.

The rules described in this post work great for unit tests. Of course, as integration tests require much more complex setup, you might need to test more than a single thing in an integration test. It's still very important to make it easy to read and understand, though.

I hope you enjoyed this post. I will try to cover more in the forthcoming parts of the series. Feel free to leave your comment if you want to discuss the topic in more detail.


Photo credit: Paul Ife Horne


comments powered by Disqus