Fix logic for validating @RecordApplicationEvents config

Prior to this commit, the SpringExtension looked up the
TestInstance.Lifecycle and ExecutionMode using
TestContextAnnotationUtils; however, using TestContextAnnotationUtils is
problematic since the TestInstance.Lifecycle and ExecutionMode can be
configured globally via configuration parameters instead of locally via
the @testinstance and @execution annotations.

This commit addresses these issues by looking up the
TestInstance.Lifecycle and ExecutionMode via JUnit Jupiter's
ExtensionContext which takes into account both global and local
configuration.

See gh-30020

Author: sbrannen

spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java

@@ -29,7 +29,7 @@
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.TestInstance.Lifecycle;
 import org.junit.jupiter.api.extension.AfterAllCallback;
 import org.junit.jupiter.api.extension.AfterEachCallback;
 import org.junit.jupiter.api.extension.AfterTestExecutionCallback;
@@ -42,7 +42,6 @@
 import org.junit.jupiter.api.extension.ParameterContext;
 import org.junit.jupiter.api.extension.ParameterResolver;
 import org.junit.jupiter.api.extension.TestInstancePostProcessor;
-import org.junit.jupiter.api.parallel.Execution;
 import org.junit.jupiter.api.parallel.ExecutionMode;
 import org.junit.platform.commons.annotation.Testable;
 
@@ -98,7 +97,7 @@ public class SpringExtension implements BeforeAllCallback, AfterAllCallback, Tes
 	private static final Namespace AUTOWIRED_VALIDATION_NAMESPACE =
 			Namespace.create(SpringExtension.class.getName() + "#autowired.validation");
 
