Ensure later exceptions take precedence over TestAbortedException

Issue: #1313
junit-team · Mar 28, 2018 · 924a38c · 924a38c
1 parent 862573f
commit 924a38c
Show file tree

Hide file tree

Showing 13 changed files with 114 additions and 262 deletions.
diff --git a/...piter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassExtensionContext.java b/...piter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassExtensionContext.java
@@ -19,9 +19,9 @@
 import org.apiguardian.api.API;
 import org.junit.jupiter.api.TestInstance.Lifecycle;
 import org.junit.jupiter.api.extension.ExtensionContext;
-import org.junit.jupiter.engine.execution.ThrowableCollector;
 import org.junit.platform.engine.ConfigurationParameters;
 import org.junit.platform.engine.EngineExecutionListener;
+import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
 
 /**
  * @since 5.0

diff --git a/...jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java b/...jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/ClassTestDescriptor.java
@@ -41,7 +41,6 @@
 import org.junit.jupiter.engine.execution.ExecutableInvoker;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
 import org.junit.jupiter.engine.execution.TestInstanceProvider;
-import org.junit.jupiter.engine.execution.ThrowableCollector;
 import org.junit.jupiter.engine.extension.ExtensionRegistry;
 import org.junit.platform.commons.JUnitException;
 import org.junit.platform.commons.util.Preconditions;
@@ -50,6 +49,7 @@
 import org.junit.platform.engine.TestTag;
 import org.junit.platform.engine.UniqueId;
 import org.junit.platform.engine.support.descriptor.ClassSource;
+import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
 
 /**
  * {@link TestDescriptor} for tests based on Java classes.

diff --git a/...iter-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodExtensionContext.java b/...iter-engine/src/main/java/org/junit/jupiter/engine/descriptor/MethodExtensionContext.java
@@ -19,9 +19,9 @@
 import org.apiguardian.api.API;
 import org.junit.jupiter.api.TestInstance.Lifecycle;
 import org.junit.jupiter.api.extension.ExtensionContext;
-import org.junit.jupiter.engine.execution.ThrowableCollector;
 import org.junit.platform.engine.ConfigurationParameters;
 import org.junit.platform.engine.EngineExecutionListener;
+import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
 
 /**
  * @since 5.0

diff --git a/...er-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestMethodTestDescriptor.java b/...er-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestMethodTestDescriptor.java
@@ -26,16 +26,16 @@
 import org.junit.jupiter.api.extension.Extension;
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.api.extension.TestExecutionExceptionHandler;
-import org.junit.jupiter.api.function.Executable;
 import org.junit.jupiter.engine.execution.AfterEachMethodAdapter;
 import org.junit.jupiter.engine.execution.BeforeEachMethodAdapter;
 import org.junit.jupiter.engine.execution.ExecutableInvoker;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
-import org.junit.jupiter.engine.execution.ThrowableCollector;
 import org.junit.jupiter.engine.extension.ExtensionRegistry;
 import org.junit.platform.commons.util.ExceptionUtils;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.UniqueId;
+import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
+import org.junit.platform.engine.support.hierarchical.ThrowableCollector.Executable;
 
 /**
  * {@link TestDescriptor} for tests based on Java methods.

diff --git a/...jupiter-engine/src/main/java/org/junit/jupiter/engine/execution/ExtensionValuesStore.java b/...jupiter-engine/src/main/java/org/junit/jupiter/engine/execution/ExtensionValuesStore.java
@@ -26,6 +26,7 @@
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
 import org.junit.jupiter.api.extension.ExtensionContextException;
+import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
 
 /**
  * {@code ExtensionValuesStore} is used inside implementations of

diff --git a/...ngine/src/main/java/org/junit/jupiter/engine/execution/JupiterEngineExecutionContext.java b/...ngine/src/main/java/org/junit/jupiter/engine/execution/JupiterEngineExecutionContext.java
@@ -21,6 +21,7 @@
 import org.junit.platform.engine.ConfigurationParameters;
 import org.junit.platform.engine.EngineExecutionListener;
 import org.junit.platform.engine.support.hierarchical.EngineExecutionContext;
+import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
 
 /**
  * @since 5.0

diff --git a/...ine/src/test/java/org/junit/jupiter/engine/descriptor/TestFactoryTestDescriptorTests.java b/...ine/src/test/java/org/junit/jupiter/engine/descriptor/TestFactoryTestDescriptorTests.java
@@ -24,9 +24,9 @@
 import org.junit.jupiter.api.TestFactory;
 import org.junit.jupiter.api.extension.ExtensionContext;
 import org.junit.jupiter.engine.execution.JupiterEngineExecutionContext;
-import org.junit.jupiter.engine.execution.ThrowableCollector;
 import org.junit.platform.engine.UniqueId;
 import org.junit.platform.engine.support.hierarchical.Node;
+import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
 
 /**
  * Unit tests for {@link TestFactoryTestDescriptor}.

diff --git a/...upiter-engine/src/test/java/org/junit/jupiter/engine/execution/ExtensionContextTests.java b/...upiter-engine/src/test/java/org/junit/jupiter/engine/execution/ExtensionContextTests.java
@@ -43,6 +43,7 @@
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.UniqueId;
 import org.junit.platform.engine.reporting.ReportEntry;
+import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
 import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
 

diff --git a/...rc/main/java/org/junit/platform/engine/support/hierarchical/HierarchicalTestExecutor.java b/...rc/main/java/org/junit/platform/engine/support/hierarchical/HierarchicalTestExecutor.java
@@ -10,17 +10,10 @@
 
 package org.junit.platform.engine.support.hierarchical;
 
-import static org.junit.platform.commons.util.BlacklistedExceptions.rethrowIfBlacklisted;
-import static org.junit.platform.engine.TestExecutionResult.Status.FAILED;
-
-import java.util.ArrayList;
-import java.util.List;
-
 import org.junit.platform.engine.EngineExecutionListener;
 import org.junit.platform.engine.ExecutionRequest;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.TestEngine;
-import org.junit.platform.engine.TestExecutionResult;
 import org.junit.platform.engine.support.hierarchical.Node.SkipResult;
 
 /**
@@ -39,8 +32,6 @@
  */
 class HierarchicalTestExecutor<C extends EngineExecutionContext> {
 
-	private static final SingleTestExecutor singleTestExecutor = new SingleTestExecutor();
-
 	private final TestDescriptor rootTestDescriptor;
 	private final EngineExecutionListener listener;
 	private final C rootContext;
@@ -59,10 +50,10 @@ class NodeExecutor {
 
 		private final TestDescriptor testDescriptor;
 		private final Node<C> node;
-		private final List<Throwable> executionErrors = new ArrayList<>();
+		private final ThrowableCollector throwableCollector = new ThrowableCollector();
 		private C context;
 		private SkipResult skipResult;
-		private TestExecutionResult executionResult;
+		private boolean started;
 
 		NodeExecutor(TestDescriptor testDescriptor) {
 			this.testDescriptor = testDescriptor;
@@ -71,127 +62,50 @@ class NodeExecutor {
 
 		void execute(C parentContext, ExecutionTracker tracker) {
 			tracker.markExecuted(testDescriptor);
-			prepare(parentContext);
-			if (executionErrors.isEmpty()) {
-				checkWhetherSkipped();
+			throwableCollector.execute(() -> context = node.prepare(parentContext));
+			if (throwableCollector.isEmpty()) {
+				throwableCollector.execute(() -> skipResult = node.shouldBeSkipped(context));
 			}
-			if (executionErrors.isEmpty() && !skipResult.isSkipped()) {
+			if (throwableCollector.isEmpty() && !skipResult.isSkipped()) {
 				executeRecursively(tracker);
 			}
 			if (context != null) {
-				cleanUp();
+				throwableCollector.execute(() -> node.cleanUp(context));
 			}
 			reportDone();
 		}
 
-		private void prepare(C parentContext) {
-			try {
-				context = node.prepare(parentContext);
-			}
-			catch (Throwable t) {
-				addExecutionError(t);
-			}
-		}
-
-		private void checkWhetherSkipped() {
-			try {
-				skipResult = node.shouldBeSkipped(context);
-			}
-			catch (Throwable t) {
-				addExecutionError(t);
-			}
-		}
-
 		private void executeRecursively(ExecutionTracker tracker) {
 			listener.executionStarted(testDescriptor);
+			started = true;
 
-			executionResult = singleTestExecutor.executeSafely(() -> {
-				Throwable failure = null;
-				try {
-					context = node.before(context);
-
-					context = node.execute(context, dynamicTestDescriptor -> {
-						listener.dynamicTestRegistered(dynamicTestDescriptor);
-						new NodeExecutor(dynamicTestDescriptor).execute(context, tracker);
-					});
-
-					// @formatter:off
-					testDescriptor.getChildren().stream()
-							.filter(child -> !tracker.wasAlreadyExecuted(child))
-							.forEach(child -> new NodeExecutor(child).execute(context, tracker));
-					// @formatter:on
-				}
-				catch (Throwable t) {
-					failure = t;
-				}
-				finally {
-					executeAfter(failure);
-				}
-			});
-		}
+			throwableCollector.execute(() -> {
+				context = node.before(context);
 
-		private void executeAfter(Throwable failure) throws Throwable {
-			try {
-				node.after(context);
-				if (failure != null) {
-					throw failure;
-				}
-			}
-			catch (Throwable t) {
-				if (failure != null && failure != t) {
-					failure.addSuppressed(t);
-					throw failure;
-				}
-				throw t;
-			}
-		}
+				context = node.execute(context, dynamicTestDescriptor -> {
+					listener.dynamicTestRegistered(dynamicTestDescriptor);
+					new NodeExecutor(dynamicTestDescriptor).execute(context, tracker);
+				});
 
-		private void cleanUp() {
-			try {
-				node.cleanUp(context);
-			}
-			catch (Throwable t) {
-				addExecutionError(t);
-			}
+				// @formatter:off
+				testDescriptor.getChildren().stream()
+						.filter(child -> !tracker.wasAlreadyExecuted(child))
+						.forEach(child -> new NodeExecutor(child).execute(context, tracker));
+				// @formatter:on
+			});
+			throwableCollector.execute(() -> node.after(context));
 		}
 
 		private void reportDone() {
-			if (executionResult != null) {
-				addExecutionErrorsToTestExecutionResult();
-				listener.executionFinished(testDescriptor, executionResult);
-			}
-			else if (executionErrors.isEmpty() && skipResult.isSkipped()) {
+			if (throwableCollector.isEmpty() && skipResult.isSkipped()) {
 				listener.executionSkipped(testDescriptor, skipResult.getReason().orElse("<unknown>"));
+				return;
 			}
-			else {
+			if (!started) {
 				// Call executionStarted first to comply with the contract of EngineExecutionListener.
 				listener.executionStarted(testDescriptor);
-				listener.executionFinished(testDescriptor, createTestExecutionResultFromExecutionErrors());
 			}
-		}
-
-		private void addExecutionErrorsToTestExecutionResult() {
-			if (executionErrors.isEmpty()) {
-				return;
-			}
-			if (executionResult.getStatus() == FAILED && executionResult.getThrowable().isPresent()) {
-				Throwable throwable = executionResult.getThrowable().get();
-				executionErrors.forEach(throwable::addSuppressed);
-			}
-			else {
-				executionResult = createTestExecutionResultFromExecutionErrors();
-			}
-		}
-
-		private TestExecutionResult createTestExecutionResultFromExecutionErrors() {
-			Throwable throwable = executionErrors.get(0);
-			executionErrors.stream().skip(1).forEach(throwable::addSuppressed);
-			return TestExecutionResult.failed(throwable);
-		}
-
-		private void addExecutionError(Throwable throwable) {
-			rethrowIfBlacklisted(throwable);
-			executionErrors.add(throwable);
+			listener.executionFinished(testDescriptor, throwableCollector.toTestExecutionResult());
 		}
 	}
 

diff --git a/...gine/src/main/java/org/junit/platform/engine/support/hierarchical/SingleTestExecutor.java b/...gine/src/main/java/org/junit/platform/engine/support/hierarchical/SingleTestExecutor.java