TestContextManager consistently handles Errors from listener invocations

Issue: SPR-13961

Author: jhoeller

spring-test/src/main/java/org/springframework/test/context/TestContextManager.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -25,6 +25,7 @@
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.util.Assert;
+import org.springframework.util.ReflectionUtils;
 
 /**
  * {@code TestContextManager} is the main entry point into the <em>Spring
@@ -194,10 +195,12 @@ public void beforeTestClass() throws Exception {
 			try {
 				testExecutionListener.beforeTestClass(getTestContext());
 			}
-			catch (Exception ex) {
-				logger.warn("Caught exception while allowing TestExecutionListener [" + testExecutionListener +
-						"] to process 'before class' callback for test class [" + testClass + "]", ex);
-				throw ex;
+			catch (Throwable ex) {
+				if (logger.isWarnEnabled()) {
+					logger.warn("Caught exception while allowing TestExecutionListener [" + testExecutionListener +
+							"] to process 'before class' callback for test class [" + testClass + "]", ex);
+				}
+				ReflectionUtils.rethrowException(ex);
 			}
 		}
 	}
@@ -227,10 +230,12 @@ public void prepareTestInstance(Object testInstance) throws Exception {
 			try {
 				testExecutionListener.prepareTestInstance(getTestContext());
 			}
-			catch (Exception ex) {
-				logger.error("Caught exception while allowing TestExecutionListener [" + testExecutionListener +
-						"] to prepare test instance [" + testInstance + "]", ex);
-				throw ex;
+			catch (Throwable ex) {
+				if (logger.isErrorEnabled()) {
+					logger.error("Caught exception while allowing TestExecutionListener [" + testExecutionListener +
+							"] to prepare test instance [" + testInstance + "]", ex);
+				}
+				ReflectionUtils.rethrowException(ex);
 			}
 		}
 	}
@@ -264,11 +269,13 @@ public void beforeTestMethod(Object testInstance, Method testMethod) throws Exce
 			try {
 				testExecutionListener.beforeTestMethod(getTestContext());
 			}
-			catch (Exception ex) {
-				logger.warn("Caught exception while allowing TestExecutionListener [" + testExecutionListener +
-						"] to process 'before' execution of test method [" + testMethod + "] for test instance [" +
-						testInstance + "]", ex);
-				throw ex;
+			catch (Throwable ex) {
+				if (logger.isWarnEnabled()) {
+					logger.warn("Caught exception while allowing TestExecutionListener [" + testExecutionListener +
+							"] to process 'before' execution of test method [" + testMethod + "] for test instance [" +
+							testInstance + "]", ex);
+				}
+				ReflectionUtils.rethrowException(ex);
 			}
 		}
 	}
@@ -305,24 +312,26 @@ public void afterTestMethod(Object testInstance, Method testMethod, Throwable ex
 		}
 		getTestContext().updateState(testInstance, testMethod, exception);
 
-		Exception afterTestMethodException = null;
+		Throwable afterTestMethodException = null;
 		// Traverse the TestExecutionListeners in reverse order to ensure proper
 		// "wrapper"-style execution of listeners.
 		for (TestExecutionListener testExecutionListener : getReversedTestExecutionListeners()) {
 			try {
 				testExecutionListener.afterTestMethod(getTestContext());
 			}
-			catch (Exception ex) {
-				logger.warn("Caught exception while allowing TestExecutionListener [" + testExecutionListener +
-						"] to process 'after' execution for test: method [" + testMethod + "], instance [" +
-						testInstance + "], exception [" + exception + "]", ex);
+			catch (Throwable ex) {
+				if (logger.isWarnEnabled()) {
+					logger.warn("Caught exception while allowing TestExecutionListener [" + testExecutionListener +
+							"] to process 'after' execution for test: method [" + testMethod + "], instance [" +
+							testInstance + "], exception [" + exception + "]", ex);
+				}
 				if (afterTestMethodException == null) {
 					afterTestMethodException = ex;
 				}
 			}
 		}
 		if (afterTestMethodException != null) {
-			throw afterTestMethodException;
+			ReflectionUtils.rethrowException(afterTestMethodException);
 		}
 	}
 
@@ -347,23 +356,25 @@ public void afterTestClass() throws Exception {
 		}
 		getTestContext().updateState(null, null, null);
 
-		Exception afterTestClassException = null;
+		Throwable afterTestClassException = null;
 		// Traverse the TestExecutionListeners in reverse order to ensure proper
 		// "wrapper"-style execution of listeners.
 		for (TestExecutionListener testExecutionListener : getReversedTestExecutionListeners()) {
 			try {
 				testExecutionListener.afterTestClass(getTestContext());
 			}
-			catch (Exception ex) {
-				logger.warn("Caught exception while allowing TestExecutionListener [" + testExecutionListener +
-						"] to process 'after class' callback for test class [" + testClass + "]", ex);
-				if (afterTestClassException == null) {
-					afterTestClassException = ex;
+			catch (Throwable ex) {
+				if (logger.isWarnEnabled()) {
+					logger.warn("Caught exception while allowing TestExecutionListener [" + testExecutionListener +
+							"] to process 'after class' callback for test class [" + testClass + "]", ex);
+					if (afterTestClassException == null) {
+						afterTestClassException = ex;
+					}
 				}
 			}
 		}
 		if (afterTestClassException != null) {
-			throw afterTestClassException;
+			ReflectionUtils.rethrowException(afterTestClassException);
 		}
 	}