Ensure @Nested classes are executed after sibling test methods

documentation/src/docs/asciidoc/release-notes/release-notes-5.13.1.adoc

@@ -35,7 +35,13 @@ on GitHub.
 [[release-notes-5.13.1-junit-jupiter-bug-fixes]]
 ==== Bug Fixes
 
-* ❓
+* The 5.13.0 release introduced a regression regarding the execution order in test classes
+  containing both test methods and `@Nested` test classes. When classpath scanning was
+  used during test discovery -- for example, when resolving a package selector for a
+  `@Suite` class -- test methods in `@Nested` classes were executed _before_ test methods
+  in their enclosing class. This undesired change in behavior has now been reverted so
+  that tests in `@Nested` test classes are always executed _after_ tests in enclosing test
+  classes again.
 
 [[release-notes-5.13.1-junit-jupiter-deprecations-and-breaking-changes]]
 ==== Deprecations and Breaking Changes

junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/discovery/MethodOrderingVisitor.java

@@ -10,12 +10,14 @@
 
 package org.junit.jupiter.engine.discovery;
 
+import static java.util.Comparator.comparing;
 import static org.junit.platform.commons.support.AnnotationSupport.findAnnotation;
 import static org.junit.platform.commons.support.AnnotationSupport.isAnnotated;
 
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Consumer;
+import java.util.function.UnaryOperator;
 
 import org.junit.jupiter.api.MethodOrderer;
 import org.junit.jupiter.api.Order;
@@ -40,6 +42,9 @@ class MethodOrderingVisitor extends AbstractOrderingVisitor {
 	private final JupiterConfiguration configuration;
 	private final Condition<MethodBasedTestDescriptor> noOrderAnnotation;
 
+	// Not a static field to avoid initialization at build time for GraalVM
+	private final UnaryOperator<List<TestDescriptor>> methodsBeforeNestedClassesOrderer;
+
 	MethodOrderingVisitor(JupiterConfiguration configuration, DiscoveryIssueReporter issueReporter) {
 		super(issueReporter);
 		this.configuration = configuration;
@@ -51,6 +56,7 @@ class MethodOrderingVisitor extends AbstractOrderingVisitor {
 						.source(MethodSource.from(testDescriptor.getTestMethod())) //
 						.build();
 			});
+		this.methodsBeforeNestedClassesOrderer = createMethodsBeforeNestedClassesOrderer();
 	}
 
 	@Override
