Skip to content

Allow @PropertySource to be specified on a test class [SPR-10232] #14865

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

Closed
spring-projects-issues opened this issue Jan 29, 2013 · 9 comments
Closed
Assignees
Labels
in: test Issues in the test module status: duplicate A duplicate of another issue type: enhancement A general enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jan 29, 2013

Paul Tomlin opened SPR-10232 and commented

Overview

When using the TestContext framework it would be nice to easily be able to specify some PropertySources to be added to the Environment, in much the same way as @ActiveProfiles allows specifying the active profiles.

Ideally, something like below would result in a ResourcePropertySource being registered with the environment of the test's ApplicationContext prior to refresh:

@ContextConfiguration
@PropertySource("classpath:foo.properties")
public class MyTest {
   // ...
}

Work-around

In Spring Framework 3.1, there is a workaround, but it's not nearly as tidy as the above proposal.

@ContextConfiguration(
  locations = { ... },
  loader = MyTest.CustomeContextLoader.class
)
public class MyTest {
  public static class CustomContextLoader extends GenericXmlContextLoader {
    @Override
    protected void customizeContext(GenericApplicationContext context) {
      // exception handling elided
      context.getEnvironment()
             .getPropertySources()
             .addFirst(new ResourcePropertySource("classpath:foo.properties"));
    }
  }
}

There may be something in 3.2.x, probably as a result of #13650 and ApplicationContextInitializer, but ContextLoaderUtils doesn't seem to suggest so, and I'm not yet familiar enough to know.


Analysis

  • As with @ActiveProfiles, @PropertySource declarations on test classes should be inherited by default but overridable.
  • @PropertySource is not an @Inherited annotation, but AnnotationUtils.findAnnotationDeclaringClass() should take care of this.
  • Inheritance and overriding behavior of @PropertySource in integration tests must be consistent with the existing behavior in @Configuration classes.
    • See code snippets from ConfigurationClassPostProcessor and ConfigurationClassParser below.
  • As far as possible, the existing business logic in ConfigurationClassParser.processPropertySource() should be reused and not duplicated in the testing framework; in other words, consider extracting the existing logic into a static utility method or similar.
  • The context cache key (i.e., MergedContextConfiguration) must take test property sources into account.
Relevant code from ConfigurationClassPostProcessor
// ...
// Handle any @PropertySource annotations
Stack<PropertySource<?>> parsedPropertySources = parser.getPropertySources();
if (!parsedPropertySources.isEmpty()) {
	if (!(this.environment instanceof ConfigurableEnvironment)) {
		logger.warn("Ignoring @PropertySource annotations. " +
				"Reason: Environment must implement ConfigurableEnvironment");
	}
	else {
		MutablePropertySources envPropertySources = ((ConfigurableEnvironment)this.environment).getPropertySources();
		while (!parsedPropertySources.isEmpty()) {
			envPropertySources.addLast(parsedPropertySources.pop());
		}
	}
}
// ...
Relevant code from ConfigurationClassParser
private void processPropertySource(AnnotationAttributes propertySource) throws IOException {
	String name = propertySource.getString("name");
	String[] locations = propertySource.getStringArray("value");
	int nLocations = locations.length;
	if (nLocations == 0) {
		throw new IllegalArgumentException("At least one @PropertySource(value) location is required");
	}
	for (int i = 0; i < nLocations; i++) {
		locations[i] = this.environment.resolveRequiredPlaceholders(locations[i]);
	}
	ClassLoader classLoader = this.resourceLoader.getClassLoader();
	if (!StringUtils.hasText(name)) {
		for (String location : locations) {
			this.propertySources.push(new ResourcePropertySource(location, classLoader));
		}
	}
	else {
		if (nLocations == 1) {
			this.propertySources.push(new ResourcePropertySource(name, locations[0], classLoader));
		}
		else {
			CompositePropertySource ps = new CompositePropertySource(name);
			for (String location : locations) {
				ps.addPropertySource(new ResourcePropertySource(location, classLoader));
			}
			this.propertySources.push(ps);
		}
	}
}

