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

JUnit Jupiter @Nested class cannot share enclosing class's ApplicationContext if nested class is deemed to be a configuration candidate [SPR-16595] #21136

Closed
spring-projects-issues opened this issue Mar 15, 2018 · 19 comments
Assignees
Labels
in: test Issues in the test module type: bug A general bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Mar 15, 2018

Andy Wilkinson opened SPR-16595 and commented

This feels like a bug, although it may just be a (rather large) limitation.

It appears to be impossible for an inner test class annotated with @Nested to use the enclosing class's application context if the enclosing class is deemed to be a configuration candidate (for example, because it uses @Import).

To be able to share the context, the inner test class must have the same configuration as its enclosing class so that they have the same context cache key. In other words, if the enclosing class has used @Import the inner class must do so too. This leads to code like this:

import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.springframework.context.annotation.Import;
import org.springframework.test.context.junit.jupiter.SpringExtension;

@ExtendWith(SpringExtension.class)
@Import(Imported.class)
class NestedLimitationsTests {

	@Nested
	@Import(Imported.class)
	class NestedTests {

		@Test
		public void test() {

		}

	}

}

class Imported {
}

This fails to launch with the following exception:

Caused by: org.springframework.beans.factory.UnsatisfiedDependencyException: Error creating bean with name 'example.NestedLimitationsTests$NestedTests': Unsatisfied dependency expressed through constructor parameter 0; nested exception is org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type 'example.NestedLimitationsTests' available: expected at least 1 bean which qualifies as autowire candidate. Dependency annotations: {}
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:729)
	at org.springframework.beans.factory.support.ConstructorResolver.autowireConstructor(ConstructorResolver.java:192)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.autowireConstructor(AbstractAutowireCapableBeanFactory.java:1270)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBeanInstance(AbstractAutowireCapableBeanFactory.java:1127)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.doCreateBean(AbstractAutowireCapableBeanFactory.java:545)
	at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.createBean(AbstractAutowireCapableBeanFactory.java:502)
	at org.springframework.beans.factory.support.AbstractBeanFactory.lambda$doGetBean$0(AbstractBeanFactory.java:312)
	at org.springframework.beans.factory.support.DefaultSingletonBeanRegistry.getSingleton(DefaultSingletonBeanRegistry.java:228)
	at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:310)
	at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:200)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.preInstantiateSingletons(DefaultListableBeanFactory.java:760)
	at org.springframework.context.support.AbstractApplicationContext.finishBeanFactoryInitialization(AbstractApplicationContext.java:868)
	at org.springframework.context.support.AbstractApplicationContext.refresh(AbstractApplicationContext.java:549)
	at org.springframework.test.context.support.AbstractGenericContextLoader.loadContext(AbstractGenericContextLoader.java:128)
	at org.springframework.test.context.support.AbstractGenericContextLoader.loadContext(AbstractGenericContextLoader.java:60)
	at org.springframework.test.context.support.AbstractDelegatingSmartContextLoader.delegateLoading(AbstractDelegatingSmartContextLoader.java:107)
	at org.springframework.test.context.support.AbstractDelegatingSmartContextLoader.loadContext(AbstractDelegatingSmartContextLoader.java:251)
	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContextInternal(DefaultCacheAwareContextLoaderDelegate.java:99)
	at org.springframework.test.context.cache.DefaultCacheAwareContextLoaderDelegate.loadContext(DefaultCacheAwareContextLoaderDelegate.java:117)
	... 88 common frames omitted
Caused by: org.springframework.beans.factory.NoSuchBeanDefinitionException: No qualifying bean of type 'example.NestedLimitationsTests' available: expected at least 1 bean which qualifies as autowire candidate. Dependency annotations: {}
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.raiseNoMatchingBeanFound(DefaultListableBeanFactory.java:1509)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.doResolveDependency(DefaultListableBeanFactory.java:1104)
	at org.springframework.beans.factory.support.DefaultListableBeanFactory.resolveDependency(DefaultListableBeanFactory.java:1065)
	at org.springframework.beans.factory.support.ConstructorResolver.resolveAutowiredArgument(ConstructorResolver.java:815)
	at org.springframework.beans.factory.support.ConstructorResolver.createArgumentArray(ConstructorResolver.java:721)
	... 106 common frames omitted

An attempt is being made to create a NestedTests bean as its @Import annotation means that ConfigurationClassUtils considers it to be a configuration candidate. An instance of the enclosing class is needed to create NestedTests as the class is not static. It fails because the enclosing class isn't available as a bean.

Declaring the inner class as static overcomes the immediate problem but creates another. Being static prevents the inner class from accessing the enclosing class's non-static fields and declaring the enclosing class's fields as static means that they're no longer autowired.

The situation that's described above means that many of Spring Boot's testing annotations do not work. In fact, as far as I can tell, the only one that does work is @SpringBootTest as it is not meta-annotated with @Import.


Affects: 5.0.4

Reference URL: spring-projects/spring-restdocs#490

Issue Links:

Referenced from: commits spring-attic/spring-framework-issues@7a3db4a

1 votes, 6 watchers

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 15, 2018

Sam Brannen commented

This is related to #19930.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

This feels like a bug, although it may just be a (rather large) limitation.

It appears to be impossible for an inner test class annotated with @Nested to use the enclosing class's application context if the enclosing class is deemed to be a configuration candidate (for example, because it uses @Import).