@@ -80,6 +86,7 @@ private void orderContainedMethods(ClassBasedTestDescriptor classBasedTestDescri
 
 	private void orderContainedMethods(ClassBasedTestDescriptor classBasedTestDescriptor, Class<?> testClass,
 			Optional<MethodOrderer> methodOrderer) {
+
 		DescriptorWrapperOrderer<?, DefaultMethodDescriptor> descriptorWrapperOrderer = createDescriptorWrapperOrderer(
 			testClass, methodOrderer);
 
@@ -89,6 +96,11 @@ private void orderContainedMethods(ClassBasedTestDescriptor classBasedTestDescri
 			DefaultMethodDescriptor::new, //
 			descriptorWrapperOrderer);
 
+		if (methodOrderer.isEmpty()) {
+			// If there is an orderer, this is ensured by the call above
+			classBasedTestDescriptor.orderChildren(methodsBeforeNestedClassesOrderer);
+		}
+
 		// Note: MethodOrderer#getDefaultExecutionMode() is guaranteed
 		// to be invoked after MethodOrderer#orderMethods().
 		methodOrderer //
@@ -126,4 +138,12 @@ private Optional<Consumer<MethodBasedTestDescriptor>> toValidationAction(Optiona
 		return Optional.of(noOrderAnnotation::check);
 	}
 
+	private static UnaryOperator<List<TestDescriptor>> createMethodsBeforeNestedClassesOrderer() {
+		var methodsFirst = comparing(MethodBasedTestDescriptor.class::isInstance).reversed();
+		return children -> {
+			children.sort(methodsFirst);
+			return children;
+		};
+	}
+
 }

jupiter-tests/src/test/java/org/junit/jupiter/engine/NestedTestClassesTests.java

@@ -13,28 +13,36 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertAll;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.platform.engine.discovery.ClassNameFilter.includeClassNamePatterns;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectMethod;
+import static org.junit.platform.engine.discovery.DiscoverySelectors.selectPackage;
 import static org.junit.platform.engine.discovery.DiscoverySelectors.selectUniqueId;
 import static org.junit.platform.launcher.LauncherConstants.CRITICAL_DISCOVERY_ISSUE_SEVERITY_PROPERTY_NAME;
 import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;
 import static org.junit.platform.testkit.engine.EventConditions.finishedWithFailure;
 import static org.junit.platform.testkit.engine.TestExecutionResultConditions.message;
 
 import java.util.List;
+import java.util.function.Consumer;
+import java.util.regex.Pattern;
 
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Named;
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.engine.NestedTestClassesTests.OuterClass.NestedClass;
 import org.junit.jupiter.engine.NestedTestClassesTests.OuterClass.NestedClass.RecursiveNestedClass;
 import org.junit.jupiter.engine.NestedTestClassesTests.OuterClass.NestedClass.RecursiveNestedSiblingClass;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 import org.junit.platform.engine.DiscoveryIssue.Severity;
 import org.junit.platform.engine.TestDescriptor;
 import org.junit.platform.engine.support.descriptor.ClassSource;
 import org.junit.platform.launcher.LauncherDiscoveryRequest;
+import org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder;
 import org.junit.platform.testkit.engine.EngineExecutionResults;
 import org.junit.platform.testkit.engine.Events;
 
@@ -53,20 +61,36 @@ void nestedTestsAreCorrectlyDiscovered() {
 		assertEquals(5, engineDescriptor.getDescendants().size(), "# resolved test descriptors");
 	}
 
-	@Test
-	void nestedTestsAreExecuted() {
-		EngineExecutionResults executionResults = executeTestsForClass(TestCaseWithNesting.class);
-		Events containers = executionResults.containerEvents();
-		Events tests = executionResults.testEvents();
+	@ParameterizedTest(name = "{0}")
+	@MethodSource
+	void nestedTestsAreExecutedInTheRightOrder(Consumer<LauncherDiscoveryRequestBuilder> configurer) {
+		EngineExecutionResults executionResults = executeTests(configurer);
 
+		Events tests = executionResults.testEvents();
 		assertEquals(3, tests.started().count(), "# tests started");
 		assertEquals(2, tests.succeeded().count(), "# tests succeeded");
 		assertEquals(1, tests.failed().count(), "# tests failed");
 
+		assertThat(tests.started().map(it -> it.getTestDescriptor().getDisplayName())) //
+				.containsExactlyInAnyOrder("someTest()", "successful()", "failing()") //
+				.containsSubsequence("someTest()", "successful()") //
+				.containsSubsequence("someTest()", "failing()");
+
+		Events containers = executionResults.containerEvents();
 		assertEquals(3, containers.started().count(), "# containers started");
 		assertEquals(3, containers.finished().count(), "# containers finished");
 	}
 
+	static List<Named<Consumer<LauncherDiscoveryRequestBuilder>>> nestedTestsAreExecutedInTheRightOrder() {
+		return List.of( //
+			Named.of("class selector", request -> request //
+					.selectors(selectClass(TestCaseWithNesting.class))),
+			Named.of("package selector", request -> request //
+					.selectors(selectPackage(TestCaseWithNesting.class.getPackageName())) //
+					.filters(includeClassNamePatterns(Pattern.quote(TestCaseWithNesting.class.getName()) + ".*"))) //
+		);
+	}
+
 	@Test
 	void doublyNestedTestsAreCorrectlyDiscovered() {
 		LauncherDiscoveryRequest request = request().selectors(selectClass(TestCaseWithDoubleNesting.class)).build();