TestNG and Expected Exceptions

I've started reading Next Generation Java Testing which was just released last week. So far it's interesting enough. I've seen multiple presentations on TestNG, and read Cedric Beust's blog posts that describe the impetus for a new Java software testing approach, so the first chapter hasn't told me much I hadn't already known.

If this sounds like a lackluster introduction, I apologize. It's wonderfully written and informative so far.

Still, I've come across something that bothers me. Consider this example from the book:

public class BookingTest {
  private Plane plane;

  @BeforeMethod
  public void init() {
    plane = createPlane();
  }

  @Test(expectedExceptions = PlaneFullException.class)
  public void shouldThrowIfPlaneIsFull() {
    plane.bookAllSeats();
    plane.bookPlane(createValidItinerary(), null);
  }
}

(One Very Nice Thing about TestNG is that it's easier to read than JUnit, even if you don't know either testing API.)

My specific concern is the notion of expected exceptions in TestNG. The book explains that the expectedExceptions annotation is a reasonable replacement for the try/fail/catch pattern you find in JUnit:

public void testShouldThrowIfPlaneIsFull() {
  plane.bookAllSeats();
  try {
    plane.bookPlane(createValidItinerary(), null);
    fail("Exception expected");
  } catch(PlaneFullException expected) {
    // do nothing; exception expected.
  }
}

(Who puts semicolons in comments? I only put them in example comments. That makes them exemplary comments.)

I agree that the TestNG example is still easier to read than the JUnit example, but the problem is they're not effectively testing the same thing. Here's a more correct translation of the TestNG test using JUnit:

public void testShouldThrowIfPlaneIsFull() {
  try {
    plane.bookAllSeats();
    plane.bookPlane(createValidItinerary(), null);
    fail("Exception expected");
   } catch(PlaneFullException expected) {
     // do nothing; exception expected.
   }
 }

Did you catch the difference? The difference is in how exceptions thrown from plane.bookAllSeats() re handled. In the first JUnit example, if plane.bookAllSeats() throws any exception, the test will fail. In both the TestNG example and the second JUnit example, if plane.bookAllSeats() throws PlaneFullException, the test will still pass.

This bothered me all day. Let's say the engineer was very careful when he constructed this TestNG test. He would have known knew that plane.bookAllSeats() did not throw a PlaneFullException, so he could accept this mild semantic difference for the sake of readability. If, some time later, plane.bookAllSeats() is refactored to throw a PlaneFullException, and in addition, is accidentally broken, the TestNG test will falsely pass, and my JUnit test will correctly fail.

Here we have our problem: the expectedExceptions annotation attribute can't be used on the statement level. If I could annotate on the statement level, I could have this:

@Test
public void shouldThrowIfPlaneIsFull() {
  plane.bookAllSeats();
  @ExceptionExpected(class = PlaneFullException.class) {
    plane.bookPlane(createValidItinerary(), null);
  }
}

Semantically, you can't put annotations on the statement level, it doesn't exist in Java (though I have seen JLS proposals for this.)

Of course, I could simply use the try/fail/catch pattern in TestNG, but that's not The TestNG Way. (I want to say it's not "TestEnGee-Eee" or "TestNGish", but they both sound stupid.)

Let's analyze this test a little further. The first statement in the test is actually just more setup before calling plane.bookPlane(). I could move the set-up code to a specialized method annotated with @BeforeMethod, but given that that kind of set-up operations specialized for each test, readability reduces as the number of @BeforeMethod methods grows. Besides, it's nice to read each specific test as a sequence of statements.

Finally I realized an easy way to make this work:

public class BookingTest {
   private Plane plane;
   private boolean setupComplete;

   @BeforeMethod
   public void init() {
     plane = createPlane();
   }

  private void setupComplete() {
    this.setupComplete = true;
  }

  @AfterMethod
  public void verify() {
    if (!setupComplete) {
      throw new RuntimeException(
        "exception thrown before running critical test statement");
    }
  }

  @Test(expectedExceptions = PlaneFullException.class)
    public void shouldThrowIfPlaneIsFull() {
      plane.bookAllSeats();
      setupComplete();
      plane.bookPlane(createValidItinerary(), null);
    }
  }

In this case, setupComplete() operates as a gate. Now your test doesn't just pass if a PlaneFullException is thrown, you have to get past the setupComplete() method to be considered a successful test.

This solution works, but I don't quite love it. What about using Closures?

@Test
public void shouldThrowIfPlaneIsFull() {
  plane.bookAllSeats();
  expectException(PlaneFullException.class) {
    plane.bookPlane(createValidItinerary(), null);
  }
}

This looks both correct and concise. I love it.

Post-script

1. The authors are sensible enough to point out that there are plenty of times when using the expectedExceptions annotation attribute should be avoided, and they cover those cases. I just don't think I like expectedExceptions at all.

2. Would adding closures to the Java language obviate the need for statement-level annotations? It does here.

3. Those of you paying close attention are screaming that I missed something. Who said createValidItinerary() doesn't throw PlaneFullException?

@Test
public void shouldThrowIfPlaneIsFull() {
  plane.bookAllSeats();

  Itinerary itinerary = createValidItinerary();
  expectException(PlaneFullException.class) {
    plane.bookPlane(itinerary, null);
  }
}

4. This is yet another good argument for writing specialized exceptions for your use cases. It's somewhat unlikely that plane.bookAllSeats() and createValidItinerary() would throw PlaneFullException, but consider the difficulties if a Lazy Programmer designed the Plane API and used ArrayIndexOutOfBoundsException everywhere instead of PlaneFullException. Separating exeptions in a test method are just the beginning of that guy's troubles.