From a3e4b97efb03bd7b52d18c17b9dc185d112a04e8 Mon Sep 17 00:00:00 2001 From: Marc Philipp Date: Wed, 3 May 2017 11:52:54 +0200 Subject: [PATCH] Replace Node.isLeaf() with explicit tracking of executed descriptors Fixes #812. --- .../docs/asciidoc/release-notes-5.0.0-M5.adoc | 2 ++ .../descriptor/JupiterTestDescriptor.java | 5 --- .../descriptor/TestFactoryTestDescriptor.java | 5 --- .../TestTemplateTestDescriptor.java | 5 --- .../hierarchical/ExecutionTracker.java | 33 +++++++++++++++++++ .../HierarchicalTestExecutor.java | 24 +++++++------- .../engine/support/hierarchical/Node.java | 9 ----- .../DemoHierarchicalContainerDescriptor.java | 5 --- .../DemoHierarchicalEngineDescriptor.java | 5 --- .../DemoHierarchicalTestDescriptor.java | 5 --- .../HierarchicalTestExecutorTests.java | 11 ------- 11 files changed, 47 insertions(+), 62 deletions(-) create mode 100644 junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ExecutionTracker.java diff --git a/documentation/src/docs/asciidoc/release-notes-5.0.0-M5.adoc b/documentation/src/docs/asciidoc/release-notes-5.0.0-M5.adoc index 6eaa65bd4158..6b120ddeb086 100644 --- a/documentation/src/docs/asciidoc/release-notes-5.0.0-M5.adoc +++ b/documentation/src/docs/asciidoc/release-notes-5.0.0-M5.adoc @@ -29,6 +29,8 @@ on GitHub. - `junit-platform-commons`: `ReflectionUtils.findAllClassesInClasspathRoot(Path, Predicate, Predicate)` * The `junit-platform-surefire-provider` now requires `maven-surefire-plugin` version 2.20 or higher. +* The `isLeaf()` method of the `org.junit.platform.engine.support.hierarchical.Node` + interface has been removed. ===== New Features and Improvements diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java index 5c4e2056ac7b..e352dc26bba9 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/JupiterTestDescriptor.java @@ -94,11 +94,6 @@ protected static String determineDisplayName(E elem // --- Node ---------------------------------------------------------------- - @Override - public boolean isLeaf() { - return !isContainer(); - } - protected SkipResult shouldContainerBeSkipped(JupiterEngineExecutionContext context) { ConditionEvaluationResult evaluationResult = conditionEvaluator.evaluateForContainer( context.getExtensionRegistry(), context.getConfigurationParameters(), diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestFactoryTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestFactoryTestDescriptor.java index 248be9342f2e..98aaf5af7a43 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestFactoryTestDescriptor.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestFactoryTestDescriptor.java @@ -58,11 +58,6 @@ public boolean hasTests() { // --- Node ---------------------------------------------------------------- - @Override - public boolean isLeaf() { - return true; - } - @Override protected void invokeTestMethod(JupiterEngineExecutionContext context, DynamicTestExecutor dynamicTestExecutor) { TestExtensionContext testExtensionContext = (TestExtensionContext) context.getExtensionContext(); diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java index a044a17ade39..b08613da0018 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java @@ -53,11 +53,6 @@ public boolean hasTests() { // --- Node ---------------------------------------------------------------- - @Override - public boolean isLeaf() { - return true; - } - @Override public JupiterEngineExecutionContext prepare(JupiterEngineExecutionContext context) throws Exception { ExtensionRegistry registry = populateNewExtensionRegistryFromExtendWith(getTestMethod(), diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ExecutionTracker.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ExecutionTracker.java new file mode 100644 index 000000000000..3b6038349448 --- /dev/null +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/ExecutionTracker.java @@ -0,0 +1,33 @@ +/* + * Copyright 2015-2017 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v1.0 which + * accompanies this distribution and is available at + * + * http://www.eclipse.org/legal/epl-v10.html + */ + +package org.junit.platform.engine.support.hierarchical; + +import java.util.HashSet; +import java.util.Set; + +import org.junit.platform.engine.TestDescriptor; +import org.junit.platform.engine.UniqueId; + +/** + * @since 1.0 + */ +class ExecutionTracker { + + private final Set executedUniqueIds = new HashSet<>(); + + void markExecuted(TestDescriptor testDescriptor) { + executedUniqueIds.add(testDescriptor.getUniqueId()); + } + + boolean wasAlreadyExecuted(TestDescriptor testDescriptor) { + return executedUniqueIds.contains(testDescriptor.getUniqueId()); + } +} diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/HierarchicalTestExecutor.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/HierarchicalTestExecutor.java index 5e810f9f80d9..4d780c2c2f24 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/HierarchicalTestExecutor.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/HierarchicalTestExecutor.java @@ -48,11 +48,12 @@ class HierarchicalTestExecutor { } void execute() { - execute(this.rootTestDescriptor, this.rootContext); + execute(this.rootTestDescriptor, this.rootContext, new ExecutionTracker()); } - private void execute(TestDescriptor testDescriptor, C parentContext) { + private void execute(TestDescriptor testDescriptor, C parentContext, ExecutionTracker tracker) { Node node = asNode(testDescriptor); + tracker.markExecuted(testDescriptor); C preparedContext; try { @@ -77,20 +78,19 @@ private void execute(TestDescriptor testDescriptor, C parentContext) { C context = preparedContext; try { context = node.before(context); - C dynamicTestContext = context; + + C contextForDynamicChildren = context; context = node.execute(context, dynamicTestDescriptor -> { this.listener.dynamicTestRegistered(dynamicTestDescriptor); - execute(dynamicTestDescriptor, dynamicTestContext); + execute(dynamicTestDescriptor, contextForDynamicChildren, tracker); }); - // If a node is NOT a leaf, execute its children recursively. - // Note: executing children for a leaf could result in accidental - // execution of dynamically added children. - if (!node.isLeaf()) { - for (TestDescriptor child : testDescriptor.getChildren()) { - execute(child, context); - } - } + C contextForStaticChildren = context; + // @formatter:off + testDescriptor.getChildren().stream() + .filter(child -> !tracker.wasAlreadyExecuted(child)) + .forEach(child -> execute(child, contextForStaticChildren, tracker)); + // @formatter:on } finally { node.after(context); diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/Node.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/Node.java index f0f6cb6af121..77e77be7c303 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/Node.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/hierarchical/Node.java @@ -29,15 +29,6 @@ @API(Experimental) public interface Node { - /** - * Determine if this {@code Node} is a leaf in the hierarchy. - * - *

