Polish contribution

See #1454

Author: sbrannen

junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/LifecycleMethodExecutionExceptionHandler.java

@@ -10,127 +10,114 @@
 
 package org.junit.jupiter.api.extension;
 
-import static org.apiguardian.api.API.Status.STABLE;
+import static org.apiguardian.api.API.Status.EXPERIMENTAL;
 
 import org.apiguardian.api.API;
 
 /**
  * {@code LifecycleMethodExecutionExceptionHandler} defines the API for
  * {@link Extension Extensions} that wish to handle exceptions thrown during
- * execution of lifecycle methods (annotated with {@code @BeforeAll},
- * {@code @BeforeEach}, {@code @AfterEach} and {@code @AfterAll}.
+ * the execution of {@code @BeforeAll}, {@code @BeforeEach}, {@code @AfterEach},
+ * and {@code @AfterAll} lifecycle methods.
  *
  * <p>Common use cases include swallowing an exception if it's anticipated,
- * logging or rolling back a transaction in certain error scenarios.
+ * logging errors, or rolling back a transaction in certain error scenarios.
  *
- * <p>This extension needs to be declared on a class level if class level methods
- * ({@code @BeforeAll}, {@code @AfterAll}) are to be covered. If declared on Test
- * level, only handlers for {@code @BeforeEach} and {@code @AfterEach} will execute
+ * <p>Implementations of this extension API must be registered at the class level
+ * if exceptions thrown from {@code @BeforeAll} or {@code @AfterAll} methods are
+ * to be handled. When registered at the test level, only exceptions thrown from
+ * {@code @BeforeEach} or {@code @AfterEach} methods will be handled.
  *
  * <h3>Constructor Requirements</h3>
  *
  * <p>Consult the documentation in {@link Extension} for details on constructor
  * requirements.
  *
- * @see TestExecutionExceptionHandler
+ * <h3 id="implementation-guidelines">Implementation Guidelines</h3>
+ *
+ * <p>An implementation of an exception handler method defined in this API must
+ * perform one of the following.
+ *
+ * <ol>
+ * <li>Rethrow the supplied {@code Throwable} <em>as is</em>, which is the default implementation.</li>
+ * <li>Swallow the supplied {@code Throwable}, thereby preventing propagation.</li>
+ * <li>Throw a new exception, potentially wrapping the supplied {@code Throwable}.</li>
+ * </ol>
+ *
+ * <p>If the supplied {@code Throwable} is swallowed by a handler method, subsequent
+ * handler methods for the same lifecycle will not be invoked; otherwise, the
+ * corresponding handler method of the next registered
+ * {@code LifecycleMethodExecutionExceptionHandler} (if there is one) will be
+ * invoked with any {@link Throwable} thrown by the previous handler.
  *
+ * @see TestExecutionExceptionHandler
  * @since 5.5
  */
