Skip to content

Commit

Permalink
Ensure exceptions from methodBlock() don't result in unrooted tests.
Browse files Browse the repository at this point in the history
The introduction of the runLeaf() method in BlockJUnit4ClassRunner in
JUnit 4.9 introduced a regression with regard to exception handling.

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

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.

This commit addresses this issue by wrapping the invocation of
methodBlock() within a try-catch block. If an exception is not thrown,
the resulting Statement is passed to runLeaf(). If an exception is
thrown, it is wrapped in a Fail statement which is passed to runLeaf().

Closes #1066
Closes #1082
  • Loading branch information
sbrannen authored and kcooney committed Mar 1, 2015
1 parent 0f0152a commit a90b496
Show file tree
Hide file tree
Showing 3 changed files with 105 additions and 4 deletions.
17 changes: 13 additions & 4 deletions src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@
* @since 4.5
*/
public class BlockJUnit4ClassRunner extends ParentRunner<FrameworkMethod> {

private final ConcurrentHashMap<FrameworkMethod, Description> methodDescriptions = new ConcurrentHashMap<FrameworkMethod, Description>();

/**
* Creates a BlockJUnit4ClassRunner to run {@code testClass}
*
Expand All @@ -75,10 +77,17 @@ protected void runChild(final FrameworkMethod method, RunNotifier notifier) {
if (isIgnored(method)) {
notifier.fireTestIgnored(description);
} else {
runLeaf(methodBlock(method), description, notifier);
Statement statement;
try {
statement = methodBlock(method);
}
catch (Throwable ex) {
statement = new Fail(ex);
}
runLeaf(statement, description, notifier);
}
}

/**
* Evaluates whether {@link FrameworkMethod}s are ignored based on the
* {@link Ignore} annotation.
Expand Down Expand Up @@ -390,10 +399,10 @@ private List<org.junit.rules.MethodRule> getMethodRules(Object target) {
protected List<MethodRule> rules(Object target) {
List<MethodRule> rules = getTestClass().getAnnotatedMethodValues(target,
Rule.class, MethodRule.class);

rules.addAll(getTestClass().getAnnotatedFieldValues(target,
Rule.class, MethodRule.class));

return rules;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package org.junit.runners;

import static org.junit.Assert.assertEquals;

import java.util.concurrent.atomic.AtomicInteger;

import org.junit.Test;
import org.junit.runner.Description;
import org.junit.runner.notification.Failure;
import org.junit.runner.notification.RunListener;
import org.junit.runner.notification.RunNotifier;
import org.junit.runners.model.FrameworkMethod;
import org.junit.runners.model.InitializationError;
import org.junit.runners.model.Statement;

/**
* Tests that verify proper behavior for custom runners that extend
* {@link BlockJUnit4ClassRunner}.
*
* @author Sam Brannen
* @since 4.13
*/
public class CustomBlockJUnit4ClassRunnerTest {

@Test
public void exceptionsFromMethodBlockMustNotResultInUnrootedTests() throws Exception {
TrackingRunListener listener = new TrackingRunListener();
RunNotifier notifier = new RunNotifier();
notifier.addListener(listener);

new CustomBlockJUnit4ClassRunner(CustomBlockJUnit4ClassRunnerTestCase.class).run(notifier);
assertEquals("tests started.", 2, listener.testStartedCount.get());
assertEquals("tests failed.", 1, listener.testFailureCount.get());
assertEquals("tests finished.", 2, listener.testFinishedCount.get());
}


public static class CustomBlockJUnit4ClassRunnerTestCase {
@Test public void shouldPass() { /* no-op */ }
@Test public void throwException() { /* no-op */ }
}

/**
* Custom extension of {@link BlockJUnit4ClassRunner} that always throws
* an exception from the {@code methodBlock()} if a test method is named
* exactly {@code "throwException"}.
*/
private static class CustomBlockJUnit4ClassRunner extends BlockJUnit4ClassRunner {

CustomBlockJUnit4ClassRunner(Class<?> testClass) throws InitializationError {
super(testClass);
}

@Override
protected Statement methodBlock(FrameworkMethod method) {
if ("throwException".equals(method.getName())) {
throw new RuntimeException("throwException() test method invoked");
}
return super.methodBlock(method);
}
}

/**
* Simple {@link RunListener} that tracks the number of times that
* certain callbacks are invoked.
*/
private static class TrackingRunListener extends RunListener {

final AtomicInteger testStartedCount = new AtomicInteger();
final AtomicInteger testFailureCount = new AtomicInteger();
final AtomicInteger testFinishedCount = new AtomicInteger();


@Override
public void testStarted(Description description) throws Exception {
testStartedCount.incrementAndGet();
}

@Override
public void testFailure(Failure failure) throws Exception {
testFailureCount.incrementAndGet();
}

@Override
public void testFinished(Description description) throws Exception {
testFinishedCount.incrementAndGet();
}
}

}
2 changes: 2 additions & 0 deletions src/test/java/org/junit/tests/AllTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import org.junit.runner.notification.ConcurrentRunNotifierTest;
import org.junit.runner.notification.RunNotifierTest;
import org.junit.runner.notification.SynchronizedRunListenerTest;
import org.junit.runners.CustomBlockJUnit4ClassRunnerTest;
import org.junit.runners.Suite;
import org.junit.runners.Suite.SuiteClasses;
import org.junit.runners.model.FrameworkFieldTest;
Expand Down Expand Up @@ -203,6 +204,7 @@
RuleMemberValidatorTest.class,
RuleChainTest.class,
BlockJUnit4ClassRunnerTest.class,
CustomBlockJUnit4ClassRunnerTest.class,
MethodSorterTest.class,
TestedOnSupplierTest.class,
StacktracePrintingMatcherTest.class,
Expand Down

0 comments on commit a90b496

Please sign in to comment.