exception handling

[Jeanne Boyarsky]

I'm curious what people think of the two common ways of handling exceptions in a test where the method throws an Exception. (this doesn't apply to the case where you actually test the exception is thrown - only for the normal case)

Option 1:

public void testMethod() throws Exception { ... }

Option 2:

public void testMethod() { 
  try {
    method();
  } catch (MyException e) {
    fail("message");
  }
}

I tend to favor option one because it is more compact and makes my code easier to read. But others say that it is better to fail because if an exception is thrown it is an application exception and a problem with the code under test. They feel that it is better to reserve errors for environment setup.
[ August 10, 2004: Message edited by: Jeanne Boyarsky ]

[Ernest Friedman-Hill]

I vastly prefer #1, because it makes things go faster. If you use option #2, you lose the useful information in the stack trace, so it's harder to find out why a test has suddenly started to fail.

[Jeanne Boyarsky]

Ernest,
You don't lose info if you do something like this:
fail("message:" + e);

[Ilja Preuss]

Originally posted by Jeanne Boyarsky:
Ernest,
You don't lose info if you do something like this:
fail("message:" + e);

You still use the stack trace, which in some IDEs complicates jumping to the source of the problem.

I don't see the point in using the try-catch block. I actually never know wether I have a failure or an error - I only see the red bar and then immediately jump to the specific cause of the problem.

[somkiat puisungnoen]

In my work in use

  
public void testMethod() throws MyException { ... }
 

I think , when my app thrown myException is easy to debug program.

[Pradeep bhatt]

By using the 2nd approach, we needn't code the fail statement.

[Kam-Wing Pang]

In certain cases you may expect an error to happen in the test which doesn't neccessary mean a test failure. Using the second approach would allow you make sure that such an error doesn't automatically fail the test.

[Lasse Koskela]

Originally posted by Kam-Wing Pang:
In certain cases you may expect an error to happen in the test which doesn't neccessary mean a test failure. Using the second approach would allow you make sure that such an error doesn't automatically fail the test.

Just to make sure, you're talking about errors such as "our database is down" so this test didn't fail because the code is flawed but because of "external" reasons?

[Ernest Friedman-Hill]

Originally posted by Jeanne Boyarsky:
Ernest,
You don't lose info if you do something like this:
fail("message:" + e);

This is true -- that's a bit better.

JS4-94 is an API for embedding Java rule engines. I was a member of the expert group. I wrote the reference implementation. Other people write the test compatibility kit (TCK) which was JUnit-based and Ant-driven. Finding and fixing the source of TCK failures was one of the most painful things I've ever done. All of their test methods were 50-100 lines long, and each had dozens of assertions. The test methods all looked like this:

public void testXXXX() {
  try {
    // 75 lines of code
  } catch (Throwable t) {
    fail(t.getMessage());
  }
}

OK, so when you get a NullPointerException, how do you know what actually went wrong? You don't.

Yes, of course, the real problem is the enormously long test methods. But that actually wouldn't have been a problem if it weren't for the swallowing up of the stack traces. Far better to just let JUnit catch and report the exceptions.

[Ilja Preuss]

Originally posted by Ernest Friedman-Hill:
The test methods all looked like this:

Well, that must be some definition of *expert* group I wasn't previously aware of... :roll:
[ August 11, 2004: Message edited by: Ilja Preuss ]

[Junilu Lacar]

I have to agree with Ilja: I see no point in using try-catch {fail()} if you're not actually expecting the Exception. I prefer to let unexpected exceptions bubble out to the JUnit framework to be reported as an error. This seems more in line with what the creators of the framework had in mind by making a distinction between failures and errors.

Also, not using the try-catch {fail()} kind of forces you to really think about how exceptions should be handled by classes and their clients: should the client expect the exception and can it reasonably recover from it (then a checked exception should be thrown)? Is the exception thrown because the class/method was not being used properly (contract violation, runtime exception should be thrown)? Should an exception not be thrown at all (is class not doing something it should or is there something wrong with the runtime environment)?

In other words, I tend to see errors as a sign that my design is incomplete and failures as a sign that my implementation is not correct.

[J. B. Rainsberger]

http://c2.com/cgi/wiki?CodingJavaUnitExceptionTests

[Junilu Lacar]

