Skip to content

Commit 731d5df

Browse files
mpkorstanjemarcphilipp
mpkorstanje
and
marcphilipp
authored
Introduce API for ordering test descriptor children in place (#4289)
Resolves #4290. --------- Co-authored-by: Marc Philipp <mail@marcphilipp.de>
1 parent abd9489 commit 731d5df

File tree

8 files changed

+473
-56
lines changed

8 files changed

+473
-56
lines changed

‎documentation/src/docs/asciidoc/release-notes/release-notes-5.12.0-RC1.adoc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ repository on GitHub.
2626
[[release-notes-5.12.0-RC1-junit-platform-new-features-and-improvements]]
2727
==== New Features and Improvements
2828

29-
* ❓
29+
* New `TestDescriptor.orderChildren(UnaryOperator<List<TestDescriptor>> orderer)`
30+
method to order children in place
3031

3132

3233
[[release-notes-5.12.0-RC1-junit-jupiter]]

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/AbstractOrderingVisitor.java‎

Lines changed: 59 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -10,15 +10,16 @@
1010

1111
package org.junit.jupiter.engine.discovery;
1212

13+
import static java.util.Comparator.comparing;
1314
import static java.util.stream.Collectors.toCollection;
15+
import static java.util.stream.Collectors.toList;
1416

1517
import java.util.ArrayList;
16-
import java.util.LinkedHashSet;
18+
import java.util.HashMap;
1719
import java.util.List;
18-
import java.util.Set;
20+
import java.util.Map;
1921
import java.util.function.Consumer;
2022
import java.util.function.Function;
21-
import java.util.stream.Collectors;
2223
import java.util.stream.Stream;
2324

2425
import org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor;
@@ -60,9 +61,8 @@ protected void doWithMatchingDescriptor(Class<PARENT> parentTestDescriptorType,
6061
protected void orderChildrenTestDescriptors(TestDescriptor parentTestDescriptor, Class<CHILD> matchingChildrenType,
6162
Function<CHILD, WRAPPER> descriptorWrapperFactory, DescriptorWrapperOrderer descriptorWrapperOrderer) {
6263

63-
Set<? extends TestDescriptor> children = parentTestDescriptor.getChildren();
64-
65-
List<WRAPPER> matchingDescriptorWrappers = children.stream()//
64+
List<WRAPPER> matchingDescriptorWrappers = parentTestDescriptor.getChildren()//
65+
.stream()//
6666
.filter(matchingChildrenType::isInstance)//
6767
.map(matchingChildrenType::cast)//
6868
.map(descriptorWrapperFactory)//
@@ -74,50 +74,33 @@ protected void orderChildrenTestDescriptors(TestDescriptor parentTestDescriptor,
7474
}
7575

7676
if (descriptorWrapperOrderer.canOrderWrappers()) {
77-
List<TestDescriptor> nonMatchingTestDescriptors = children.stream()//
78-
.filter(childTestDescriptor -> !matchingChildrenType.isInstance(childTestDescriptor))//
79-
.collect(Collectors.toList());
80-
81-
// Make a local copy for later validation
82-
Set<WRAPPER> originalWrappers = new LinkedHashSet<>(matchingDescriptorWrappers);
83-
84-
descriptorWrapperOrderer.orderWrappers(matchingDescriptorWrappers);
85-
86-
int difference = matchingDescriptorWrappers.size() - originalWrappers.size();
87-
if (difference > 0) {
88-
descriptorWrapperOrderer.logDescriptorsAddedWarning(difference);
89-
}
90-
else if (difference < 0) {
91-
descriptorWrapperOrderer.logDescriptorsRemovedWarning(difference);
92-
}
93-
94-
Set<TestDescriptor> orderedTestDescriptors = matchingDescriptorWrappers.stream()//
95-
.filter(originalWrappers::contains)//
96-
.map(AbstractAnnotatedDescriptorWrapper::getTestDescriptor)//
97-
.collect(toCollection(LinkedHashSet::new));
98-
99-
// There is currently no way to removeAll or addAll children at once, so we
100-
// first remove them all and then add them all back.
101-
Stream.concat(orderedTestDescriptors.stream(), nonMatchingTestDescriptors.stream())//
102-
.forEach(parentTestDescriptor::removeChild);
103-
104-
// If we are ordering children of type ClassBasedTestDescriptor, that means we
105-
// are ordering top-level classes or @Nested test classes. Thus, the
106-
// nonMatchingTestDescriptors list is either empty (for top-level classes) or
107-
// contains only local test methods (for @Nested classes) which must be executed
108-
// before tests in @Nested test classes. So we add the test methods before adding
109-
// the @Nested test classes.
110-
if (matchingChildrenType == ClassBasedTestDescriptor.class) {
111-
Stream.concat(nonMatchingTestDescriptors.stream(), orderedTestDescriptors.stream())//
112-
.forEach(parentTestDescriptor::addChild);
113-
}
114-
// Otherwise, we add the ordered descriptors before the non-matching descriptors,
115-
// which is the case when we are ordering test methods. In other words, local
116-
// test methods always get added before @Nested test classes.
117-
else {
118-
Stream.concat(orderedTestDescriptors.stream(), nonMatchingTestDescriptors.stream())//
119-
.forEach(parentTestDescriptor::addChild);
120-
}
77+
parentTestDescriptor.orderChildren(children -> {
78+
Stream<TestDescriptor> nonMatchingTestDescriptors = children.stream()//
79+
.filter(childTestDescriptor -> !matchingChildrenType.isInstance(childTestDescriptor));
80+
81+
descriptorWrapperOrderer.orderWrappers(matchingDescriptorWrappers);
82+
83+
Stream<TestDescriptor> orderedTestDescriptors = matchingDescriptorWrappers.stream()//
84+
.map(AbstractAnnotatedDescriptorWrapper::getTestDescriptor);
85+
86+
// If we are ordering children of type ClassBasedTestDescriptor, that means we
87+
// are ordering top-level classes or @Nested test classes. Thus, the
88+
// nonMatchingTestDescriptors list is either empty (for top-level classes) or
89+
// contains only local test methods (for @Nested classes) which must be executed
90+
// before tests in @Nested test classes. So we add the test methods before adding
91+
// the @Nested test classes.
92+
if (matchingChildrenType == ClassBasedTestDescriptor.class) {
93+
return Stream.concat(nonMatchingTestDescriptors, orderedTestDescriptors)//
94+
.collect(toList());
95+
}
96+
// Otherwise, we add the ordered descriptors before the non-matching descriptors,
97+
// which is the case when we are ordering test methods. In other words, local
98+
// test methods always get added before @Nested test classes.
99+
else {
100+
return Stream.concat(orderedTestDescriptors, nonMatchingTestDescriptors)//
101+
.collect(toList());
102+
}
103+
});
121104
}
122105

123106
// Recurse through the children in order to support ordering for @Nested test classes.
@@ -167,7 +150,32 @@ private boolean canOrderWrappers() {
167150
}
168151

169152
private void orderWrappers(List<WRAPPER> wrappers) {
170-
this.orderingAction.accept(wrappers);
153+
List<WRAPPER> orderedWrappers = new ArrayList<>(wrappers);
154+
this.orderingAction.accept(orderedWrappers);
155+
Map<Object, Integer> distinctWrappersToIndex = distinctWrappersToIndex(orderedWrappers);
156+
157+
int difference = orderedWrappers.size() - wrappers.size();
158+
int distinctDifference = distinctWrappersToIndex.size() - wrappers.size();
159+
if (difference > 0) { // difference >= distinctDifference
160+
logDescriptorsAddedWarning(difference);
161+
}
162+
if (distinctDifference < 0) { // distinctDifference <= difference
163+
logDescriptorsRemovedWarning(distinctDifference);
164+
}
165+
166+
wrappers.sort(comparing(wrapper -> distinctWrappersToIndex.getOrDefault(wrapper, -1)));
167+
}
168+
169+
private Map<Object, Integer> distinctWrappersToIndex(List<?> wrappers) {
170+
Map<Object, Integer> toIndex = new HashMap<>();
171+
for (int i = 0; i < wrappers.size(); i++) {
172+
// Avoid ClassCastException if a misbehaving ordering action added a non-WRAPPER
173+
Object wrapper = wrappers.get(i);
174+
if (!toIndex.containsKey(wrapper)) {
175+
toIndex.put(wrapper, i);
176+
}
177+
}
178+
return toIndex;
171179
}
172180

173181
private void logDescriptorsAddedWarning(int number) {

‎junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,16 @@
1010

1111
package org.junit.platform.engine;
1212

13+
import static org.apiguardian.api.API.Status.EXPERIMENTAL;
1314
import static org.apiguardian.api.API.Status.STABLE;
1415

16+
import java.util.ArrayList;
1517
import java.util.Collections;
1618
import java.util.LinkedHashSet;
19+
import java.util.List;
1720
import java.util.Optional;
1821
import java.util.Set;
22+
import java.util.function.UnaryOperator;
1923

2024
import org.apiguardian.api.API;
2125
import org.junit.platform.commons.util.Preconditions;
@@ -172,6 +176,39 @@ default Set<? extends TestDescriptor> getDescendants() {
172176
*/
173177
void removeFromHierarchy();
174178

179+
/**
180+
* Order the children from this descriptor.
181+
*
182+
* <p>The {@code orderer} is provided a modifiable list of child test
183+
* descriptors in this test descriptor; never {@code null}. The
184+
* {@code orderer} must return a list containing the same descriptors in any
185+
* order; potentially the same list, but never {@code null}. If descriptors
186+
* were added or removed, an exception is thrown.
187+
*
188+
* @param orderer a unary operator to order the children of this test
189+
* descriptor.
190+
*/
191+
@API(since = "5.12", status = EXPERIMENTAL)
192+
default void orderChildren(UnaryOperator<List<TestDescriptor>> orderer) {
193+
Preconditions.notNull(orderer, "orderer must not be null");
194+
Set<? extends TestDescriptor> originalChildren = getChildren();
195+
List<TestDescriptor> suggestedOrder = orderer.apply(new ArrayList<>(originalChildren));
196+
Preconditions.notNull(suggestedOrder, "orderer may not return null");
197+
198+
Set<? extends TestDescriptor> orderedChildren = new LinkedHashSet<>(suggestedOrder);
199+
boolean unmodified = originalChildren.equals(orderedChildren);
200+
Preconditions.condition(unmodified && originalChildren.size() == suggestedOrder.size(),
201+
"orderer may not add or remove test descriptors");
202+
203+
suggestedOrder.stream() //
204+
.distinct() //
205+
.filter(originalChildren::contains)//
206+
.forEach(testDescriptor -> {
207+
removeChild(testDescriptor);
208+
addChild(testDescriptor);
209+
});
210+
}
211+
175212
/**
176213
* Determine if this descriptor is a <em>root</em> descriptor.
177214
*

‎junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/AbstractTestDescriptor.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,13 @@
1313
import static java.util.Collections.emptySet;
1414
import static org.apiguardian.api.API.Status.STABLE;
1515

16+
import java.util.ArrayList;
1617
import java.util.Collections;
1718
import java.util.LinkedHashSet;
19+
import java.util.List;
1820
import java.util.Optional;
1921
import java.util.Set;
22+
import java.util.function.UnaryOperator;
2023

2124
import org.apiguardian.api.API;
2225
import org.junit.platform.commons.util.Preconditions;
@@ -147,6 +150,24 @@ public void removeFromHierarchy() {
147150
this.children.clear();
148151
}
149152

153+
/**
154+
* {@inheritDoc}
155+
*/
156+
@Override
157+
public void orderChildren(UnaryOperator<List<TestDescriptor>> orderer) {
158+
Preconditions.notNull(orderer, "orderer must not be null");
159+
List<TestDescriptor> suggestedOrder = orderer.apply(new ArrayList<>(this.children));
160+
Preconditions.notNull(suggestedOrder, "orderer may not return null");
161+
162+
Set<? extends TestDescriptor> orderedChildren = new LinkedHashSet<>(suggestedOrder);
163+
boolean unmodified = this.children.equals(orderedChildren);
164+
Preconditions.condition(unmodified && this.children.size() == suggestedOrder.size(),
165+
"orderer may not add or remove test descriptors");
166+
167+
this.children.clear();
168+
this.children.addAll(orderedChildren);
169+
}
170+
150171
@Override
151172
public Optional<? extends TestDescriptor> findByUniqueId(UniqueId uniqueId) {
152173
Preconditions.notNull(uniqueId, "UniqueId must not be null");

0 commit comments

Comments
 (0)