From f3f3dc693ac3b5eabda2178b30bce833a4b1cc02 Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Sat, 13 May 2023 19:17:18 +0200 Subject: [PATCH] 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 --- .../junit/jupiter/SpringExtension.java | 47 +++++++++---------- ...llelApplicationEventsIntegrationTests.java | 30 +++++------- 2 files changed, 35 insertions(+), 42 deletions(-) diff --git a/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java b/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java index aebf7280ec..f4aaa32f0c 100644 --- a/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java +++ b/spring-test/src/main/java/org/springframework/test/context/junit/jupiter/SpringExtension.java @@ -29,7 +29,7 @@ import org.junit.jupiter.api.AfterAll; 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.ExtensionContext.Store; 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 class SpringExtension implements BeforeAllCallback, AfterAllCallback, Tes } /** - * 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 @@ public class SpringExtension implements BeforeAllCallback, AfterAllCallback, Tes 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; - } - final TestInstance testInstance = TestContextAnnotationUtils.findMergedAnnotation(testClass, TestInstance.class); - - if (testInstance == null || testInstance.value() != TestInstance.Lifecycle.PER_CLASS) { - return NO_AUTOWIRED_VIOLATIONS_DETECTED; + boolean recording = TestContextAnnotationUtils.hasAnnotation(testClass, RecordApplicationEvents.class); + if (!recording) { + 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.getTestInstanceLifecycle().orElse(Lifecycle.PER_METHOD) == Lifecycle.PER_METHOD) { + 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."; + if (context.getExecutionMode() == ExecutionMode.SAME_THREAD) { + return NO_VIOLATIONS_DETECTED; + } + + 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,7 +203,7 @@ public class SpringExtension implements BeforeAllCallback, AfterAllCallback, Tes 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, " + @@ -214,7 +211,7 @@ public class SpringExtension implements BeforeAllCallback, AfterAllCallback, Tes testClass.getName(), Arrays.toString(methodsWithErrors))); }, String.class); - if (errorMessage != NO_AUTOWIRED_VIOLATIONS_DETECTED) { + if (errorMessage != NO_VIOLATIONS_DETECTED) { throw new IllegalStateException(errorMessage); } } diff --git a/spring-test/src/test/java/org/springframework/test/context/junit/jupiter/event/ParallelApplicationEventsIntegrationTests.java b/spring-test/src/test/java/org/springframework/test/context/junit/jupiter/event/ParallelApplicationEventsIntegrationTests.java index fd1a27973d..109665bb9a 100644 --- a/spring-test/src/test/java/org/springframework/test/context/junit/jupiter/event/ParallelApplicationEventsIntegrationTests.java +++ b/spring-test/src/test/java/org/springframework/test/context/junit/jupiter/event/ParallelApplicationEventsIntegrationTests.java @@ -32,8 +32,6 @@ import org.junit.jupiter.api.Test; 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,28 +66,27 @@ 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))// @@ -97,7 +94,7 @@ class ParallelApplicationEventsIntegrationTests { .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 @@ class ParallelApplicationEventsIntegrationTests { @SpringJUnitConfig @RecordApplicationEvents - @Execution(ExecutionMode.CONCURRENT) @TestInstance(Lifecycle.PER_METHOD) static class TestInstancePerMethodTestCase { @@ -211,7 +207,7 @@ class ParallelApplicationEventsIntegrationTests { @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);