junit-team / junit4

A programmer-oriented testing framework for Java.
https://junit.org/junit4
Eclipse Public License 1.0
8.53k stars 3.28k forks source link

Exceptions thrown from methodBlock() result in the failing method being unrooted in test reports #1066

Closed sbrannen closed 9 years ago

sbrannen commented 9 years ago

The introduction of the runLeaf() method in BlockJUnit4ClassRunner in JUnit 4.9 introduces a regression with regard to exception handling.

Consult the following versions to see the difference:

  1. BlockJUnit4ClassRunner (4.8)
  2. BlockJUnit4ClassRunner (4.9)

As stated in the Javadoc for methodBlock(), methodBlock() can be overridden in subclasses, either by overriding this method, or the implementations creating each sub-statement.

Thus, although other changes to the JUnit codebase itself in the JUnit 4.9 release do not result in an exception being thrown from methodBlock(), custom modifications to methodBlock() or the methods it invokes may in fact throw exceptions. In such cases, exceptions thrown from methodBlock() cause the current test execution to abort immediately. As a result, the failing test method is unrooted in test reports, and subsequent test methods are never invoked. Furthermore, RunListeners registered with JUnit are not notified.

The root cause is that the invocation of methodBlock() is no longer executed within a try-catch block as was the case in previous versions of JUnit.

A fix for this regression can be seen in recent changes to the SpringJUnit4ClassRunner from the Spring Framework.

See also: SPR-12613 in Spring's JIRA issue tracker.

kcooney commented 9 years ago

Thanks for the report. Since this bug was introduced so long ago, I'm guessing we won't do a bug-fix release for 4.12, and instead try to get a fix into 4.13

As a work-around, have your implementation of methodBlock() do thetry ... catch, and if an exception is thrown, return a Statement that rethrows the exception.

sbrannen commented 9 years ago

@kcooney, did you look at the SpringJUnit4ClassRunner link above?

It already does exactly what you suggested (except in the runChild() method in order to maintain the previous behavior). ;)

kcooney commented 9 years ago

@sbrannen I was just leaving the comment about the work-around in case someone finds this thread.

It's rather unfortunate that SpringJUnit4ClassRunner has to duplicate so much code.

sbrannen commented 9 years ago

I was just leaving the comment about the work-around in case someone finds this thread.

Ahhh, OK. Thanks!

It's rather unfortunate that SpringJUnit4ClassRunner has to duplicate so much code.

Yes, I couldn't agree more!

It's always been unfortunate and yet a necessity in order to properly integrate Spring's testing support into JUnit. If JUnit were to introduce an SPI/API that allows developers and third-party frameworks to tie into the test execution lifecycle in JUnit declaratively or by automatic discovery (e.g., similar to Spring's TestContextBootstrapper, TestContextManager, TestExecutionListener API, and SpringJUnit4ClassRunner combination), the SpringJUnit4ClassRunner would likely/hopefully become obsolete. See issue #874 for a related discussion.

marcphilipp commented 9 years ago

@sbrannen Do you have time to submit a pull request for this?

sbrannen commented 9 years ago

Done: pull request #1082.