Well, that doesn't appear to be the case in Core Spring.

For example, annotating NestedTestCase (in NestedTestsWithSpringAndJUnitJupiterTestCase in the spring-test test suite) with @Import does not result in any exception.

Thus, I assume it is Spring Boot Test's custom support for annotating test classes with @Import that is causing the behavior you have described.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Oops. I misread your explanation.

In any case, annotating NestedTestsWithSpringAndJUnitJupiterTestCase in the spring-test test suite with @Import also does not result in any exception.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

A little digging reveals that org.springframework.boot.test.context.ImportsContextCustomizer.ImportsSelector from spring-boot-test is selecting the test class as an imported @Configuration class even though it's an inner class which is not a valid candidate for a @Configuration class.

So perhaps Spring Boot should look into a solution?

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Or...

Juergen Hoeller, do you think Core Spring should do anything here?

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

Stéphane Nicoll, what's your take on this?

@spring-projects-issues
Copy link
Collaborator Author

Andy Wilkinson commented

org.springframework.boot.test.context.ImportsContextCustomizer.ImportsSelector from spring-boot-test is selecting the test class as an imported @Configuration class even though it's an inner class which is not a valid candidate for a @Configuration class

That doesn't match what I'm seeing. Boot's ImportsSelector is only selecting the enclosing class. It's ConfigurationClassParser that then processes the member classes. It finds an @Import meta-annotation on the inner class so it deems it to be a configuration candidate despite it not being static.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Oh.

OK... I didn't literally debug it. So I'll have to investigate further.

Thanks for the tip!

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Aha.... I'm guessing you're referring to ConfigurationClassUtils.isLiteConfigurationCandidate(...) which is invoked indirectly by ConfigurationClassParser.processMemberClasses(...).

isLiteConfigurationCandidate(...) considers any class annotated with any of the following to be a "lite" configuration class.

  • @Component
  • @ComponentScan
  • @Import
  • @ImportResource

So, it looks like a possible solution to the problem would be to add a "not an inner class" check.

Juergen Hoeller, what do you think about that proposed change?

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Actually, I think it would be a good idea to ensure that isConfigurationCandidate(...) (which calls isFullConfigurationCandidate() and isLiteConfigurationCandidate()) always returns false if the candidate class is an inner class, since a configuration class can never be an inner class.

Or am I missing something?

Juergen Hoeller?

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

As far as I see, at the ConfigurationClassParser level we identify and process all configuration classes, ultimately failing for non-static inner classes when trying to construct them... but not simply ignoring them upfront. If we'd like to completely ignore them for certain scenarios, we'll probably need a more specific check at a different level...or some kind of marker that tells us it's clearly not something ConfigurationClassParser would have to process since it's something else's job.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

I've put some more thought into the overall issue, and I believe that the fix needs to be in Spring Boot.

I unfortunately don't have time to go into details now, but I'll try to put together a list of failing use cases in Spring Boot Test with regard to how it currently supports @Import on test classes.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Mar 16, 2018

Andy Wilkinson commented

Thanks, Stefan. #19930 is tracking that part of the problem.

@spring-projects-issues
Copy link
Collaborator Author

Stefan Ludwig commented

Ups - I moved the comment there.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

If we'd like to completely ignore them for certain scenarios, we'll probably need a more specific check at a different level...or some kind of marker that tells us it's clearly not something ConfigurationClassParser would have to process since it's something else's job.

Juergen Hoeller, after having put more thought into it, I tend to agree with you.

As I currently see it, I think there are only two real options.

  1. Spring Core could introduce a mechanism for explicitly marking an inner class as not being a configuration candidate. This feature could then be leveraged in Spring Boot Test.
  2. Spring Boot Test could implement a new mechanism to support @Import that does not use Spring Core's infrastructure for processing configuration classes.

Juergen Hoeller, Andy Wilkinson, Stéphane Nicoll,

What do you think?

Thoughts?

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

I'm actually not yet certain that proposal #1 above would work.

Consider the fact that one can introduce multiple levels of @Nested test classes in JUnit Jupiter, and each of those non-static nested classes (i.e., inner classes) could in turn be annotated with @Import. In such a scenario, even though the outermost enclosing class would either be a top-level or static nested class -- which would allow it to be directly annotated with @Import but have its member classes ignored when treated as the "test class" -- what would happen when a @Nested test class annotated with @Import is used as the "test class" for loading an ApplicationContext?

Would that work in conjunction with how Spring Boot Test supports @Import?

If you know the answer, feel free to share it.

Otherwise just consider these rhetorical questions that I will investigate later. ;-)

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Sep 12, 2018

Juergen Hoeller commented

This should be sorted out along with #21379 now since we're only going to consider nested classes on component types now, not for classes with any configuration indicators such as @Import.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Great, Juergen Hoeller!

Thanks

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

FYI: I confirmed the fix by submitting a sample application.

https://github.com/spring-projects/spring-framework-issues/tree/master/SPR-16595

Specifically, if you run the build against Spring 5.1.0.RC3, it fails. Whereas, if you switch to 5.1.0.BUILD-SNAPSHOT (as checked into GitHub) it passes.

https://github.com/spring-projects/spring-framework-issues/blob/7a3db4a47857dbbfa1af5551dc1566e6f561f15a/SPR-16595/build.gradle#L24-L27

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants