-
Notifications
You must be signed in to change notification settings - Fork 38.2k
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
Discover test configuration on enclosing class for nested test class [SPR-15366] #19930
Comments
Sam Brannen commented Current work is being performed in the following branch: https://github.com/sbrannen/spring-framework/tree/SPR-15366 |
Andy Wilkinson commented Sam asked me to comment on this with reference to this Spring Boot issue. In looking at that issue I observed that the following test would start two separate application contexts: @ExtendWith(SpringExtension.class)
@SpringBootTest
public class TwoDifferentContextsTests {
@Autowired
ApplicationContext context;
@Nested
class NestedTests {
@Test
public void test() {
}
}
} One context is started for // If neither of the candidates supports the mergedConfig based on resources but
// ACIs or customizers were declared, then delegate to the annotation config
// loader.
if (!mergedConfig.getContextInitializerClasses().isEmpty() || !mergedConfig.getContextCustomizers().isEmpty()) {
return delegateLoading(getAnnotationConfigLoader(), mergedConfig);
}
// else...
throw new IllegalStateException(String.format(
"Neither %s nor %s was able to load an ApplicationContext from %s.", name(getXmlLoader()),
name(getAnnotationConfigLoader()), mergedConfig)); |
Sam Brannen commented I actually assumed it was due to an auto-registered feature of Spring Boot Test (i.e., something like a So thank you for confirming my suspicions! |
Stefan Ludwig commented FYI: I've extended the original bug example from the (GitHub issue) with "Boot - less" implementations of The following test fails because the @SpringJUnitWebConfig(classes = Application.class)
@ExtendWith(RestDocumentationExtension.class)
class FailingTest {
MockMvc mockMvc;
@BeforeEach
void setup(WebApplicationContext wac, RestDocumentationContextProvider restDocumentation) {
mockMvc = MockMvcBuilders.webAppContextSetup(wac) //
.apply(documentationConfiguration(restDocumentation)) //
.build();
}
@Nested
class NestedTests {
@Test
void testAndDocumentation() throws Exception {
mockMvc.perform(get("/hello"))
.andExpect(status().is2xxSuccessful())
.andExpect(jsonPath("message", equalTo("Hello World!")))
.andDo(document("hello-get-200"));
}
}
} Stacktrace;
If the context configuration is duplicated by adding (comment moved from SPR-16595) |
Sam Brannen commented I think the behavior you've described might actually be a bug in JUnit Jupiter instead of Spring. Can you please tell me what happens if you move the setup for @SpringJUnitWebConfig(Application.class)
@ExtendWith(RestDocumentationExtension.class)
class FailingTest {
MockMvc mockMvc;
FailingTest(WebApplicationContext wac, RestDocumentationContextProvider restDocumentation) {
mockMvc = MockMvcBuilders.webAppContextSetup(wac) //
.apply(documentationConfiguration(restDocumentation)) //
.build();
}
@Nested
class NestedTests {
@Test
void testAndDocumentation() throws Exception {
mockMvc.perform(get("/hello"))
.andExpect(status().is2xxSuccessful())
.andExpect(jsonPath("message", equalTo("Hello World!")))
.andDo(document("hello-get-200"));
}
}
} Thanks in advance for feedback! |
Stefan Ludwig commented If I do that, it works! |
Sam Brannen commented
Awesome... and depressing... at the same time. ;-) Awesome that it works for you! Depressing (in a facetious way) for me because that means it is in fact an issue in JUnit Jupiter's handling of And if that's too much of a mouthful for you, don't worry: I'll address the issue you've described within JUnit Jupiter. Cheers, Sam |
Stefan Ludwig commented Glad to have helped! And sorry to depress you ;) But are you sure it's not Springs management / caching of application contexts? I thought it might be the same issue as, or at least very closely related to, #21136 because when I debug the following example: @SpringJUnitWebConfig(classes = Application.class)
@ExtendWith(RestDocumentationExtension.class)
class FailingTest {
MockMvc mockMvc;
@BeforeEach
void setup(WebApplicationContext wac, RestDocumentationContextProvider restDocumentation) {
mockMvc = MockMvcBuilders.webAppContextSetup(wac) //
.apply(documentationConfiguration(restDocumentation)) //
.build();
}
@Test
void testAndDocumentation() throws Exception {
mockMvc.perform(get("/hello"))
.andExpect(status().is2xxSuccessful())
.andExpect(jsonPath("message", equalTo("Hello World!")))
.andDo(document("hello-get-200"));
}
@Nested
class NestedTests {
@Test
void testAndDocumentation() throws Exception {
mockMvc.perform(get("/hello"))
.andExpect(status().is2xxSuccessful())
.andExpect(jsonPath("message", equalTo("Hello World!")))
.andDo(document("hello-get-200"));
}
}
} Adding a breakpoint in the That sounds to me like what was described in #21136. Especially because adding |
Sam Brannen commented FYI: I have opened the following issue for JUnit Jupiter. |
Sam Brannen commented
Well, you have Otherwise, Spring will not load any |
Sam Brannen commented
No. #21136 covers use cases where |
Stefan Ludwig commented Oh, I did not think about the |
Sam Brannen commented You're welcome. And... yeah... I agree that having |
Sam Brannen commented
Speaking of which, thanks for creating that! It's a nice collection of examples. |
Andy Wilkinson commented
It doesn’t. You have to using |
Sam Brannen commented I of course meant "change things for tests executing with the Spring TestContext Framework". Sorry if that was not apparent. |
Sam Brannen commented
No, there is currently no built-in mechanism for disabling a Maybe we should introduce something analogous to Boot's |
Sam Brannen commented Another question on Stack Overflow: https://stackoverflow.com/questions/53236807/spring-boot-testing-run-script-in-a-nested-test-sql-script-sql/53240875 |
Hey @sbrannen, I got several developers (including the awesome @jnizet) asking for a fix for spring-projects/spring-boot#12470 which depend on that issues, any chance you could fix it for 5.2 in order to allow Boot team to support correctly nested classes in 2.2? |
@sdeleuze, I'll do my best to prioritize this one. |
Now tentatively slated for 5.2 M1. |
Use MetaAnnotationUtils.findMergedAnnotation() instead of the MergedAnnotations API in order to provide proper support for @NestedTestConfiguration. See gh-19930
Use MetaAnnotationUtils.findMergedAnnotation() instead of the MergedAnnotations API in order to provide proper support for @NestedTestConfiguration. See gh-19930
This commit also includes several experiments that will be later deleted. These will remain here in the commit history so that they do not get lost in case parts of them are needed at a later date. See gh-19930
Reopening to address additional deliverables. |
This commit introduces TestContextAnnotationUtils as a replacement for MetaAnnotationUtils, with dedicated support for honoring the new @NestedTestConfiguration annotation and related annotation search semantics. MetaAnnotationUtils has been reverted to its previous scope and is now deprecated. See gh-19930
gh-19930 introduced support for finding class-level test configuration annotations on enclosing classes when using JUnit Jupiter @nested test classes, but support for @DynamicPropertySource methods got overlooked since they are method-level annotations. This commit addresses this shortcoming by introducing full support for @NestedTestConfiguration semantics for @DynamicPropertySource methods on enclosing classes. Closes gh-26091
The TYPE_HIERARCHY_AND_ENCLOSING_CLASSES search strategy for MergedAnnotations was originally introduced to support @nested test classes in JUnit Jupiter (see spring-projects#23378). However, while implementing spring-projects#19930, we determined that the TYPE_HIERARCHY_AND_ENCLOSING_CLASSES search strategy unfortunately could not be used since it does not allow the user to control when to recurse up the enclosing class hierarchy. For example, this search strategy will automatically search on enclosing classes for static nested classes as well as for inner classes, when the user probably only wants one such category of "enclosing class" to be searched. Consequently, TestContextAnnotationUtils was introduced in the Spring TestContext Framework to address the shortcomings of the TYPE_HIERARCHY_AND_ENCLOSING_CLASSES search strategy. Since this search strategy is unlikely to be useful to general users, the team has decided to deprecate this search strategy in Spring Framework 5.3.x and remove it in 6.0. Closes spring-projectsgh-28079
Sam Brannen opened SPR-15366 and commented
Status Quo
Spring's support for JUnit Jupiter already supports detection of test configuration (e.g., (
@ContextConfiguration
) on@Nested
classes.However, if a
@Nested
class does not declare its own test configuration, Spring will not find the configuration from the enclosing class.See also this discussion on Stack Overflow regarding nested test classes and
@Transactional
.Proposal
Inspired by issue 8 from the
spring-test-junit5
project, it would perhaps be desirable if the Spring TestContext Framework would discover test configuration on an enclosing class for a@Nested
test class.Deliverables
@NestedTestConfiguration
withEnclosingConfiguration
enum supportingINHERITED
andOVERRIDE
modes.EnclosingConfiguration
mode globally configurable viaSpringProperties
EnclosingConfiguration
mode toINHERITED
@ContextConfiguration
/@ContextHierarchy
@ActiveProfiles
@TestPropertySource
/@TestPropertySources
@WebAppConfiguration
@TestConstructor
@BootstrapWith
@TestExecutionListeners
@DirtiesContext
@Transactional
@Rollback
/@Commit
@Sql
/@SqlConfig
/@SqlGroup
@NestedTestConfiguration
support in the reference manual@NestedTestConfiguration
support and the switching of the default mode toINHERITED
in the upgrade notes in the wikiAffects: 5.0
Issue Links:
@Nested
class cannot share enclosing class's ApplicationContext if nested class is deemed to be a configuration candidate6 votes, 10 watchers
The text was updated successfully, but these errors were encountered: