From 011263184dd6d1659bfe6e2f3043c83b4e0a68bd Mon Sep 17 00:00:00 2001 From: "M.P. Korstanje" Date: Mon, 3 Feb 2025 23:14:37 +0100 Subject: [PATCH] Order wrappers without transferring between collections --- .../discovery/AbstractOrderingVisitor.java | 34 ++++---- .../engine/extension/OrderedMethodTests.java | 86 +++++++++++++++++++ 2 files changed, 103 insertions(+), 17 deletions(-) diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/AbstractOrderingVisitor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/AbstractOrderingVisitor.java index 012bc2e82f18..6863baaaa9b8 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/AbstractOrderingVisitor.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/AbstractOrderingVisitor.java @@ -10,13 +10,13 @@ package org.junit.jupiter.engine.discovery; -import static java.util.stream.Collectors.toCollection; +import static java.util.Comparator.comparing; import static java.util.stream.Collectors.toList; import java.util.ArrayList; -import java.util.LinkedHashSet; +import java.util.HashMap; import java.util.List; -import java.util.Set; +import java.util.Map; import java.util.function.Consumer; import java.util.function.Function; import java.util.stream.Stream; @@ -60,12 +60,12 @@ protected void doWithMatchingDescriptor(Class parentTestDescriptorType, protected void orderChildrenTestDescriptors(TestDescriptor parentTestDescriptor, Class matchingChildrenType, Function descriptorWrapperFactory, DescriptorWrapperOrderer descriptorWrapperOrderer) { - Set matchingDescriptorWrappers = parentTestDescriptor.getChildren()// + List matchingDescriptorWrappers = parentTestDescriptor.getChildren()// .stream()// .filter(matchingChildrenType::isInstance)// .map(matchingChildrenType::cast)// .map(descriptorWrapperFactory)// - .collect(toCollection(LinkedHashSet::new)); + .collect(toList()); // If there are no children to order, abort early. if (matchingDescriptorWrappers.isEmpty()) { @@ -149,13 +149,13 @@ private boolean canOrderWrappers() { return this.orderingAction != null; } - private void orderWrappers(Set wrappers) { + private void orderWrappers(List wrappers) { List orderedWrappers = new ArrayList<>(wrappers); this.orderingAction.accept(orderedWrappers); - Set distinctOrderedWrappers = new LinkedHashSet<>(orderedWrappers); + Map distinctWrappersToIndex = distinctWrappersToIndex(orderedWrappers); int difference = orderedWrappers.size() - wrappers.size(); - int distinctDifference = distinctOrderedWrappers.size() - wrappers.size(); + int distinctDifference = distinctWrappersToIndex.size() - wrappers.size(); if (difference > 0) { // difference >= distinctDifference logDescriptorsAddedWarning(difference); } @@ -163,19 +163,19 @@ private void orderWrappers(Set wrappers) { logDescriptorsRemovedWarning(distinctDifference); } - orderWrappersWithOrderFrom(wrappers, distinctOrderedWrappers); + wrappers.sort(comparing(wrapper -> distinctWrappersToIndex.getOrDefault(wrapper, -1))); } - @SuppressWarnings("unchecked") - private void orderWrappersWithOrderFrom(Set wrappers, Set orderedWrappers) { - // Avoid ClassCastException if a misbehaving ordering action added a non-WRAPPER - for (Object wrapper : orderedWrappers) { - //noinspection SuspiciousMethodCalls - if (wrappers.remove(wrapper)) { - // guaranteed by wrappers.contains - wrappers.add((WRAPPER) wrapper); + private Map distinctWrappersToIndex(List wrappers) { + Map toIndex = new HashMap<>(); + for (int i = 0; i < wrappers.size(); i++) { + // Avoid ClassCastException if a misbehaving ordering action added a non-WRAPPER + Object wrapper = wrappers.get(i); + if (!toIndex.containsKey(wrapper)) { + toIndex.put(wrapper, i); } } + return toIndex; } private void logDescriptorsAddedWarning(int number) { diff --git a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/OrderedMethodTests.java b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/OrderedMethodTests.java index f1eb99d7d838..12818d6686bc 100644 --- a/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/OrderedMethodTests.java +++ b/jupiter-tests/src/test/java/org/junit/jupiter/engine/extension/OrderedMethodTests.java @@ -20,10 +20,14 @@ import static org.junit.jupiter.engine.Constants.PARALLEL_EXECUTION_ENABLED_PROPERTY_NAME; import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass; +import java.lang.annotation.Annotation; +import java.lang.reflect.Method; import java.util.Collections; import java.util.HashSet; import java.util.LinkedHashSet; +import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.logging.Level; import java.util.logging.LogRecord; @@ -279,6 +283,15 @@ void misbehavingMethodOrdererThatAddsElements(@TrackLogRecords LogRecordListener assertExpectedLogMessage(listener, expectedMessage); } + @Test + void misbehavingMethodOrdererThatImpersonatesElements() { + Class testClass = MisbehavingByImpersonatingTestCase.class; + + executeTestsInParallel(testClass).assertStatistics(stats -> stats.succeeded(2)); + + assertThat(callSequence).containsExactlyInAnyOrder("test1()", "test2()"); + } + @Test void misbehavingMethodOrdererThatRemovesElements(@TrackLogRecords LogRecordListener listener) { Class testClass = MisbehavingByRemovingTestCase.class; @@ -655,6 +668,24 @@ void test1() { } } + @SuppressWarnings("JUnitMalformedDeclaration") + @TestMethodOrder(MisbehavingByImpersonating.class) + static class MisbehavingByImpersonatingTestCase { + + @BeforeEach + void trackInvocations(TestInfo testInfo) { + callSequence.add(testInfo.getDisplayName()); + } + + @Test + void test2() { + } + + @Test + void test1() { + } + } + @SuppressWarnings("JUnitMalformedDeclaration") @TestMethodOrder(MisbehavingByRemoving.class) static class MisbehavingByRemovingTestCase { @@ -721,6 +752,61 @@ static T mockMethodDescriptor() { } + static class MisbehavingByImpersonating implements MethodOrderer { + + @Override + public void orderMethods(MethodOrdererContext context) { + context.getMethodDescriptors().sort(comparing(MethodDescriptor::getDisplayName)); + MethodDescriptor method1 = context.getMethodDescriptors().get(0); + MethodDescriptor method2 = context.getMethodDescriptors().get(1); + + context.getMethodDescriptors().set(0, createMethodDescriptorImpersonator(method1)); + context.getMethodDescriptors().set(1, createMethodDescriptorImpersonator(method2)); + } + + @SuppressWarnings("unchecked") + static T createMethodDescriptorImpersonator(MethodDescriptor method) { + MethodDescriptor stub = new MethodDescriptor() { + @Override + public Method getMethod() { + return null; + } + + @Override + public String getDisplayName() { + return null; + } + + @Override + public boolean isAnnotated(Class annotationType) { + return false; + } + + @Override + public Optional findAnnotation(Class annotationType) { + return null; + } + + @Override + public List findRepeatableAnnotations(Class annotationType) { + return null; + } + + @SuppressWarnings("EqualsWhichDoesntCheckParameterClass") + @Override + public boolean equals(Object obj) { + return method.equals(obj); + } + + @Override + public int hashCode() { + return method.hashCode(); + } + }; + return (T) stub; + } + } + static class MisbehavingByRemoving implements MethodOrderer { @Override