minor simplification

Author: peng

junit-platform-launcher/src/main/java/org/junit/platform/launcher/core/EngineDiscoveryResultValidator.java

@@ -34,19 +34,20 @@
  */
 class EngineDiscoveryResultValidator {
 
-	static abstract class ValidationError {
+	static abstract class ValidationError extends RuntimeException {
+		static final long serialVersionUID = -7034897190745766939L;
 
 		public abstract String message();
 	}
 
 	static class CyclicGraphError extends ValidationError {
-		TestDescriptor node1;
-		TestDescriptor node2;
+		static final long serialVersionUID = -7034897190745766939L;
+
+		LinkedList<TestDescriptor> p1;
+		LinkedList<TestDescriptor> p2;
 
 		@Override
 		public String message() {
-			LinkedList<TestDescriptor> p1 = getPath(node1);
-			LinkedList<TestDescriptor> p2 = getPath(node2);
 
 			StringBuilder result = new StringBuilder();
 			result.append("Graph cycle found:\n\t");
@@ -90,130 +91,132 @@ public void accept(TestDescriptor x) {
 
 	}
 
-	static class NonReciprocalParentError extends ValidationError {
-		TestDescriptor parent;
+
+	static class IllegalParentError extends ValidationError {
+		static final long serialVersionUID = -7034897190745766939L;
+		Optional<TestDescriptor> parent = Optional.empty();
 		TestDescriptor child;
 
 		@Override
 		public String message() {
-			String parent2 = null;
-			if (child.getParent().isPresent()) {
-				parent2 = child.getParent().get().getUniqueId().toString();
-			}
-			else {
-				parent2 = "<none>";
-			}
+			String parentStr = parent.map(v -> v.getUniqueId().toString()).orElse("<none>");
+			String parent2Str = child.getParent().map(v -> v.getUniqueId().toString()).orElse("<none>");
 
 			return String.format("%s is ill-formed: parent should be %s\n" + "\tbut found %s", child.getUniqueId(),
-				parent.getUniqueId(), parent2);
+					parentStr, parent2Str);
 		}
 	}
 
-	static class SelfReferringParentError extends ValidationError {
-		TestDescriptor node;
-
-		@Override
-		public String message() {
+	static class ValidatorExecution {
+		TestDescriptor root;
+		protected List<ValidationError> errors = new ArrayList<>();
 
-			return String.format("%s is ill-formed: parent cannot be itself", node.getUniqueId());
+		ValidatorExecution(TestDescriptor root) {
+			this.root = root;
 		}
-	}
-
-	/**
-	 *  Perform common validation checks.
-	 *
-	 * @throws org.junit.platform.commons.PreconditionViolationException if any check fails
-	 */
-	void validate(TestEngine testEngine, TestDescriptor root) {
-		Preconditions.notNull(root,
-			() -> String.format(
-				"The discover() method for TestEngine with ID '%s' must return a non-null root TestDescriptor.",
-				testEngine.getId()));
-		Optional<String> msgs = getValidationErrorMsg(root);
-		msgs.ifPresent(s -> Preconditions.condition(true,
-			() -> String.format("The discover() method for TestEngine with ID '%s' returned a cyclic graph:\n" + "%s",
-				testEngine.getId(), s)));
 
-	}
+		void verifyParentChild(Optional<TestDescriptor> parent, TestDescriptor child) {
+			Optional<TestDescriptor> effectiveParent = parent.filter(v -> !v.equals(child));
+			Optional<TestDescriptor> parent2 = child.getParent();
+			if (!parent2.equals(effectiveParent)) {
+				IllegalParentError error = new IllegalParentError();
+				error.parent = effectiveParent;
+				error.child = child;
 
-	/**
-	 * @return {@code true} if the tree does <em>not</em> contain a cycle; else {@code false}.
-	 */
-	boolean hasNoError(TestDescriptor root) {
-		return getValidationErrors(root).isEmpty();
-	}
+				errors.add(error);
 
-	static LinkedList<TestDescriptor> getPath(TestDescriptor v) {
+				throw error;
+			}
+		}
 
-		LinkedList<TestDescriptor> path = new LinkedList<>();
-		path.add(v);
+		 LinkedList<TestDescriptor> getPath(TestDescriptor v) {
 
-		Optional<TestDescriptor> next = v.getParent();
-		while (next.isPresent()) {
-			TestDescriptor parent = next.get();
-			path.add(next.get());
-			next = parent.getParent();
-		}
+			LinkedList<TestDescriptor> path = new LinkedList<>();
+			path.add(v);
 
-		return path;
-	}
+			Optional<TestDescriptor> next = v.getParent();
+			while (next.isPresent()) {
+				TestDescriptor current = next.get();
+				path.add(next.get());
+				next = current.getParent();
+				verifyParentChild(next, current);
+			}
 
-	Optional<String> getValidationErrorMsg(TestDescriptor root) {
+			return path;
+		}
 
-		List<ValidationError> errors = getValidationErrors(root);
+		List<ValidationError> getErrors() {
 
-		Stream<String> strs = errors.stream().map(ValidationError::message);
+			Map<UniqueId, TestDescriptor> visited = new HashMap<>();
+			visited.put(root.getUniqueId(), root);
 
-		return strs.reduce((a, b) -> a.concat("\n").concat(b));
-	}
+			Queue<TestDescriptor> queue = new ArrayDeque<>();
+			queue.add(root);
 
-	List<ValidationError> getValidationErrors(TestDescriptor root) {
-		Map<UniqueId, TestDescriptor> visited = new HashMap<>();
-		visited.put(root.getUniqueId(), root);
+			try {
+				while (!queue.isEmpty()) {
+					TestDescriptor next = queue.remove();
+					for (TestDescriptor child : next.getChildren()) {
+						UniqueId uid = child.getUniqueId();
 
-		Queue<TestDescriptor> queue = new ArrayDeque<>();
-		queue.add(root);
+						verifyParentChild(Optional.of(next), child);
 
-		List<ValidationError> errors = new ArrayList<>();
+						TestDescriptor existing = visited.put(uid, child);
+						if (existing != null) {
+							CyclicGraphError error = new CyclicGraphError();
+							error.p1 = getPath(existing);
+							error.p2 = getPath(child);
 
-		while (!queue.isEmpty()) {
-			TestDescriptor next = queue.remove();
-			for (TestDescriptor child : next.getChildren()) {
-				UniqueId uid = child.getUniqueId();
-				Optional<TestDescriptor> next2 = child.getParent();
-				if (!next2.equals(Optional.of(next))) {
-					NonReciprocalParentError error = new NonReciprocalParentError();
-					error.parent = next;
-					error.child = child;
-					errors.add(error);
+							errors.add(error);
 
-					return errors;
+							throw error;
+						}
+						if (child.isContainer()) {
+							queue.add(child);
+						}
+					}
 				}
+			}
+			catch ( ValidationError ignored) {
+			}
+			return errors;
+		}
 
-				if (next2.equals(Optional.of(child))) {
-					SelfReferringParentError error = new SelfReferringParentError();
-					error.node = child;
-					errors.add(error);
 
-					return errors;
-				}
+		Optional<String> getErrorMsg() {
 
-				TestDescriptor existingOrNull = visited.put(uid, child);
-				if (existingOrNull != null) {
-					CyclicGraphError error = new CyclicGraphError();
-					error.node1 = existingOrNull;
-					error.node2 = child;
+			List<ValidationError> errors = getErrors();
 
-					errors.add(error);
+			Stream<String> strs = errors.stream().map(ValidationError::message);
 
-					return errors; // id already known: cycle detected!
-				}
-				if (child.isContainer()) {
-					queue.add(child);
-				}
-			}
+			return strs.reduce((a, b) -> a.concat("\n").concat(b));
 		}
-		return errors;
 	}
 
+	/**
+	 *  Perform common validation checks.
+	 *
+	 * @throws org.junit.platform.commons.PreconditionViolationException if any check fails
+	 */
+	void validate(TestEngine testEngine, TestDescriptor root) {
+		Preconditions.notNull(root,
+				() -> String.format(
+						"The discover() method for TestEngine with ID '%s' must return a non-null root TestDescriptor.",
+						testEngine.getId()));
+		Optional<String> msgs = new ValidatorExecution(root).getErrorMsg();
+		msgs.ifPresent(s -> Preconditions.condition(true,
+				() -> String.format("The discover() method for TestEngine with ID '%s' returned a cyclic graph:\n" + "%s",
+						testEngine.getId(), s)));
+	}
+
+	Optional<String> getErrorMsg(TestDescriptor root) {
+		return new ValidatorExecution(root).getErrorMsg();
+	}
+
+	/**
+	 * @return {@code true} if the tree does <em>not</em> contain a cycle; else {@code false}.
+	 */
+	boolean hasNoError(TestDescriptor root) {
+		return new ValidatorExecution(root).getErrors().isEmpty();
+	}
 }

platform-tests/src/test/java/org/junit/platform/launcher/core/EngineDiscoveryResultValidatorTests.java

@@ -33,8 +33,9 @@ void detectCycleWithDoubleRoot() {
 
 		root.addChild(root);
 		assertFalse(validator.hasNoError(root));
-		assertEquals("[engine:root] is ill-formed: parent cannot be itself",
-			validator.getValidationErrorMsg(root).get());
+		assertEquals("[engine:root] is ill-formed: parent should be <none>\n" +
+						"\tbut found [engine:root]",
+			validator.getErrorMsg(root).get());
 	}
 
 	@Test
@@ -51,7 +52,7 @@ void detectCycleWithDoubleGroup() {
 		assertFalse(validator.hasNoError(root));
 		assertEquals("[engine:root]/[group:1] is ill-formed: parent should be [engine:root]\n"
 				+ "\tbut found [engine:root]/[group:2]",
-			validator.getValidationErrorMsg(root).get());
+			validator.getErrorMsg(root).get());
 	}
 
 	@Test
@@ -72,6 +73,6 @@ void detectCycleWithDoubleTest() {
 		assertFalse(validator.hasNoError(root));
 		assertEquals("[engine:root]/[test:1] is ill-formed: parent should be [engine:root]/[group:1]\n"
 				+ "\tbut found [engine:root]/[group:2]",
-			validator.getValidationErrorMsg(root).get());
+			validator.getErrorMsg(root).get());
 	}
 }