Merge pull request junit-team#452 from davidhart82/issue433

TestWatcher exceptions hide test exceptions

src/main/java/org/junit/rules/TestWatcher.java

@@ -1,7 +1,11 @@
 package org.junit.rules;
 
+import java.util.ArrayList;
+import java.util.List;
+
 import org.junit.internal.AssumptionViolatedException;
 import org.junit.runner.Description;
+import org.junit.runners.model.MultipleFailureException;
 import org.junit.runners.model.Statement;
 
 /**
@@ -43,22 +47,62 @@ public Statement apply(final Statement base, final Description description) {
 		return new Statement() {
 			@Override
 			public void evaluate() throws Throwable {
-				starting(description);
+				List<Throwable> errors = new ArrayList<Throwable>();
+
+				startingQuietly(description, errors);
 				try {
 					base.evaluate();
-					succeeded(description);
+					succeededQuietly(description, errors);
 				} catch (AssumptionViolatedException e) {
 					throw e;
 				} catch (Throwable t) {
-					failed(t, description);
-					throw t;
+					errors.add(t);
+					failedQuietly(t, description, errors);
 				} finally {
-					finished(description);
+					finishedQuietly(description, errors);
 				}
+
+				MultipleFailureException.assertEmpty(errors);
 			}
 		};
 	}
 
+	private void succeededQuietly(Description description,
+			List<Throwable> errors) {
+		try {
+			succeeded(description);
+		} catch (Throwable t) {
+			errors.add(t);
+		}
+	}
+
+	private void failedQuietly(Throwable t, Description description,
+			List<Throwable> errors) {
+		try {
+			failed(t, description);
+		} catch (Throwable t1) {
+			errors.add(t1);
+		}
+	}
+
+	private void startingQuietly(Description description, 
+			List<Throwable> errors) {
+		try {
+			starting(description);
+		} catch (Throwable t) {
+			errors.add(t);
+		}
+	}
+
+	private void finishedQuietly(Description description,
+			List<Throwable> errors) {
+		try {
+			finished(description);
+		} catch (Throwable t) {
+			errors.add(t);
+		}
+	}
+
 	/**
 	 * Invoked when a test succeeds
 	 * 
@@ -84,7 +128,6 @@ protected void failed(Throwable e, Description description) {
 	protected void starting(Description description) {
 	}
 
-
 	/**
 	 * Invoked when a test method finishes (whether passing or failing)
 	 * 

src/test/java/org/junit/tests/experimental/rules/TestWatcherTest.java

@@ -4,10 +4,15 @@
 import static org.hamcrest.CoreMatchers.is;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assume.assumeTrue;
+import static org.junit.experimental.results.PrintableResult.testResult;
+import static org.junit.experimental.results.ResultMatchers.*;
 import static org.junit.runner.JUnitCore.runClasses;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.experimental.results.PrintableResult;
 import org.junit.rules.TestRule;
+import org.junit.rules.TestWatcher;
+import org.junit.runner.Description;
 
 public class TestWatcherTest {
 	public static class ViolatedAssumptionTest {
@@ -49,4 +54,79 @@ public void logFailingTest() {
 		assertThat(FailingTest.watchedLog.toString(),
 				is("starting failed finished "));
 	}
+
+	public static class TestWatcherFailedThrowsExceptionTest {
+		@Rule
+		public TestRule watcher= new TestWatcher() {
+			@Override
+			protected void failed(Throwable e, Description description) {
+				throw new RuntimeException("watcher failure");
+			}
+		};
+
+		@Test
+		public void fails() {
+			throw new IllegalArgumentException("test failure");
+		}
+	}
+
+	@Test
+	public void testWatcherFailedThrowsException() {
+		PrintableResult result= testResult(TestWatcherFailedThrowsExceptionTest.class);
+		assertThat(result, failureCountIs(2));
+		assertThat(result, hasFailureContaining("test failure"));
+		assertThat(result, hasFailureContaining("watcher failure"));
+	}
+
+	public static class TestWatcherStartingThrowsExceptionTest {
+		@Rule
+		public TestRule watcher= new TestWatcher() {
+			@Override
+			protected void starting(Description description) {
+				throw new RuntimeException("watcher failure");
+			}
+		};
+
+		@Test
+		public void fails() {
+			throw new IllegalArgumentException("test failure");
+		}
+	}
+
+	@Test
+	public void testWatcherStartingThrowsException() {
+		PrintableResult result= testResult(TestWatcherStartingThrowsExceptionTest.class);
+		assertThat(result, failureCountIs(2));
+		assertThat(result, hasFailureContaining("test failure"));
+		assertThat(result, hasFailureContaining("watcher failure"));
+	}
+
+	public static class TestWatcherFailedAndFinishedThrowsExceptionTest {
+		@Rule
+		public TestRule watcher= new TestWatcher() {
+			@Override
+			protected void failed(Throwable t, Description description) {
+				throw new RuntimeException("watcher failed failure");
+			}
+
+			@Override
+			protected void finished(Description description) {
+				throw new RuntimeException("watcher finished failure");
+			}
+		};
+
+		@Test
+		public void fails() {
+			throw new IllegalArgumentException("test failure");
+		}
+	}
+
+	@Test
+	public void testWatcherFailedAndFinishedThrowsException() {
+		PrintableResult result= testResult(TestWatcherFailedAndFinishedThrowsExceptionTest.class);
+		assertThat(result, failureCountIs(3));
+		assertThat(result, hasFailureContaining("test failure"));
+		assertThat(result, hasFailureContaining("watcher failed failure"));
+		assertThat(result, hasFailureContaining("watcher finished failure"));
+	}	
 }