Support for JUnit 4.7 @Rule (was: message checking with @ExpectedException) [SPR-4643]

[spring-projects-issues]

Dave Syer opened SPR-4643 and commented

Additional @ExpectedException attribute to allow the exception message to be checked, e.g. a substring, or regex, or both.

---

Affects: 2.5.2

Referenced from: commits f9f9b43

[spring-projects-issues]

Paul Benedict commented

What's the purpose of this feature? Seems strange to fail a test based on a message, rather than type.

[spring-projects-issues]

Dave Syer commented

I consider it best practice to check the message on the exception in a unit test. Otherwise how do you know you caught the right IllegalArgumentException? How do you verify that the exception message is meaningful to a user?

[spring-projects-issues]

Paul Benedict commented

Unless document by the API you're using, the string of messages are not part of public behavior. This is particularly true for the Java API:
http://mail-archives.apache.org/mod_mbox/harmony-dev/200607.mbox/%3c44BAE039.3060903@pobox.com%3e

If you need to distinguish -- and you own the calling code -- you need to devise a method, as such, to distinguish the errors. Perhaps you want different subclasses, or codes on the exception that indicate the type of failure. Checking messages is far from a best practice, imo.

[spring-projects-issues]

Vincent Ricard commented

If it can help, here is how the exceptions work and are tested on my project.
We have an abstract class for the our core exceptions which override equals() (and some other methods):

public boolean equals(Object pObj) {
return EqualsBuilder.reflectionEquals(this, pObj, false, this.getClass());
}

The core exceptions does not have default constructor. Each exception have a specific constructor with meaningful parameters (and some getters), eg:
public FooException(String ownerId, Action attemptedAction) {
...
}

And in our tests we do:
try {
// some code throwing the FooException
fail("Should have thrown FooException");
} catch (FooException e) {
assertEquals(new FooException("example", Action.BAR), e);
}

So, maybe @ExpectedException could take an instance of "Exception", or something like that?

Hope this help.

[spring-projects-issues]

Dave Syer commented

Checking messages is far from a best practice, imo.

I disagree, but that's not a debate we have to have here. (If I write the code that throws the exception and the message is going to mean anything to the user, then I am going to want to assert something about the message.)

An instance of Exception with a strict assertEquals doesn't really fit the requirement as described - it's too specific for the particular custom Exception hierarchy that you are using, and a regex is quite a useful test of the message, in terms of usability, so that would be better.

[spring-projects-issues]

Paul Benedict commented

I fully understand that validating messages can help determine which reason, among multiple reasons, one exception was thrown. No doubt, that's a solution. I have no problems with the message checking AS LONG AS the API states the message is part of the public behavior. If I were to validate messages, I would do it on my code only and never on external API unless the "public behavior" rule is met.

[spring-projects-issues]

Dave Syer commented

JUnit 4.7 has a new @Rule feature (ExpectedException) that meets this requirement. Suggest this issue is closed.

[spring-projects-issues]

Dave Syer commented

Cancel that. The SpringJUnit4ClassRunner does not evaluate the @Rules in a test. We could change this issue to signal that a refactor of the test context framework is needed to support JUnit 4.7 @Rule features.

[spring-projects-issues]

Juergen Hoeller commented

Sam, I'm assigning this to you primarily for evaluation purposes: If it's easy enough to do (possibly through reflective checks when running against JUnit 4.7), we should try to get it into Spring 3.0 still.

Juergen

[spring-projects-issues]

Sam Brannen commented

FYI: SpringJUnit4ClassRunner now optionally calls JUnit 4.7's BlockJUnit4ClassRunner.withRules() method using reflection in order to provide backward compatibility with JUnit 4.5 and 4.6.

You should see this change in the next nightly build.

Please give it a try and let us know if you run into any issues.

Thanks,

Sam