Affects: 3.1 GA

Issue Links:

Referenced from: commits 3210041

4 votes, 8 watchers

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Paul Tomlin,

Indeed, an ApplicationContextInitializer is the preferred mechanism for setting a PropertySource for an integration test, if you are using String-based locations (i.e., XML configuration files).

On the other hand, if you're using annotated classes (i.e., @Configuration classes), you can simply annotate one of those classes with @PropertySource. For example, if you use a nested, static @Configuration class within your test class, you could annotate that configuration class with @PropertySource and @Import, where the latter imports your production configuration classes.

I am therefore inclined to close this issue, but I'll leave it open for the time being in order to allow others to comment.

Regards,

Sam

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Out of curiosity, why did you look to ContextLoaderUtils for suggestions?

After all, it's not exactly a public API. ;)

However, I will update the class-level Javadoc for ContextLoaderUtils to mention ApplicationContextInitializer support as well.

@spring-projects-issues
Copy link
Collaborator Author

Paul Tomlin commented

I looked at ContextLoaderUtils because it's the class that handles the registration of @ActiveProfile annotations on the test class into the test context. I was looking for similar support for @PropertySource.

I realize there are a few ways of achieving registration of PropertySources, such as the custom ContextLoader or @Configuration with @PropertySource, but in terms of out of the box support, it seems PropertySources are second class when compared to profiles.

I don't know how hard it would be to build an ApplicationContextInitializer that looks at the test class for @PropertySource annotations, and applies them to the context, as I'm not in 3.2 land yet.

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Ah, OK. Let me explain the status quo.

Support for @ActiveProfiles was added in 3.1; whereas, support for ApplicationContextInitalizer classes (via the initializers attribute of @ContextConfiguration) was not added until 3.2.

Thus, at the time of 3.1, @ActiveProfiles was the only way to set active bean definition profiles in a test (other than implementing a custom SmartContextLoader).

However, the added support for ApplicationContextInitalizer classes in 3.2 basically makes the use of @ActiveProfiles no longer necessary, since an ApplicationContextInitalizer can be used to programmatically configure the application context (e.g., to set the active profiles or register custom property sources).

In other words, a custom ApplicationContextInitalizer is a one-stop shop for programmatically configuring your application context, and the nice part about an ApplicationContextInitalizer is that you can reuse it across different test class hierarchies. So if you're looking for a simple, reusable, programmatic approach, an ApplicationContextInitalizer is the way to go. Having said that, however, in contrast to @ActiveProfiles there is no out-of-the-box support for declaratively configuring property sources for integration tests.

In summary, it all boils down to whether or not there is sufficient added value associated with a declarative approach.

- Sam

@spring-projects-issues
Copy link
Collaborator Author

Paul Tomlin commented

Understood and agreed. +1 for "yes please" ;)

@spring-projects-issues
Copy link
Collaborator Author

Sam Brannen commented

Paul Tomlin,

While analyzing the implementation of ConfigurationClassPostProcessor.processConfigBeanDefinitions(), I came up with the following question...

In your CustomContextLoader, why did you invoke addFirst() instead of addLast() on the environment's MutablePropertySources?

@spring-projects-issues
Copy link
Collaborator Author

Emerson Farrugia commented

Although adding a (possibly nested) @Configuration to use a @PropertySource works, it seems like an unnecessary hoop if that's the only benefit the @Configuration is giving you. Supporting @PropertySource on @ContextConfiguration seems more intuitive to me, but it's admittedly subjective.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Jul 31, 2014

Sam Brannen commented

Please note that this issue has been superseded by #16667.

@spring-projects-issues
Copy link
Collaborator Author

spring-projects-issues commented Aug 13, 2014

Sam Brannen commented

If you are following this issue, you might be interested in knowing that #16667 has been resolved.

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 status: duplicate A duplicate of another issue type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

2 participants