Core Java

Use JUnit’s expected exceptions sparingly

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:

  1. The method call on ubyte(), which is under test. This doesn’t change
  2. The message we want to pass to the failure report (in a string vs. in a method name)
  3. 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:

  1. Processable documentation (e.g. @Deprecated)
  2. Custom “modifiers” on methods, members, types, etc. (e.g. @Override)
  3. 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:

Rest in peace, expected

(it is no longer part of the JUnit 5 @Test annotation, anyway)

Lukas Eder

Lukas is a Java and SQL enthusiast developer. He created the Data Geekery GmbH. He is the creator of jOOQ, a comprehensive SQL library for Java, and he is blogging mostly about these three topics: Java, SQL and jOOQ.
Subscribe
Notify of
guest

This site uses Akismet to reduce spam. Learn how your comment data is processed.

3 Comments
Oldest
Newest Most Voted
Inline Feedbacks
View all comments
Will
Will
8 years ago

Better yet, learn to use JUnit’s Rules. They have a Rule for exceptions. See http://www.javacodegeeks.com/2013/02/testing-expected-exceptions-with-junit-rules.html. On top of it, it works for versions of Java before Java 8.

MECH
MECH
8 years ago

You should use http://junit.org/apidocs/org/junit/rules/ExpectedException.html as it has the added value of deciding exactly when you want to catch the exception (like try-catch), i.e. not during the setup but only before the unit-under-test call. Secondly you can have all that exception message checking you wanted.

AnnotationLiker
AnnotationLiker
8 years ago

I wouldn’t write all those Lambda code if there’s a JUnit Rule at hand that was useful long before Java8 and still is. Other things I don’t agree with: * “or dependency injection if you absolutely must” – Coding without IoC/DI? – Can’t imagine that anymore. Last time I had some legacy piece of code without it, it hurt every day. * “@Transactional” the only thing that made it to mainstream? – OMG… Ever used JPA? * “In most cases, AOP is a niche technique to solve problems, and you generally don’t want that in ordinary programs” – Annotations are… Read more »

Back to top button