Skip to content

Commit 6c66c9d

Browse files
pengpeng
authored andcommitted
use single class imports
EngineDiscoveryResultValidator now generate more verbose error messages Improve existing test cases of EngineDiscoveryResultValidator getFirstCyclicUID now returns optional ramp up tests for it add more verbose error message for cyclic UID in discovery
1 parent b979cd7 commit 6c66c9d

File tree

2 files changed

+180
-19
lines changed

2 files changed

+180
-19
lines changed

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

Lines changed: 165 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,16 @@
1111
package org.junit.platform.launcher.core;
1212

1313
import java.util.ArrayDeque;
14-
import java.util.HashSet;
14+
import java.util.ArrayList;
15+
import java.util.HashMap;
16+
import java.util.Iterator;
17+
import java.util.LinkedList;
18+
import java.util.List;
19+
import java.util.Map;
20+
import java.util.Optional;
1521
import java.util.Queue;
16-
import java.util.Set;
22+
import java.util.function.Consumer;
23+
import java.util.stream.Stream;
1724

1825
import org.junit.platform.commons.util.Preconditions;
1926
import org.junit.platform.engine.TestDescriptor;
@@ -27,6 +34,91 @@
2734
*/
2835
class EngineDiscoveryResultValidator {
2936

37+
static abstract class ValidationError {
38+
39+
public abstract String message();
40+
}
41+
42+
static class CyclicGraphError extends ValidationError {
43+
TestDescriptor node1;
44+
TestDescriptor node2;
45+
46+
@Override
47+
public String message() {
48+
LinkedList<TestDescriptor> p1 = getPath(node1);
49+
LinkedList<TestDescriptor> p2 = getPath(node2);
50+
51+
StringBuilder result = new StringBuilder();
52+
result.append("Graph cycle found:\n\t");
53+
54+
Iterator<TestDescriptor> itr1 = p1.descendingIterator();
55+
Iterator<TestDescriptor> itr2 = p2.descendingIterator();
56+
57+
while (itr1.hasNext() && itr2.hasNext()) {
58+
TestDescriptor x1 = itr1.next();
59+
if (x1 == itr2.next()) {
60+
result.append(" > ").append(x1.getUniqueId());
61+
}
62+
else {
63+
StringBuilder branch1 = new StringBuilder();
64+
itr1.forEachRemaining(new Consumer<TestDescriptor>() {
65+
@Override
66+
public void accept(TestDescriptor x) {
67+
branch1.append(" > ").append(x.getUniqueId());
68+
}
69+
});
70+
71+
StringBuilder branch2 = new StringBuilder();
72+
itr2.forEachRemaining(new Consumer<TestDescriptor>() {
73+
@Override
74+
public void accept(TestDescriptor x) {
75+
branch2.append(" > ").append(x.getUniqueId());
76+
}
77+
});
78+
79+
if (branch1.length() != 0)
80+
result.append("\n\t").append(branch1);
81+
if (branch2.length() != 0)
82+
result.append("\n\t").append(branch2);
83+
84+
return result.toString();
85+
}
86+
}
87+
88+
return result.toString();
89+
}
90+
91+
}
92+
93+
static class NonReciprocalParentError extends ValidationError {
94+
TestDescriptor parent;
95+
TestDescriptor child;
96+
97+
@Override
98+
public String message() {
99+
String parent2 = null;
100+
if (child.getParent().isPresent()) {
101+
parent2 = child.getParent().get().getUniqueId().toString();
102+
}
103+
else {
104+
parent2 = "<none>";
105+
}
106+
107+
return String.format("%s is ill-formed: parent should be %s\n" + "\tbut found %s", child.getUniqueId(),
108+
parent.getUniqueId(), parent2);
109+
}
110+
}
111+
112+
static class SelfReferringParentError extends ValidationError {
113+
TestDescriptor node;
114+
115+
@Override
116+
public String message() {
117+
118+
return String.format("%s is ill-formed: parent cannot be itself", node.getUniqueId());
119+
}
120+
}
121+
30122
/**
31123
* Perform common validation checks.
32124
*
@@ -37,30 +129,91 @@ void validate(TestEngine testEngine, TestDescriptor root) {
37129
() -> String.format(
38130
"The discover() method for TestEngine with ID '%s' must return a non-null root TestDescriptor.",
39131
testEngine.getId()));
40-
Preconditions.condition(isAcyclic(root),
41-
() -> String.format("The discover() method for TestEngine with ID '%s' returned a cyclic graph.",
42-
testEngine.getId()));
132+
Optional<String> msgs = getValidationErrorMsg(root);
133+
msgs.ifPresent(s -> Preconditions.condition(true,
134+
() -> String.format("The discover() method for TestEngine with ID '%s' returned a cyclic graph:\n" + "%s",
135+
testEngine.getId(), s)));
136+
43137
}
44138

45139
/**
46140
* @return {@code true} if the tree does <em>not</em> contain a cycle; else {@code false}.
47141
*/
48-
boolean isAcyclic(TestDescriptor root) {
49-
Set<UniqueId> visited = new HashSet<>();
50-
visited.add(root.getUniqueId());
142+
boolean hasNoError(TestDescriptor root) {
143+
return getValidationErrors(root).isEmpty();
144+
}
145+
146+
static LinkedList<TestDescriptor> getPath(TestDescriptor v) {
147+
148+
LinkedList<TestDescriptor> path = new LinkedList<>();
149+
path.add(v);
150+
151+
Optional<TestDescriptor> next = v.getParent();
152+
while (next.isPresent()) {
153+
TestDescriptor parent = next.get();
154+
path.add(next.get());
155+
next = parent.getParent();
156+
}
157+
158+
return path;
159+
}
160+
161+
Optional<String> getValidationErrorMsg(TestDescriptor root) {
162+
163+
List<ValidationError> errors = getValidationErrors(root);
164+
165+
Stream<String> strs = errors.stream().map(ValidationError::message);
166+
167+
return strs.reduce((a, b) -> a.concat("\n").concat(b));
168+
}
169+
170+
List<ValidationError> getValidationErrors(TestDescriptor root) {
171+
Map<UniqueId, TestDescriptor> visited = new HashMap<>();
172+
visited.put(root.getUniqueId(), root);
173+
51174
Queue<TestDescriptor> queue = new ArrayDeque<>();
52175
queue.add(root);
176+
177+
List<ValidationError> errors = new ArrayList<>();
178+
53179
while (!queue.isEmpty()) {
54-
for (TestDescriptor child : queue.remove().getChildren()) {
55-
if (!visited.add(child.getUniqueId())) {
56-
return false; // id already known: cycle detected!
180+
TestDescriptor next = queue.remove();
181+
for (TestDescriptor child : next.getChildren()) {
182+
UniqueId uid = child.getUniqueId();
183+
Optional<TestDescriptor> next2 = child.getParent();
184+
if (!next2.equals(Optional.of(next))) {
185+
NonReciprocalParentError error = new NonReciprocalParentError();
186+
error.parent = next;
187+
error.child = child;
188+
errors.add(error);
189+
190+
return errors;
191+
}
192+
193+
if (next2.equals(Optional.of(child))) {
194+
SelfReferringParentError error = new SelfReferringParentError();
195+
error.node = child;
196+
errors.add(error);
197+
198+
return errors;
199+
}
200+
201+
TestDescriptor existingOrNull = visited.put(uid, child);
202+
if (existingOrNull != null) {
203+
CyclicGraphError error = new CyclicGraphError();
204+
error.node1 = existingOrNull;
205+
error.node2 = child;
206+
207+
errors.add(error);
208+
209+
return errors; // id already known: cycle detected!
57210
}
58211
if (child.isContainer()) {
59212
queue.add(child);
60213
}
61214
}
62215
}
63-
return true;
216+
return errors;
64217
}
65218

66219
}

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
package org.junit.platform.launcher.core;
1212

13+
import static org.junit.jupiter.api.Assertions.assertEquals;
1314
import static org.junit.jupiter.api.Assertions.assertFalse;
1415
import static org.junit.jupiter.api.Assertions.assertTrue;
1516

@@ -28,10 +29,12 @@ class EngineDiscoveryResultValidatorTests {
2829
@Test
2930
void detectCycleWithDoubleRoot() {
3031
var root = new TestDescriptorStub(UniqueId.forEngine("root"), "root");
31-
assertTrue(validator.isAcyclic(root));
32+
assertTrue(validator.hasNoError(root));
3233

3334
root.addChild(root);
34-
assertFalse(validator.isAcyclic(root));
35+
assertFalse(validator.hasNoError(root));
36+
assertEquals("[engine:root] is ill-formed: parent cannot be itself",
37+
validator.getValidationErrorMsg(root).get());
3538
}
3639

3740
@Test
@@ -42,10 +45,13 @@ void detectCycleWithDoubleGroup() {
4245
TestDescriptor group2 = new TestDescriptorStub(rootId.append("group", "2"), "2");
4346
root.addChild(group1);
4447
root.addChild(group2);
45-
assertTrue(validator.isAcyclic(root));
48+
assertTrue(validator.hasNoError(root));
4649

4750
group2.addChild(group1);
48-
assertFalse(validator.isAcyclic(root));
51+
assertFalse(validator.hasNoError(root));
52+
assertEquals("[engine:root]/[group:1] is ill-formed: parent should be [engine:root]\n"
53+
+ "\tbut found [engine:root]/[group:2]",
54+
validator.getValidationErrorMsg(root).get());
4955
}
5056

5157
@Test
@@ -60,10 +66,12 @@ void detectCycleWithDoubleTest() {
6066
TestDescriptor test2 = new TestDescriptorStub(rootId.append("test", "2"), "2-2");
6167
group1.addChild(test1);
6268
group2.addChild(test2);
63-
assertTrue(validator.isAcyclic(root));
69+
assertTrue(validator.hasNoError(root));
6470

6571
group2.addChild(test1);
66-
assertFalse(validator.isAcyclic(root));
72+
assertFalse(validator.hasNoError(root));
73+
assertEquals("[engine:root]/[test:1] is ill-formed: parent should be [engine:root]/[group:1]\n"
74+
+ "\tbut found [engine:root]/[group:2]",
75+
validator.getValidationErrorMsg(root).get());
6776
}
68-
6977
}

0 commit comments

Comments
 (0)