-@API(status = STABLE, since = "5.5")
+@API(status = EXPERIMENTAL, since = "5.5")
 public interface LifecycleMethodExecutionExceptionHandler extends Extension {
 
 	/**
-	 * Handle the supplied {@link Throwable throwable}.
-	 *
-	 * <p>Implementors must perform one of the following.
-	 * <ol>
-	 * <li>Rethrow the supplied {@code throwable} <em>as is</em> which is the default implementation</li>
-	 * <li>Swallow the supplied {@code throwable}, thereby preventing propagation.</li>
-	 * <li>Throw a new exception, potentially wrapping the supplied {@code throwable}.</li>
-	 * </ol>
+	 * Handle the supplied {@link Throwable} that was thrown during execution of
+	 * a {@code @BeforeAll} lifecycle method.
 	 *
-	 * <p>If the supplied {@code throwable} is swallowed, subsequent
-	 * {@code LifecycleMethodExecutionExceptionHandler} will not be invoked;
-	 * otherwise, the next registered {@code LifecycleMethodExecutionExceptionHandler}
-	 * (if there is one) will be invoked with any {@link Throwable} thrown by
-	 * this handler.
+	 * <p>Please refer to the class-level Javadoc for
+	 * <a href="#implementation-guidelines">Implementation Guidelines</a>.
 	 *
 	 * @param context the current extension context; never {@code null}
 	 * @param throwable the {@code Throwable} to handle; never {@code null}
 	 */
 	default void handleBeforeAllMethodExecutionException(ExtensionContext context, Throwable throwable)
 			throws Throwable {
+
 		throw throwable;
 	}
 
 	/**
-	 * Handle the supplied {@link Throwable throwable}.
+	 * Handle the supplied {@link Throwable} that was thrown during execution of
+	 * a {@code @BeforeEach} lifecycle method.
 	 *
-	 * <p>Implementors must perform one of the following.
-	 * <ol>
-	 * <li>Rethrow the supplied {@code throwable} <em>as is</em> which is the default implementation</li>
-	 * <li>Swallow the supplied {@code throwable}, thereby preventing propagation.</li>
-	 * <li>Throw a new exception, potentially wrapping the supplied {@code throwable}.</li>
-	 * </ol>
-	 *
-	 * <p>If the supplied {@code throwable} is swallowed, subsequent
-	 * {@code LifecycleMethodExecutionExceptionHandler}
-	 * will not be invoked; otherwise, the next registered
-	 * {@code LifecycleMethodExecutionExceptionHandler} (if there is one)
-	 * will be invoked with any {@link Throwable} thrown by this handler.
+	 * <p>Please refer to the class-level Javadoc for
+	 * <a href="#implementation-guidelines">Implementation Guidelines</a>.
 	 *
 	 * @param context the current extension context; never {@code null}
 	 * @param throwable the {@code Throwable} to handle; never {@code null}
 	 */
 	default void handleBeforeEachMethodExecutionException(ExtensionContext context, Throwable throwable)
 			throws Throwable {
+
 		throw throwable;
 	}
 
 	/**
-	 * Handle the supplied {@link Throwable throwable}.
+	 * Handle the supplied {@link Throwable} that was thrown during execution of
+	 * a {@code @AfterEach} lifecycle method.
 	 *
-	 * <p>Implementors must perform one of the following.
-	 * <ol>
-	 * <li>Rethrow the supplied {@code throwable} <em>as is</em> which is the default implementation</li>
-	 * <li>Swallow the supplied {@code throwable}, thereby preventing propagation.</li>
-	 * <li>Throw a new exception, potentially wrapping the supplied {@code throwable}.</li>
-	 * </ol>
-	 *
-	 * <p>If the supplied {@code throwable} is swallowed, subsequent
-	 * {@code LifecycleMethodExecutionExceptionHandler} will not be invoked;
-	 * otherwise, the next registered {@code LifecycleMethodExecutionExceptionHandler}
-	 * (if there is one) will be invoked with any {@link Throwable} thrown by
-	 * this handler.
+	 * <p>Please refer to the class-level Javadoc for
+	 * <a href="#implementation-guidelines">Implementation Guidelines</a>.
 	 *
 	 * @param context the current extension context; never {@code null}
 	 * @param throwable the {@code Throwable} to handle; never {@code null}
 	 */
 	default void handleAfterEachMethodExecutionException(ExtensionContext context, Throwable throwable)
 			throws Throwable {
+
 		throw throwable;
 	}
 
 	/**
-	 * Handle the supplied {@link Throwable throwable}.
+	 * Handle the supplied {@link Throwable} that was thrown during execution of
+	 * a {@code @AfterAll} lifecycle method.
 	 *
-	 * <p>Implementors must perform one of the following.
-	 * <ol>
-	 * <li>Rethrow the supplied {@code throwable} <em>as is</em> which is the default implementation</li>
-	 * <li>Swallow the supplied {@code throwable}, thereby preventing propagation.</li>
-	 * <li>Throw a new exception, potentially wrapping the supplied {@code throwable}.</li>
-	 * </ol>
-	 *
-	 * <p>If the supplied {@code throwable} is swallowed, subsequent
-	 * {@code LifecycleMethodExecutionExceptionHandler} will not be invoked; otherwise,
-	 * the next registered {@code LifecycleMethodExecutionExceptionHandler} (if there
-	 * is one) will be invoked with any {@link Throwable} thrown by this handler.
+	 * <p>Please refer to the class-level Javadoc for
+	 * <a href="#implementation-guidelines">Implementation Guidelines</a>.
 	 *
 	 * @param context the current extension context; never {@code null}
 	 * @param throwable the {@code Throwable} to handle; never {@code null}
 	 */
 	default void handleAfterAllMethodExecutionException(ExtensionContext context, Throwable throwable)
 			throws Throwable {
+
 		throw throwable;
 	}
+
 }

junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/TestExecutionExceptionHandler.java

@@ -31,7 +31,6 @@
  * constructor requirements.
  *
  * @see LifecycleMethodExecutionExceptionHandler
