root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

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
This commit is contained in:
Sam Brannen 2023-05-13 19:17:18 +02:00
parent 5e1a4740d7
commit f3f3dc693a
2 changed files with 35 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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);
}
}

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

@ -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);