#33: Split TestDescriptor into two interfaces

Fixes #33 and #43.

junit-engine-api/src/main/java/org/junit/gen5/engine/AbstractTestDescriptor.java

@@ -10,6 +10,8 @@
 
 package org.junit.gen5.engine;
 
+import static java.util.Collections.emptySet;
+
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
@@ -21,15 +23,15 @@
 /**
  * @since 5.0
  */
-public abstract class AbstractTestDescriptor implements TestDescriptor {
+public abstract class AbstractTestDescriptor implements MutableTestDescriptor {
 
 	private final String uniqueId;
 
-	private TestDescriptor parent;
+	private MutableTestDescriptor parent;
 
 	private TestSource source;
 
-	private final Set<TestDescriptor> children = new LinkedHashSet<>();
+	private final Set<MutableTestDescriptor> children = new LinkedHashSet<>();
 
 	protected AbstractTestDescriptor(String uniqueId) {
 		Preconditions.notBlank(uniqueId, "uniqueId must not be null or empty");
@@ -42,20 +44,19 @@ public final String getUniqueId() {
 	}
 
 	@Override
-	public Optional<TestDescriptor> getParent() {
+	public Optional<MutableTestDescriptor> getParent() {
 		return Optional.ofNullable(this.parent);
 	}
 
-	public final void setParent(TestDescriptor parent) {
+	@Override
+	public final void setParent(MutableTestDescriptor parent) {
 		this.parent = parent;
 	}
 
 	@Override
-	public void removeChild(TestDescriptor child) {
+	public void removeChild(MutableTestDescriptor child) {
 		this.children.remove(child);
-		if (child instanceof AbstractTestDescriptor) {
-			((AbstractTestDescriptor) child).setParent(null);
-		}
+		child.setParent(null);
 	}
 
 	protected void removeFromHierarchy() {
@@ -66,21 +67,13 @@ protected void removeFromHierarchy() {
 		this.children.clear();
 	}
 
-	public Set<TestDescriptor> allChildren() {
-		Set<TestDescriptor> all = new HashSet<>();
-		all.addAll(this.children);
-		for (TestDescriptor child : this.children) {
-			all.addAll(((AbstractTestDescriptor) child).allChildren());
-		}
-		return all;
-	}
-
-	public Optional<TestDescriptor> findByUniqueId(String uniqueId) {
+	@Override
+	public Optional<? extends TestDescriptor> findByUniqueId(String uniqueId) {
 		if (getUniqueId().equals(uniqueId)) {
 			return Optional.of(this);
 		}
 		for (TestDescriptor child : this.children) {
-			Optional<TestDescriptor> result = child.findByUniqueId(uniqueId);
+			Optional<? extends TestDescriptor> result = child.findByUniqueId(uniqueId);
 			if (result.isPresent()) {
 				return result;
 			}
@@ -89,16 +82,14 @@ public Optional<TestDescriptor> findByUniqueId(String uniqueId) {
 	}
 
 	@Override
-	public final void addChild(TestDescriptor child) {
+	public final void addChild(MutableTestDescriptor child) {
 		Preconditions.notNull(child, "child must not be null");
-		if (child instanceof AbstractTestDescriptor) {
-			((AbstractTestDescriptor) child).setParent(this);
-		}
+		child.setParent(this);
 		this.children.add(child);
 	}
 
 	@Override
-	public final Set<TestDescriptor> getChildren() {
+	public final Set<MutableTestDescriptor> getChildren() {
 		return Collections.unmodifiableSet(this.children);
 	}
 
@@ -116,7 +107,7 @@ public void accept(Visitor visitor) {
 
 	@Override
 	public Set<TestTag> getTags() {
-		return Collections.emptySet();
+		return emptySet();
 	}
 
 	@Override

junit-engine-api/src/main/java/org/junit/gen5/engine/MutableTestDescriptor.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2015 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.gen5.engine;
+
+import java.util.Optional;
+import java.util.Set;
+
+/**
+ * @since 5.0
+ */
+public interface MutableTestDescriptor extends TestDescriptor {
+
+	@Override
+	Optional<MutableTestDescriptor> getParent();
+
+	void setParent(MutableTestDescriptor parent);
+
+	@Override
+	Set<MutableTestDescriptor> getChildren();
+
+	void addChild(MutableTestDescriptor descriptor);
+
+	void removeChild(MutableTestDescriptor descriptor);
+
+}

junit-engine-api/src/main/java/org/junit/gen5/engine/TestDescriptor.java

@@ -10,18 +10,14 @@
 
 package org.junit.gen5.engine;
 
+import java.util.HashSet;
 import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 
 /**
  * @since 5.0
  */
-// TODO Divide into public facing TestDescriptor and engine-internal MutableTestDescriptor.
-//
-// The tree of TestDescriptors should be parallel to (and not just a super type of)
-// MutableTestDescriptor so that clients won't rely on implementation details of
-// MutableTestDescriptor.
 public interface TestDescriptor {
 
 	/**
@@ -34,7 +30,7 @@ public interface TestDescriptor {
 
 	String getDisplayName();
 
-	Optional<TestDescriptor> getParent();
+	Optional<? extends TestDescriptor> getParent();
 
 	boolean isTest();
 
@@ -44,11 +40,16 @@ default boolean isRoot() {
 
 	Set<TestTag> getTags();
 
-	void addChild(TestDescriptor descriptor);
+	Set<? extends TestDescriptor> getChildren();
 
-	void removeChild(TestDescriptor descriptor);
-
-	Set<TestDescriptor> getChildren();
+	default Set<? extends TestDescriptor> allDescendants() {
+		Set<TestDescriptor> all = new HashSet<>();
+		all.addAll(getChildren());
+		for (TestDescriptor child : getChildren()) {
+			all.addAll(child.allDescendants());
+		}
+		return all;
+	}
 
 	default long countStaticTests() {
 		AtomicLong staticTests = new AtomicLong(0);
@@ -65,7 +66,7 @@ default boolean hasTests() {
 		return (isTest() || getChildren().stream().anyMatch(TestDescriptor::hasTests));
 	}
 
-	default Optional<TestDescriptor> findByUniqueId(String uniqueId) {
+	default Optional<? extends TestDescriptor> findByUniqueId(String uniqueId) {
 		if (getUniqueId().equals(uniqueId)) {
 			return Optional.of(this);
 		}

junit-launcher/src/main/java/org/junit/gen5/launcher/TestPlan.java

@@ -10,7 +10,12 @@
 
 package org.junit.gen5.launcher;
 
-import java.util.*;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Optional;
+import java.util.Set;
 
 import org.junit.gen5.engine.TestDescriptor;
 import org.junit.gen5.engine.TestEngine;
@@ -22,6 +27,7 @@
  * @since 5.0
  */
 public final class TestPlan implements TestDescriptor {
+
 	private final HashMap<String, TestDescriptor> engineRootTestDescriptors = new HashMap<>();
 
 	TestPlan() {
@@ -36,6 +42,7 @@ public Collection<TestDescriptor> getEngineRootTestDescriptors() {
 		return Collections.unmodifiableCollection(engineRootTestDescriptors.values());
 	}
 
+	@Override
 	public long countStaticTests() {
 		return this.engineRootTestDescriptors.values().stream().mapToLong(
 			engineDescriptor -> engineDescriptor.countStaticTests()).sum();
@@ -70,16 +77,6 @@ public Set<TestTag> getTags() {
 		return null;
 	}
 
-	@Override
-	public void addChild(TestDescriptor descriptor) {
-		throw new UnsupportedOperationException("Only use addTestDescriptor to add children");
-	}
-
-	@Override
-	public void removeChild(TestDescriptor descriptor) {
-		engineRootTestDescriptors.remove(descriptor);
-	}
-
 	@Override
 	public Set<TestDescriptor> getChildren() {
 		Set<TestDescriptor> children = new HashSet<>();
@@ -90,7 +87,7 @@ public Set<TestDescriptor> getChildren() {
 	@Override
 	public void accept(Visitor visitor) {
 		visitor.visit(this, () -> {
-			//the test plan itself will never be removed
+			// the test plan itself will never be removed
 		});
 		new HashSet<>(engineRootTestDescriptors.values()).forEach(child -> child.accept(visitor));
 	}

junit-launcher/src/main/java/org/junit/gen5/launcher/listeners/TestExecutionSummary.java

@@ -85,7 +85,8 @@ private String describeTest(TestDescriptor descriptor) {
 		return descriptionParts.stream().collect(Collectors.joining(":"));
 	}
 
-	private void collectTestDescription(Optional<TestDescriptor> optionalDescriptor, List<String> descriptionParts) {
+	private void collectTestDescription(Optional<? extends TestDescriptor> optionalDescriptor,
+			List<String> descriptionParts) {
 		optionalDescriptor.ifPresent(descriptor -> {
 			if (descriptor instanceof TestPlan) {
 			}