- *
  * @since 5.0
  */
 @FunctionalInterface

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassBasedTestDescriptor.java

@@ -371,7 +371,7 @@ private void invokeBeforeAllMethods(JupiterEngineExecutionContext context) {
 						ReflectiveInterceptorCall.ofVoidMethod(InvocationInterceptor::interceptBeforeAllMethod));
 				}
 				catch (Throwable throwable) {
-					invokeBeforeAllExecutionExceptionHandlers(registry, extensionContext, throwable);
+					invokeBeforeAllMethodExecutionExceptionHandlers(registry, extensionContext, throwable);
 				}
 			});
 			if (throwableCollector.isNotEmpty()) {
@@ -380,13 +380,11 @@ private void invokeBeforeAllMethods(JupiterEngineExecutionContext context) {
 		}
 	}
 
-	private void invokeBeforeAllExecutionExceptionHandlers(ExtensionRegistry registry, ExtensionContext context,
+	private void invokeBeforeAllMethodExecutionExceptionHandlers(ExtensionRegistry registry, ExtensionContext context,
 			Throwable throwable) {
 
-		invokeExecutionExceptionHandlers(throwable,
-			registry.getReversedExtensions(LifecycleMethodExecutionExceptionHandler.class),
-			(ex, handler) -> () -> ((LifecycleMethodExecutionExceptionHandler) handler).handleBeforeAllMethodExecutionException(
-				context, ex));
+		invokeExecutionExceptionHandlers(LifecycleMethodExecutionExceptionHandler.class, registry, throwable,
+			(handler, handledThrowable) -> handler.handleBeforeAllMethodExecutionException(context, handledThrowable));
 	}
 
 	private void invokeAfterAllMethods(JupiterEngineExecutionContext context) {
@@ -401,18 +399,16 @@ private void invokeAfterAllMethods(JupiterEngineExecutionContext context) {
 					ReflectiveInterceptorCall.ofVoidMethod(InvocationInterceptor::interceptAfterAllMethod));
 			}
 			catch (Throwable throwable) {
-				invokeAfterAllExecutionExceptionHandlers(registry, extensionContext, throwable);
+				invokeAfterAllMethodExecutionExceptionHandlers(registry, extensionContext, throwable);
 			}
 		}));
 	}
 