Originally posted by J. B. Rainsberger:
http://c2.com/cgi/wiki?CodingJavaUnitExceptionTests

Ah, good to see there's other folks who think the same

From the page cited above:

Also notice the block catching all exceptions and reporting a test failure on them. I used to do this, but have found that it's extra code for no real gain. Not only that, but after seeing a few exceptions thrown that were symptoms of a bad operating environment, I decided these should be errors and not failures. For this reason, I prefer to propagate the exceptions up to the JUnit framework and let the framework handle them.

[Jeanne Boyarsky]

In other words, I tend to see errors as a sign that my design is incomplete and failures as a sign that my implementation is not correct.

Junilu,
On first read, I thought this was the standard argument of people favoring approach #2 (errors = environment problems, failures = code problems.)

Then I read it more carefully. This is a very interesting way of looking at it. I like the design vs implementation distinction.

[Jeanne Boyarsky]

Not to hijack my own question, but how much detail do you go into to maintain the error vs failure distinction?

For example, would you code both of the following:
assertNull("not null", result);
assertEquals("value", result.getField());

or just assume that result was non-null?

[Lasse Koskela]

Originally posted by Jeanne Boyarsky:
would you code both of the following:
assertNull("not null", result);
assertEquals("value", result.getField());

or just assume that result was non-null?

I would probably omit assertNull() and let another test verify that scenario.

[Jeanne Boyarsky]

Oops. I meant to type assertNotNull() instead of assertNull()

[J. B. Rainsberger]

Not to hijack my own question, but how much detail do you go into to maintain the error vs failure distinction?

For example, would you code both of the following:

assertNotNull("not null", result);
assertEquals("value", result.getField());

or just assume that result was non-null?

I would write assertion #2 first, then, if I saw that result was null, I would add assertion #1, just to avoid the silly NullPointerException.

[Jeanne Boyarsky]

J.B.,
That's a good point to add it if/when it comes up.

[Ilja Preuss]

Originally posted by J. B. Rainsberger:
I would write assertion #2 first, then, if I saw that result was null, I would add assertion #1, just to avoid the silly NullPointerException.

Mhh, does this really add value? In my opinion, trying to access a member field/method already communicates that the reference is not allowed to be null. And I don't find to be a NPE more silly than an AssertionFailedError...

[J. B. Rainsberger]

Mhh, does this really add value?

I do it. That doesn't mean it adds value. (I'd like it if that were true, but it isn't always.)

[Jeanne Boyarsky]

First, I have to admit that I rarely add in the assertNotNull() check. But I do think it adds value if it happens relatively often.

This is due to the fact that I can add a message to assertNonNull() which will tell me immediately what was null rather than having to look at the stack trace.

[J. B. Rainsberger]

I just realized, from the first message, that this is wrong:

public void testMethod() { 
  try {
    method();
  } catch (MyException e) {
    fail("message");
  }
}

The fail() is in the wrong spot. It should be the last line of the try block.

[Jeanne Boyarsky]

J. B.,
Actually the post is correct.

This is one of the two patterns for when the code throws an exception that it isn't supposed to:

public void testMethod() { 
  try {
    method();
  } catch (MyException e) {
    fail("message");
  }
}

This is the pattern to test the code throws an exception when it is supposed to.

public void testMethod() { 
  try {
    method();
    fail("message");
  } catch (MyException e) {
    // expected
  }
}

[Junilu Lacar]

Originally posted by Jeanne Boyarsky:
Actually the post is correct.
This is one of the two patterns for when the code throws an exception that it isn't supposed to:

That's the point I think we've (J.B, Ilja, myself) been trying to make: This is not a good pattern to use. I don't know if I'd go so far as to say it's an anti-pattern but it doesn't really give you anything over just letting the unexpected exception bubble out to the framework to be reported as an error. At best, the test code will be misleading/confusing to read (intent not clear).

[Ilja Preuss]

Originally posted by Jeanne Boyarsky:
First, I have to admit that I rarely add in the assertNotNull() check. But I do think it adds value if it happens relatively often.

This is due to the fact that I can add a message to assertNonNull() which will tell me immediately what was null rather than having to look at the stack trace.

Well, I never read the stacktrace - I just click on it and let Eclipse show me where the error occured...

[Jeanne Boyarsky]

Junilu,
Point taken