-	private static final String NO_AUTOWIRED_VIOLATIONS_DETECTED = "NO AUTOWIRED VIOLATIONS DETECTED";
+	private static final String NO_VIOLATIONS_DETECTED = "NO VIOLATIONS DETECTED";
 
 	/**
 	 * {@link Namespace} in which {@code @RecordApplicationEvents} validation error messages
@@ -153,8 +152,8 @@ public void postProcessTestInstance(Object testInstance, ExtensionContext contex
 	}
 
 	/**
-	 * Validate that test class or its enclosing class doesn't attempt to record
-	 * application events in a parallel mode that makes it un-deterministic
+	 * Validate that the test class or its enclosing class doesn't attempt to record
+	 * application events in a parallel mode that makes it non-deterministic
 	 * ({@code @TestInstance(PER_CLASS)} and {@code @Execution(CONCURRENT)}
 	 * combination).
 	 * @since 6.1.0
@@ -165,30 +164,28 @@ private void validateRecordApplicationEventsConfig(ExtensionContext context) {
 		Store store = context.getStore(RECORD_APPLICATION_EVENTS_VALIDATION_NAMESPACE);
 
 		String errorMessage = store.getOrComputeIfAbsent(context.getRequiredTestClass(), testClass -> {
-			boolean record = TestContextAnnotationUtils.hasAnnotation(testClass, RecordApplicationEvents.class);
-			if (!record) {
-				return NO_AUTOWIRED_VIOLATIONS_DETECTED;
+			boolean recording = TestContextAnnotationUtils.hasAnnotation(testClass, RecordApplicationEvents.class);
+			if (!recording) {
+				return NO_VIOLATIONS_DETECTED;
 			}
-			final TestInstance testInstance = TestContextAnnotationUtils.findMergedAnnotation(testClass, TestInstance.class);
 
-			if (testInstance == null || testInstance.value() != TestInstance.Lifecycle.PER_CLASS) {
-				return NO_AUTOWIRED_VIOLATIONS_DETECTED;
+			if (context.getTestInstanceLifecycle().orElse(Lifecycle.PER_METHOD) == Lifecycle.PER_METHOD) {
+				return NO_VIOLATIONS_DETECTED;
 			}
 
-			final Execution execution = TestContextAnnotationUtils.findMergedAnnotation(testClass, Execution.class);
-
-			if (execution == null || execution.value() != ExecutionMode.CONCURRENT) {
-				return NO_AUTOWIRED_VIOLATIONS_DETECTED;
+			if (context.getExecutionMode() == ExecutionMode.SAME_THREAD) {
+				return NO_VIOLATIONS_DETECTED;
 			}
 
-			return "Test classes or inner classes that @RecordApplicationEvents must not be run in parallel "
-					+ "with the @TestInstance(Lifecycle.PER_CLASS) configuration. Use either @Execution(SAME_THREAD), "
-					+ "@TestInstance(PER_METHOD) or disable parallel execution altogether. Note that when recording "
-					+ "events in parallel, one might see events published by other tests as the application context "
-					+ "can be common.";
+			return """
+					Test classes or @Nested test classes that @RecordApplicationEvents must not be run \
+					in parallel with the @TestInstance(PER_CLASS) lifecycle mode. Configure either \
+					@Execution(SAME_THREAD) or @TestInstance(PER_METHOD) semantics, or disable parallel \
+					execution altogether. Note that when recording events in parallel, one might see events \
+					published by other tests since the application context may be shared.""";
 		}, String.class);
 
-		if (errorMessage != NO_AUTOWIRED_VIOLATIONS_DETECTED) {
+		if (errorMessage != NO_VIOLATIONS_DETECTED) {
 			throw new IllegalStateException(errorMessage);
 		}
 	}
@@ -206,15 +203,15 @@ private void validateAutowiredConfig(ExtensionContext context) {
 		String errorMessage = store.getOrComputeIfAbsent(context.getRequiredTestClass(), testClass -> {
 				Method[] methodsWithErrors =
 						ReflectionUtils.getUniqueDeclaredMethods(testClass, autowiredTestOrLifecycleMethodFilter);
-				return (methodsWithErrors.length == 0 ? NO_AUTOWIRED_VIOLATIONS_DETECTED :
+				return (methodsWithErrors.length == 0 ? NO_VIOLATIONS_DETECTED :
 						String.format(
 								"Test methods and test lifecycle methods must not be annotated with @Autowired. " +
 								"You should instead annotate individual method parameters with @Autowired, " +
 								"@Qualifier, or @Value. Offending methods in test class %s: %s",
 								testClass.getName(), Arrays.toString(methodsWithErrors)));
 			}, String.class);
 
-		if (errorMessage != NO_AUTOWIRED_VIOLATIONS_DETECTED) {
+		if (errorMessage != NO_VIOLATIONS_DETECTED) {
 			throw new IllegalStateException(errorMessage);
 		}
 	}

spring-test/src/test/java/org/springframework/test/context/junit/jupiter/event/ParallelApplicationEventsIntegrationTests.java

@@ -32,8 +32,6 @@
 import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.api.TestInstance;
 import org.junit.jupiter.api.TestInstance.Lifecycle;
-import org.junit.jupiter.api.parallel.Execution;
-import org.junit.jupiter.api.parallel.ExecutionMode;
 import org.junit.platform.engine.TestExecutionResult;
 import org.junit.platform.testkit.engine.EngineExecutionResults;
 import org.junit.platform.testkit.engine.EngineTestKit;
@@ -68,36 +66,35 @@ class ParallelApplicationEventsIntegrationTests {
 	void rejectTestsInParallelWithInstancePerClassAndRecordApplicationEvents() {
 		Class<?> testClass = TestInstancePerClassTestCase.class;
 
-		final EngineExecutionResults results = EngineTestKit.engine("junit-jupiter")//
+		EngineExecutionResults results = EngineTestKit.engine("junit-jupiter")//
 				.selectors(selectClass(testClass))//
 				.configurationParameter("junit.jupiter.execution.parallel.enabled", "true")//
+				.configurationParameter("junit.jupiter.execution.parallel.mode.default", "concurrent")//
 				.configurationParameter("junit.jupiter.execution.parallel.config.dynamic.factor", "10")//
 				.execute();
 
-		//extract the messages from failed TextExecutionResults
+		// extract the messages from failed TextExecutionResults
 		assertThat(results.containerEvents().failed()//
-				.stream().map(e -> e.getRequiredPayload(TestExecutionResult.class)//
-				.getThrowable().get().getMessage()))//
+					.map(e -> e.getRequiredPayload(TestExecutionResult.class).getThrowable().get().getMessage()))//
 				.singleElement(InstanceOfAssertFactories.STRING)
-				.isEqualToIgnoringNewLines("""
-		Test classes or inner classes that @RecordApplicationEvents\s
-		must not be run in parallel with the @TestInstance(Lifecycle.PER_CLASS) configuration.\s
-		Use either @Execution(SAME_THREAD), @TestInstance(PER_METHOD) or disable parallel\s
-		execution altogether. Note that when recording events in parallel, one might see events\s
-		published by other tests as the application context can be common.
-		""");
+				.isEqualTo("""
+					Test classes or @Nested test classes that @RecordApplicationEvents must not be run \
+					in parallel with the @TestInstance(PER_CLASS) lifecycle mode. Configure either \
+					@Execution(SAME_THREAD) or @TestInstance(PER_METHOD) semantics, or disable parallel \
+					execution altogether. Note that when recording events in parallel, one might see events \
+					published by other tests since the application context may be shared.""");
 	}
 
 	@Test
-	void executeTestsInParallelInstancePerMethod() {
+	void executeTestsInParallelWithInstancePerMethod() {
 		Class<?> testClass = TestInstancePerMethodTestCase.class;
 		Events testEvents = EngineTestKit.engine("junit-jupiter")//
 				.selectors(selectClass(testClass))//
 				.configurationParameter("junit.jupiter.execution.parallel.enabled", "true")//
 				.configurationParameter("junit.jupiter.execution.parallel.config.dynamic.factor", "10")//
 				.execute()//
 				.testEvents();
-		//list failed events in case of test errors to get a sense of which tests failed
+		// list failed events in case of test errors to get a sense of which tests failed
 		Events failedTests = testEvents.failed();
 		if (failedTests.count() > 0) {
 			failedTests.debug();
@@ -141,7 +138,6 @@ void resetPayloads() {
 
 	@SpringJUnitConfig
 	@RecordApplicationEvents
-	@Execution(ExecutionMode.CONCURRENT)
 	@TestInstance(Lifecycle.PER_METHOD)
 	static class TestInstancePerMethodTestCase {
 
@@ -211,7 +207,7 @@ void compareToApplicationEventsHolder(ApplicationEvents applicationEvents) {
 
 		@Test
 		void asyncPublication(ApplicationEvents events) throws InterruptedException {
-			final ExecutorService executorService = Executors.newSingleThreadExecutor();
+			ExecutorService executorService = Executors.newSingleThreadExecutor();
 			executorService.execute(() -> this.context.publishEvent("asyncPublication"));
 			executorService.shutdown();
 			executorService.awaitTermination(10, TimeUnit.SECONDS);