Ensure exceptions from methodBlock() don't result in unrooted tests

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().

Issue: #1066

Author: sbrannen

src/main/java/org/junit/runners/BlockJUnit4ClassRunner.java

@@ -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}
      *
@@ -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.
@@ -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;
     }
 

src/test/java/org/junit/runners/CustomBlockJUnit4ClassRunnerTest.java

@@ -0,0 +1,102 @@
+/*
+ * Copyright 2015 the original author or authors.
+ *
+ * All rights reserved. This program and the accompanying materials are
+ * made available under the terms of the Eclipse Public License v1.0 which
+ * accompanies this distribution and is available at
+ *
+ * http://www.eclipse.org/legal/epl-v10.html
+ */
+
+package org.junit.runners;
+
+import static org.junit.Assert.assertEquals;
+
+import java.util.concurrent.atomic.AtomicInteger;
+
+import org.junit.Ignore;
+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());
+	}
+
+
+	@Ignore("This test case is run manually by the enclosing test class")
+	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();
+		}
+	}
+
+}

src/test/java/org/junit/tests/AllTests.java

@@ -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;
@@ -203,6 +204,7 @@
         RuleMemberValidatorTest.class,
         RuleChainTest.class,
         BlockJUnit4ClassRunnerTest.class,
+        CustomBlockJUnit4ClassRunnerTest.class,
         MethodSorterTest.class,
         TestedOnSupplierTest.class,
         StacktracePrintingMatcherTest.class,