August 07, 2006

The feature that almost was

It started as a simple TestNG idea I had while riding my bicycle on my way to playing tennis.

TestNG allows you to specify that a certain test method is expected to throw an exception.  If the test does, then TestNG marks it as a success.  If it fails to throw or throws a different exception than the one expected, then TestNG will record the test as a failure.

The current syntax uses an attribute of the @Test annotation:

@Test(expectedExceptions = NumberFormatException.class)
public void shouldThrow() { ... }
But now, I was wondering if I couldn't replace this with:
@Test
public void shouldThrow() throws NumberFormatException { ... }
The idea is that whenever TestNG encounters a test method that declares a throws clause, it will expect that test method to throw that exception, exactly as if the developer had specified expectedExceptions.

I ran the idea by a couple of people who didn't see anything wrong with it, so I went ahead and implemented it. The fix took two minutes and ten lines of code.

An idea that everybody likes and that takes ten lines of code... sounds like a winner, right?

Well, it was... until I ran the TestNG regression tests and noticed that a few of them failed. I investigated and found a test like this:

@Test
public void verifyFile() throws IOException {
 // test that uses File and can throw IOException
}
Unsurprisingly, the new TestNG expected this method to throw an IOException while the throws clause is just saying that this code could throw. Since there is no reason to declare this, I decide that I can get rid of the throws clause as follows:
@Test
public void verifyFile() {
  try {
    // test that uses File and can throw IOException
  }
  catch(IOException ex) { ...
  }
}
It works, but... is it wise?

First of all, the regression tests failed. This is always a red flag: if they failed, it means you just introduced a change that could break existing code.

But more importantly, it looks like I now need to handle exceptions in all my future test methods myself. Come to think of it, it's pretty nice to be able to ignore all the possible exceptions that your test code can throw and know that TestNG will notice this and automatically fail the test, regardless of what went wrong.

When I write a test to verify that something works well, that's exactly what I want to focus on: everything must go as planned. If the slightest thing goes wrong, such as an unexpected exception, I want the testing framework to take care of it.

In the face of this mounting evidence, I decided that the change was not worth it and reverted my code.

Moral of the story: tests that break are trying to tell you something.  Listen to them.

Posted by cedric at August 7, 2006 08:44 AM

Comments

I think you made a good choice.

My tests are the one place where I break form and add a "throws Exception" clause if any type of exception can occur. I don't even bother to list the specific types of exceptions, since they'll all result in an "error" result anyway.

Posted by: earl at August 7, 2006 10:04 AM

If I'm testing an error case I just do an assert (I use JUnit, I'm not that familiar with TestNG) to make sure the exception happened, e.g.

try {
// Something that throws an exception
fail("Exception not thrown");
}
catch (Exception e) {
// Do nothing
}

or

Exception ex = null;

try {
// Something that throws an exception
}
catch (Exception e) {
ex = e;
}

assertNotNull(ex);

I don't think it's necessary to add another feature to check an exception is thrown when it's quite simple to do already.

Posted by: Miles Barr at August 7, 2006 02:59 PM

If I'm testing an error case I just do an assert (I use JUnit, I'm not that familiar with TestNG) to make sure the exception happened, e.g.

try {
// Something that throws an exception
fail("Exception not thrown");
}
catch (Exception e) {
// Do nothing
}

or

Exception ex = null;

try {
// Something that throws an exception
}
catch (Exception e) {
ex = e;
}

assertNotNull(ex);

I don't think it's necessary to add another feature to check an exception is thrown when it's quite simple to do already.

Posted by: Miles Barr at August 7, 2006 03:00 PM

Have you tried this way:

@Test (checkExceptions=true)
public void shouldThrow() throws NumberFormatException { ... }

So by default @Test will have (checkExceptions=false) in case it is not specified...

So, this way your existing code would still work:
@Test
public void verifyFile() throws IOException {
// test that uses File and can throw IOException
}

and new code will work too...

What do you think?

Posted by: Ruslan Zenin at August 7, 2006 03:17 PM

Hi Ruslan,

Yes, it would work, but now you need to specify things in two different places, so not really an improvement IMO...

Posted by: Cedric at August 7, 2006 03:18 PM

How about an

@Test-Exception annotation?

Posted by: Muthu Ramadoss at August 8, 2006 01:05 AM

Cedric,

I think you made the right decision too. Consider the method

@Test
public void shouldThrow() throws AnException, AnotherException { ... }

Typically if you write a test that will throw an exception it should only pass if it throws the specific exception that you expect. e.g.

try {
// Something that throws an exception
fail("Exception not thrown");
} catch (AnException e) {
// Pass
}

With the test config above you couldn't achieve this level of detail...

Posted by: Keith at August 8, 2006 02:07 AM

You definitely made the right choice as well. I use the expects exception mechanism a lot and many times my methods declare checked exceptions just so I don't have to handle them in my code. It keeps my tests much simpler in these scenarios.

Posted by: Mike Bosch at August 8, 2006 02:53 AM

I've seen method parameters being annotated individually - can one annotate each exception being declared as well?

Posted by: Sony Mathew at August 8, 2006 11:25 AM

异型钢管
特异钢管

Posted by: 异型钢管 at August 9, 2006 11:31 PM

除尘器

Posted by: 除尘器 at August 10, 2006 10:12 PM

Is it possible to annotate an Exception? So you can do something like:

@Test
public void testMethod throws ShouldNotBeThrown, @Expected ShouldBeThrown, ThisShouldAlsoNotBeThrown

Posted by: Jörg at August 11, 2006 03:13 AM
Post a comment






Remember personal info?