From 0adc4921ed4eadba531f12fdaa5faaf36265029b Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Thu, 18 Feb 2016 21:07:32 +0100 Subject: [PATCH] TestContextManager consistently handles Errors from listener invocations Issue: SPR-13961 --- .../test/context/TestContextManager.java | 65 +++++++++++-------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/TestContextManager.java b/spring-test/src/main/java/org/springframework/test/context/TestContextManager.java index 3fd7767890..8ce1360a56 100644 --- a/spring-test/src/main/java/org/springframework/test/context/TestContextManager.java +++ b/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.Log; 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 Spring @@ -194,10 +195,12 @@ public class TestContextManager { 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 class TestContextManager { 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 class TestContextManager { 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 class TestContextManager { } 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 class TestContextManager { } 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); } }