The default implementation returns {@code false}. - */ - default boolean isLeaf() { - return false; - } - /** * Prepare the supplied {@code context} prior to execution. * diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalContainerDescriptor.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalContainerDescriptor.java index 3bd4d362838e..c1f586445aad 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalContainerDescriptor.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalContainerDescriptor.java @@ -46,11 +46,6 @@ public Type getType() { return Type.CONTAINER; } - @Override - public boolean isLeaf() { - return false; - } - @Override public boolean hasTests() { return true; diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalEngineDescriptor.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalEngineDescriptor.java index b5600b4e6c32..c1d565966086 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalEngineDescriptor.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalEngineDescriptor.java @@ -50,9 +50,4 @@ public DemoEngineExecutionContext before(DemoEngineExecutionContext context) thr return context; } - @Override - public boolean isLeaf() { - return false; - } - } diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalTestDescriptor.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalTestDescriptor.java index 8d70b4507c98..42fe78f067a8 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalTestDescriptor.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/DemoHierarchicalTestDescriptor.java @@ -44,11 +44,6 @@ public Type getType() { return this.executeBlock != null ? Type.TEST : Type.CONTAINER; } - @Override - public boolean isLeaf() { - return isTest(); - } - public void markSkipped(String reason) { this.skipped = true; this.skippedReason = reason; diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/HierarchicalTestExecutorTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/HierarchicalTestExecutorTests.java index 25ee2d567d3e..36ccf3bee784 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/HierarchicalTestExecutorTests.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/hierarchical/HierarchicalTestExecutorTests.java @@ -407,12 +407,6 @@ protected MyContainer(UniqueId uniqueId) { public Type getType() { return Type.CONTAINER; } - - @Override - public boolean isLeaf() { - return isTest(); - } - } private static class MyLeaf extends AbstractTestDescriptor implements Node { @@ -431,11 +425,6 @@ public MyEngineExecutionContext execute(MyEngineExecutionContext context, public Type getType() { return Type.TEST; } - - @Override - public boolean isLeaf() { - return isTest(); - } } private static class MyExecutor extends HierarchicalTestExecutor {