-	private void invokeAfterAllExecutionExceptionHandlers(ExtensionRegistry registry, ExtensionContext context,
+	private void invokeAfterAllMethodExecutionExceptionHandlers(ExtensionRegistry registry, ExtensionContext context,
 			Throwable throwable) {
 
-		invokeExecutionExceptionHandlers(throwable,
-			registry.getReversedExtensions(LifecycleMethodExecutionExceptionHandler.class),
-			(ex, handler) -> () -> ((LifecycleMethodExecutionExceptionHandler) handler).handleAfterAllMethodExecutionException(
-				context, ex));
+		invokeExecutionExceptionHandlers(LifecycleMethodExecutionExceptionHandler.class, registry, throwable,
+			(handler, handledThrowable) -> handler.handleAfterAllMethodExecutionException(context, handledThrowable));
 	}
 
 	private void invokeAfterAllCallbacks(JupiterEngineExecutionContext context) {

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java

@@ -24,7 +24,6 @@
 import java.util.List;
 import java.util.Optional;
 import java.util.Set;
-import java.util.function.BiFunction;
 import java.util.function.Supplier;
 
 import org.apiguardian.api.API;
@@ -37,6 +36,7 @@
 import org.junit.jupiter.engine.config.JupiterConfiguration;
 import org.junit.jupiter.engine.execution.ConditionEvaluator;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
+import org.junit.jupiter.engine.extension.ExtensionRegistry;
 import org.junit.platform.commons.JUnitException;
 import org.junit.platform.commons.logging.Logger;
 import org.junit.platform.commons.logging.LoggerFactory;
@@ -50,7 +50,6 @@
 import org.junit.platform.engine.support.hierarchical.ExclusiveResource;
 import org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockMode;
 import org.junit.platform.engine.support.hierarchical.Node;
-import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
 
 /**
  * @since 5.0
@@ -63,7 +62,7 @@ public abstract class JupiterTestDescriptor extends AbstractTestDescriptor
 
 	private static final ConditionEvaluator conditionEvaluator = new ConditionEvaluator();
 
-	protected final JupiterConfiguration configuration;
+	final JupiterConfiguration configuration;
 
 	JupiterTestDescriptor(UniqueId uniqueId, AnnotatedElement element, Supplier<String> displayNameSupplier,
 			TestSource source, JupiterConfiguration configuration) {
@@ -78,7 +77,7 @@ public abstract class JupiterTestDescriptor extends AbstractTestDescriptor
 
 	// --- TestDescriptor ------------------------------------------------------
 
-	protected static Set<TestTag> getTags(AnnotatedElement element) {
+	static Set<TestTag> getTags(AnnotatedElement element) {
 		// @formatter:off
 		return findRepeatableAnnotations(element, Tag.class).stream()
 				.map(Tag::value)
@@ -102,24 +101,30 @@ protected static Set<TestTag> getTags(AnnotatedElement element) {
 	}
 
 	/**
-	 * Invokes handlers on the {@code Throwable} one by one until none are left or the throwable to handle
-	 * has been swallowed.
+	 * Invoke exception handlers for the supplied {@code Throwable} one by one until
+	 * none are left or the throwable to handle has been swallowed.
 	 */
-	void invokeExecutionExceptionHandlers(Throwable throwable, List<? extends Extension> handlers,
-			BiFunction<Throwable, Extension, ThrowableCollector.Executable> generator) {
+	<T extends Extension> void invokeExecutionExceptionHandlers(Class<T> handlerType, ExtensionRegistry registry,
+			Throwable throwable, ExceptionHandlerInvoker<T> handlerInvoker) {
+
+		invokeExecutionExceptionHandlers(registry.getReversedExtensions(handlerType), throwable, handlerInvoker);
+	}
+
+	private <T extends Extension> void invokeExecutionExceptionHandlers(List<T> handlers, Throwable throwable,
+			ExceptionHandlerInvoker<T> handlerInvoker) {
+
 		// No handlers left?
 		if (handlers.isEmpty()) {
 			ExceptionUtils.throwAsUncheckedException(throwable);
 		}
 
 		try {
 			// Invoke next available handler
-			ThrowableCollector.Executable executable = generator.apply(throwable, handlers.remove(0));
-			executable.execute();
+			handlerInvoker.invoke(handlers.remove(0), throwable);
 		}
 		catch (Throwable handledThrowable) {
 			BlacklistedExceptions.rethrowIfBlacklisted(handledThrowable);
-			invokeExecutionExceptionHandlers(handledThrowable, handlers, generator);
+			invokeExecutionExceptionHandlers(handlers, handledThrowable, handlerInvoker);
 		}
 	}
 
@@ -147,15 +152,15 @@ public ExecutionMode getExecutionMode() {
 		return toExecutionMode(configuration.getDefaultExecutionMode());
 	}
 
-	protected Optional<ExecutionMode> getExplicitExecutionMode() {
+	Optional<ExecutionMode> getExplicitExecutionMode() {
 		return Optional.empty();
 	}
 
-	protected Optional<ExecutionMode> getDefaultChildExecutionMode() {
+	Optional<ExecutionMode> getDefaultChildExecutionMode() {
 		return Optional.empty();
 	}
 
-	protected Optional<ExecutionMode> getExecutionModeFromAnnotation(AnnotatedElement element) {
+	Optional<ExecutionMode> getExecutionModeFromAnnotation(AnnotatedElement element) {
 		// @formatter:off
 		return findAnnotation(element, Execution.class)
 				.map(Execution::value)
@@ -173,7 +178,7 @@ public static ExecutionMode toExecutionMode(org.junit.jupiter.api.parallel.Execu
 		throw new JUnitException("Unknown ExecutionMode: " + mode);
 	}
 
-	protected Set<ExclusiveResource> getExclusiveResourcesFromAnnotation(AnnotatedElement element) {
+	Set<ExclusiveResource> getExclusiveResourcesFromAnnotation(AnnotatedElement element) {
 		// @formatter:off
 		return findRepeatableAnnotations(element, ResourceLock.class).stream()
 				.map(resource -> new ExclusiveResource(resource.value(), toLockMode(resource.mode())))
@@ -217,4 +222,11 @@ public void cleanUp(JupiterEngineExecutionContext context) throws Exception {
 		context.close();
 	}
 
+	@FunctionalInterface
+	interface ExceptionHandlerInvoker<T extends Extension> {
+
+		void invoke(T t, Throwable throwable) throws Throwable;
+
+	}
+
 }