Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Honor @NestedTestConfiguration semantic in BatchTestContextCustomizerFactory #4739

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

scordio
Copy link
Contributor

@scordio scordio commented Jan 3, 2025

Prior to this commit, BatchTestContextCustomizerFactory only looked for @SpringBatchTest on the given test class as a marker annotation. While this works fine for top-level classes, it doesn't work correctly with JUnit @Nested classes, resulting in a distinct ApplicationContext for the nested class as a side effect.

With this change, the factory no longer uses AnnotatedElementUtils.hasAnnotation and favors instead TestContextAnnotationUtils.hasAnnotation, which honors the @NestedTestConfiguration semantic.

Fixes #4738.

@scordio scordio force-pushed the gh-4738-spring-batch-test branch from 28dcfe4 to bd2b2a9 Compare January 4, 2025 08:56
@scordio scordio changed the title Replace spring.factories with @ContextCustomizerFactories Use dedicated ContextCustomizerFactory for @SpringBatchTest Jan 4, 2025
@scordio
Copy link
Contributor Author

scordio commented Jan 4, 2025

Hi @sbrannen, since you authored @ContextCustomizerFactories, may I also ask your opinion about my solution for #4738?

@sbrannen
Copy link
Member

sbrannen commented Jan 8, 2025

Having only viewed this on my phone, I can’t say for sure, but… my hunch is that the solution is to replace AnnotatedElementUtils.hasAnnotation(testClass, SpringBatchTest.class) with TestContextAnnotationUtils.hasAnnotation(testClass, SpringBatchTest.class) and then revert all other changes.

Give that a try and let me know how it goes… 😉

@scordio
Copy link
Contributor Author

scordio commented Jan 8, 2025

Thanks, will do!

@scordio scordio force-pushed the gh-4738-spring-batch-test branch from bd2b2a9 to b188e4b Compare January 8, 2025 21:26
@scordio scordio changed the title Use dedicated ContextCustomizerFactory for @SpringBatchTest Honor @NestedTestConfiguration semantic in BatchTestContextCustomizerFactory Jan 8, 2025
@scordio
Copy link
Contributor Author

scordio commented Jan 8, 2025

Thanks a lot, @sbrannen!

With your input, the fix is now simpler and doesn't require the removal of spring.factories 👍

@scordio scordio force-pushed the gh-4738-spring-batch-test branch 2 times, most recently from 2f5f3e8 to bcb8fd8 Compare January 8, 2025 21:44
@scordio scordio force-pushed the gh-4738-spring-batch-test branch from bcb8fd8 to 690c683 Compare January 8, 2025 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

@SpringBatchTest with JUnit @Nested triggers new test context creation
2 participants