Use JUnit’s expected exceptions sparingly

Lukas EderJanuary 21st, 2016Last Updated: January 21st, 2016

3 42 4 minutes read

Sometimes, when we get pull requests for jOOQ or our other libraries, people change the code in our unit tests to be more “idiomatic JUnit”. In particular, this means that they tend to change this (admittedly not so pretty code):

@Test
public void testValueOfIntInvalid() {
    try {
        ubyte((UByte.MIN_VALUE) - 1);
        fail();
    }
    catch (NumberFormatException e) {}
    try {
        ubyte((UByte.MAX_VALUE) + 1);
        fail();
    }
    catch (NumberFormatException e) {}
}

… into this, “better” and “cleaner” version:

@Test(expected = NumberFormatException.class)
public void testValueOfShortInvalidCase1() {
    ubyte((short) ((UByte.MIN_VALUE) - 1));
}

@Test(expected = NumberFormatException.class)
public void testValueOfShortInvalidCase2() {
    ubyte((short) ((UByte.MAX_VALUE) + 1));
}

What have we gained?

Nothing!

Sure, we already have to use the @Test annotation, so we might as well use its attribute expected right? I claim that this is completely wrong. For two reasons. And when I say “two”, I mean “four”:

1. We’re not really gaining anything in terms of number of lines of code

Compare the semantically interesting bits:

// This:
try {
    ubyte((UByte.MIN_VALUE) - 1);
    fail("Reason for failing");
}
catch (NumberFormatException e) {}

// Vs this:
@Test(expected = NumberFormatException.class)
public void reasonForFailing() {
    ubyte((short) ((UByte.MAX_VALUE) + 1));
}

Give or take whitespace formatting, there are exactly the same amount of essential semantic pieces of information:

The method call on ubyte(), which is under test. This doesn’t change
The message we want to pass to the failure report (in a string vs. in a method name)
The exception type and the fact that it is expected

So, even from a stylistic point of view, this isn’t really a meaningful change.

2. We’ll have to refactor it back anyway

In the annotation-driven approach, all I can do is test for the exception type. I cannot make any assumptions about the exception message for instance, in case I do want to add further tests, later on. Consider this:

// This:
try {
    ubyte((UByte.MIN_VALUE) - 1);
    fail("Reason for failing");
}
catch (NumberFormatException e) {
    assertEquals("some message", e.getMessage());
    assertNull(e.getCause());
    ...
}

3. The single method call is not the unit

The unit test was called testValueOfIntInvalid(). So, the semantic “unit” being tested is that of the UByte type’s valueOf() behaviour in the event of invalid input in general. Not for a single value, such as UByte.MIN_VALUE - 1.

It shouldn’t be split into further smaller units, just because that’s the only way we can shoehorn the @Test annotation into its limited scope of what it can do.

Hear this, TDD folks. I NEVER want to shoehorn my API design or my logic into some weird restrictions imposed by your “backwards” test framework (nothing personal, JUnit). NEVER! “My” API is 100x more important than “your” tests. This includes me not wanting to:

Make everything public
Make everything non-final
Make everything injectable
Make everything non-static
Use annotations. I hate annotations.

Nope. You’re wrong. Java is already a not-so-sophisticated language, but let me at least use the few features it offers in any way I want.

Don’t impose your design or semantic disfigurement on my code because of testing.

OK. I’m overreacting. I always am, in the presence of annotations. Because…

4. Annotations are always a bad choice for control flow structuring

Time and again, I’m surprised by the amount of abuse of annotations in the Java ecosystem. Annotations are good for three things:

Processable documentation (e.g. @Deprecated)
Custom “modifiers” on methods, members, types, etc. (e.g. @Override)
Aspect oriented programming (e.g. @Transactional)

And beware that @Transactional is the one of the very few really generally useful aspect that ever made it to mainstream (logging hooks being another one, or dependency injection if you absolutely must). In most cases, AOP is a niche technique to solve problems, and you generally don’t want that in ordinary programs.

It is decidedly NOT a good idea to model control flow structures, let alone test behaviour, with annotations

Yes. Java has come a long (slow) way to embrace more sophisticated programming idioms. But if you really get upset with the verbosity of the occasional try { .. } catch { .. } statement in your unit tests, then there’s a solution for you. It’s Java 8.

How to do it better with Java 8

JUnit lambda is in the works: http://junit.org/junit-lambda.html

And they have added new functional API to the new Assertions class: https://github.com/junit-team/junit-lambda/blob/master/junit5-api/src/main/java/org/junit/gen5/api/Assertions.java

Everything is based around the Executable functional interface:

@FunctionalInterface
public interface Executable {
    void execute() throws Exception;
}

This executable can now be used to implement code that is asserted to throw (or not to throw) an exception. See the following methods in Assertions

public static void assertThrows(Class<? extends Throwable> expected, Executable executable) {
    expectThrows(expected, executable);
}

public static <T extends Throwable> T expectThrows(Class<T> expectedType, Executable executable) {
    try {
        executable.execute();
    }
    catch (Throwable actualException) {
        if (expectedType.isInstance(actualException)) {
            return (T) actualException;
        }
        else {
            String message = Assertions.format(expectedType.getName(), actualException.getClass().getName(),
                "unexpected exception type thrown;");
            throw new AssertionFailedError(message, actualException);
        }
    }
    throw new AssertionFailedError(
        String.format("Expected %s to be thrown, but nothing was thrown.", expectedType.getName()));
}

That’s it! Now, those of you who object to the verbosity of try { .. } catch { .. } blocks can rewrite this:

try {
    ubyte((UByte.MIN_VALUE) - 1);
    fail("Reason for failing");
}
catch (NumberFormatException e) {}

… into this:

expectThrows(NumberFormatException.class, () -> 
    ubyte((UByte.MIN_VALUE) - 1));

And if I want to do further checks on my exception, I can do so:

Exception e = expectThrows(NumberFormatException.class, () -> 
    ubyte((UByte.MIN_VALUE) - 1));
assertEquals("abc", e.getMessage());
...

Great work, JUnit lambda team!

Functional programming beats annotations every time

Annotations were abused for a lot of logic, mostly in the JavaEE and Spring environments, which were all too eager to move XML configuration back into Java code. This has gone the wrong way, and the example provided here clearly shows that there is almost always a better way to write out control flow logic explicitly both using object orientation or functional programming, than by using annotations.

In the case of @Test(expected = ...), I conclude: