From 9b8c821d9c8feaa0839ed88b05017eef2595b506 Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Fri, 31 Jan 2025 13:46:31 +0100 Subject: [PATCH] Relax constraints on orderer --- .../junit/platform/engine/TestDescriptor.java | 17 +++-- .../descriptor/AbstractTestDescriptor.java | 11 ++-- .../TestDescriptorOrderChildrenTest.java | 63 +++++++++++-------- 3 files changed, 51 insertions(+), 40 deletions(-) diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java index 855102721ab2..05cd5685bcf1 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/TestDescriptor.java @@ -10,6 +10,7 @@ package org.junit.platform.engine; +import static java.util.Comparator.comparingInt; import static org.apiguardian.api.API.Status.EXPERIMENTAL; import static org.apiguardian.api.API.Status.STABLE; @@ -183,21 +184,19 @@ default Set getDescendants() { * this test descriptor. Never {@code null}. The {@code orderer} may return * the elements of list in any order but may not add or remove descriptors. *

- * If descriptors were added or removed this method must throw a - * {@link org.junit.platform.commons.JUnitException JUnitException} explaining - * that an {@code orderer} may not remove children from this descriptor. + * If descriptors were added, these descriptors will be ignored. If descriptors + * are removed, the original descriptors will be retained in arbitrary order. * * @param orderer a unary operator to order the children from this test descriptor. */ @API(since = "5.12", status = EXPERIMENTAL) default void orderChildren(UnaryOperator> orderer) { Preconditions.notNull(orderer, "orderer must not be null"); - Set children = getChildren(); - Set orderedChildren = new LinkedHashSet<>(orderer.apply(new ArrayList<>(children))); - boolean unmodified = children.equals(orderedChildren); - Preconditions.condition(unmodified, "orderer may not add or remove test descriptors"); - orderedChildren.forEach(this::removeChild); - orderedChildren.forEach(this::addChild); + List copyOfChildren = new ArrayList<>(getChildren()); + List suggestedOrder = orderer.apply(new ArrayList<>(copyOfChildren)); + copyOfChildren.sort(comparingInt(suggestedOrder::indexOf)); + copyOfChildren.forEach(this::removeChild); + copyOfChildren.forEach(this::addChild); } /** diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/AbstractTestDescriptor.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/AbstractTestDescriptor.java index 753d2a4ab582..0fa40ce80033 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/AbstractTestDescriptor.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/descriptor/AbstractTestDescriptor.java @@ -11,6 +11,7 @@ package org.junit.platform.engine.support.descriptor; import static java.util.Collections.emptySet; +import static java.util.Comparator.comparingInt; import static org.apiguardian.api.API.Status.STABLE; import java.util.ArrayList; @@ -153,11 +154,11 @@ public void removeFromHierarchy() { @Override public void orderChildren(UnaryOperator> orderer) { Preconditions.notNull(orderer, "orderer must not be null"); - Set orderedChildren = new LinkedHashSet<>(orderer.apply(new ArrayList<>(children))); - boolean unmodified = children.equals(orderedChildren); - Preconditions.condition(unmodified, "orderer may not add or remove test descriptors"); - children.clear(); - children.addAll(orderedChildren); + List copyOfChildren = new ArrayList<>(this.children); + List suggestedOrder = orderer.apply(new ArrayList<>(copyOfChildren)); + copyOfChildren.sort(comparingInt(suggestedOrder::indexOf)); + this.children.clear(); + this.children.addAll(copyOfChildren); } @Override diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/TestDescriptorOrderChildrenTest.java b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/TestDescriptorOrderChildrenTest.java index c91da524d028..f7c951f473a0 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/TestDescriptorOrderChildrenTest.java +++ b/platform-tests/src/test/java/org/junit/platform/engine/support/descriptor/TestDescriptorOrderChildrenTest.java @@ -12,13 +12,11 @@ import static java.util.Comparator.comparing; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.Test; -import org.junit.platform.commons.JUnitException; import org.junit.platform.engine.TestDescriptor; import org.junit.platform.engine.UniqueId; @@ -64,49 +62,62 @@ default void orderChildrenInSameOrder() { @Test default void orderChildrenRemovesDescriptor() { var testDescriptor = createTestDescriptorWithChildren(); - var exception = assertThrows(JUnitException.class, () -> { - testDescriptor.orderChildren(children -> { - children.removeFirst(); - return children; - }); + var childrenInOriginalOrder = new ArrayList<>(testDescriptor.getChildren()); + testDescriptor.orderChildren(children -> { + children.remove(1); + return children; }); - assertThat(exception).hasMessage("orderer may not add or remove test descriptors"); + List children = new ArrayList<>(testDescriptor.getChildren()); + assertThat(children).isEqualTo(List.of(// + childrenInOriginalOrder.get(0), // + childrenInOriginalOrder.get(2), // + childrenInOriginalOrder.get(1))// + ); } @Test default void orderChildrenAddsDescriptor() { var testDescriptor = createTestDescriptorWithChildren(); - var exception = assertThrows(JUnitException.class, () -> { - testDescriptor.orderChildren(children -> { - children.add(new StubTestDescriptor(UniqueId.root("extra", "extra1"))); - return children; - }); + var childrenInOriginalOrder = new ArrayList<>(testDescriptor.getChildren()); + testDescriptor.orderChildren(children -> { + children.add(1, new StubTestDescriptor(UniqueId.root("extra", "extra1"))); + return children; }); - assertThat(exception).hasMessage("orderer may not add or remove test descriptors"); + List children = new ArrayList<>(testDescriptor.getChildren()); + assertThat(children).isEqualTo(childrenInOriginalOrder); } @Test default void orderChildrenReplacesDescriptor() { var testDescriptor = createTestDescriptorWithChildren(); - var exception = assertThrows(JUnitException.class, () -> { - testDescriptor.orderChildren(children -> { - children.set(0, new StubTestDescriptor(UniqueId.root("replaced", "replaced1"))); - return children; - }); + var childrenInOriginalOrder = new ArrayList<>(testDescriptor.getChildren()); + + testDescriptor.orderChildren(children -> { + children.set(1, new StubTestDescriptor(UniqueId.root("replaced", "replaced1"))); + return children; }); - assertThat(exception).hasMessage("orderer may not add or remove test descriptors"); + List children = new ArrayList<>(testDescriptor.getChildren()); + assertThat(children).isEqualTo(List.of(// + childrenInOriginalOrder.get(0), // + childrenInOriginalOrder.get(2), // + childrenInOriginalOrder.get(1))// + ); } @Test default void orderChildrenDuplicatesDescriptor() { var testDescriptor = createTestDescriptorWithChildren(); - var exception = assertThrows(JUnitException.class, () -> { - testDescriptor.orderChildren(children -> { - children.set(1, children.getFirst()); - return children; - }); + var childrenInOriginalOrder = new ArrayList<>(testDescriptor.getChildren()); + testDescriptor.orderChildren(children -> { + children.add(1, children.getLast()); + return children; }); - assertThat(exception).hasMessage("orderer may not add or remove test descriptors"); + List children = new ArrayList<>(testDescriptor.getChildren()); + assertThat(children).isEqualTo(List.of(// + childrenInOriginalOrder.get(0), // + childrenInOriginalOrder.get(2), // + childrenInOriginalOrder.get(1))// + ); } }