Pull up isTestDisabled() behavior into TestExecutionNode

junit-team · Nov 15, 2015 · 87cff3b · 87cff3b
1 parent fcc6649
commit 87cff3b
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 26 deletions.
diff --git a/...5-engine/src/main/java/org/junit/gen5/engine/junit5/execution/ClassTestExecutionNode.java b/...5-engine/src/main/java/org/junit/gen5/engine/junit5/execution/ClassTestExecutionNode.java
@@ -10,8 +10,9 @@
 
 package org.junit.gen5.engine.junit5.execution;
 
-import static org.junit.gen5.commons.util.AnnotationUtils.*;
+import static org.junit.gen5.commons.util.AnnotationUtils.findAnnotatedMethods;
-import static org.junit.gen5.commons.util.ReflectionUtils.*;
+import static org.junit.gen5.commons.util.ReflectionUtils.invokeMethod;
+import static org.junit.gen5.commons.util.ReflectionUtils.newInstance;
 
 import java.lang.reflect.Method;
 import java.util.List;
@@ -26,7 +27,6 @@
 import org.junit.gen5.commons.util.ReflectionUtils.MethodSortOrder;
 import org.junit.gen5.engine.ExecutionRequest;
 import org.junit.gen5.engine.junit5.descriptor.ClassTestDescriptor;
-import org.opentestalliance.TestSkippedException;
 
 /**
  * @author Stefan Bechtold
@@ -37,8 +37,6 @@ class ClassTestExecutionNode extends TestExecutionNode {
 
 	private final ClassTestDescriptor testDescriptor;
 
-	private final ConditionEvaluator conditionEvaluator = new ConditionEvaluator();
-
 	ClassTestExecutionNode(ClassTestDescriptor testDescriptor) {
 		this.testDescriptor = testDescriptor;
 	}
@@ -50,19 +48,12 @@ public ClassTestDescriptor getTestDescriptor() {
 
 	@Override
 	public void execute(ExecutionRequest request, TestExecutionContext context) {
-		Class<?> testClass = context.getTestClass().get();
+		if (isTestDisabled(request, context)) {
-
-		Result result = this.conditionEvaluator.evaluate(context);
-		if (!result.isSuccess()) {
-			// TODO Determine if we really need an explicit TestSkippedException.
-			TestSkippedException testSkippedException = new TestSkippedException(String.format(
-				"Skipping test class [%s]; reason: %s", testClass.getName(), result.getReason().orElse("unknown")));
-			request.getTestExecutionListener().testSkipped(getTestDescriptor(), testSkippedException);
-
 			// Abort execution of the test completely at this point.
 			return;
 		}
 
+		Class<?> testClass = context.getTestClass().get();
 		Object testInstance = createTestInstance();
 
 		try {
@@ -79,6 +70,12 @@ public void execute(ExecutionRequest request, TestExecutionContext context) {
 		}
 	}
 
+	@Override
+	protected String buildTestSkippedMessage(Result result, TestExecutionContext context) {
+		return String.format("Skipping test class [%s]; reason: %s", context.getTestClass().get().getName(),
+			result.getReason().orElse("unknown"));
+	}
+
 	private void executeBeforeAllMethods(Class<?> testClass, Object testInstance) throws Exception {
 		for (Method method : findAnnotatedMethods(testClass, BeforeAll.class, MethodSortOrder.HierarchyDown)) {
 			invokeMethod(method, testInstance);

diff --git a/...-engine/src/main/java/org/junit/gen5/engine/junit5/execution/MethodTestExecutionNode.java b/...-engine/src/main/java/org/junit/gen5/engine/junit5/execution/MethodTestExecutionNode.java
@@ -33,8 +33,6 @@ class MethodTestExecutionNode extends TestExecutionNode {
 
 	private final MethodTestDescriptor testDescriptor;
 
-	private final ConditionEvaluator conditionEvaluator = new ConditionEvaluator();
-
 	MethodTestExecutionNode(MethodTestDescriptor testDescriptor) {
 		this.testDescriptor = testDescriptor;
 	}
@@ -46,16 +44,7 @@ public MethodTestDescriptor getTestDescriptor() {
 
 	@Override
 	public void execute(ExecutionRequest request, TestExecutionContext context) {
-		final Method testMethod = getTestDescriptor().getTestMethod();
+		if (isTestDisabled(request, context)) {
-
-		Result result = this.conditionEvaluator.evaluate(context);
-		if (!result.isSuccess()) {
-			// TODO Determine if we really need an explicit TestSkippedException.
-			TestSkippedException testSkippedException = new TestSkippedException(
-				String.format("Skipping test method [%s]; reason: %s", testMethod.toGenericString(),
-					result.getReason().orElse("unknown")));
-			request.getTestExecutionListener().testSkipped(getTestDescriptor(), testSkippedException);
-
 			// Abort execution of the test completely at this point.
 			return;
 		}
@@ -93,6 +82,12 @@ else if (exceptionThrown instanceof TestAbortedException) {
 		}
 	}
 
+	@Override
+	protected String buildTestSkippedMessage(Result result, TestExecutionContext context) {
+		return String.format("Skipping test method [%s]; reason: %s", context.getTestMethod().get().toGenericString(),
+			result.getReason().orElse("unknown"));
+	}
+
 	private void invokeTestMethod(Method method, TestExecutionContext context) {
 		Object target = context.getTestInstance().get();
 		invokeMethodInContext(method, context, context.getArgumentResolvers(), target);

diff --git a/junit5-engine/src/main/java/org/junit/gen5/engine/junit5/execution/TestExecutionNode.java b/junit5-engine/src/main/java/org/junit/gen5/engine/junit5/execution/TestExecutionNode.java
@@ -17,10 +17,12 @@
 import java.util.List;
 import java.util.Set;
 
+import org.junit.gen5.api.Condition.Result;
 import org.junit.gen5.api.extension.MethodArgumentResolver;
 import org.junit.gen5.api.extension.TestExecutionContext;
 import org.junit.gen5.engine.ExecutionRequest;
 import org.junit.gen5.engine.TestDescriptor;
+import org.opentestalliance.TestSkippedException;
 
 /**
  * @author Stefan Bechtold
@@ -33,6 +35,8 @@ public abstract class TestExecutionNode {
 
 	private List<TestExecutionNode> children = new LinkedList<>();
 
+	private final ConditionEvaluator conditionEvaluator = new ConditionEvaluator();
+
 	public void addChild(TestExecutionNode child) {
 		this.children.add(child);
 		child.parent = this;
@@ -50,6 +54,22 @@ public final List<TestExecutionNode> getChildren() {
 
 	public abstract void execute(ExecutionRequest request, TestExecutionContext context);
 
+	protected final boolean isTestDisabled(ExecutionRequest request, TestExecutionContext context) {
+		Result result = this.conditionEvaluator.evaluate(context);
+		if (!result.isSuccess()) {
+			// TODO Determine if we really need an explicit TestSkippedException.
+			request.getTestExecutionListener().testSkipped(getTestDescriptor(),
+				new TestSkippedException(buildTestSkippedMessage(result, context)));
+			return true;
+		}
+		return false;
+	}
+
+	protected String buildTestSkippedMessage(Result result, TestExecutionContext context) {
+		return String.format("Skipping [%s]; reason: %s", context.getDisplayName(),
+			result.getReason().orElse("unknown"));
+	}
+
 	protected void executeChild(TestExecutionNode child, ExecutionRequest request, TestExecutionContext parentContext,
 			Object testInstance) {