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

Add YamlPropertySourceFactory that can be used for loading YAML files through the @TestPropertySource annotation #42603

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

Conversation

nosan
Copy link
Contributor

@nosan nosan commented Oct 11, 2024

I have prepared two potential solutions for this #33434 enhancement.

The first approach offers a simple and intuitive way to use it:

@SpringJUnitConfig
@TestPropertySource(locations = "classpath:test.yaml", factory = YamlPropertySourceFactory.class)
class SomeTest {

}

YamlPropertySourceFactory loads all properties from the specified YAML file using YamlPropertySourceLoader and returns a CompositePropertySource containing the loaded properties.

As a potential improvement, a dedicated annotation like @TestYamlPropertySource could be introduced to simplify the loading of YAML files in tests.

@Target(ElementType.TYPE)
@Retention(RetentionPolicy.RUNTIME)
@Documented
@Inherited
@TestPropertySource(factory = YamlPropertySourceFactory.class)
@Repeatable(TestYamlPropertySources.class)
public @interface TestYamlPropertySource {

	@AliasFor(attribute = "value", annotation = TestPropertySource.class)
	String[] value() default {};

	@AliasFor(attribute = "locations", annotation = TestPropertySource.class)
	String[] locations() default {};

	@AliasFor(attribute = "inheritLocations", annotation = TestPropertySource.class)
	boolean inheritLocations() default true;

	@AliasFor(attribute = "properties", annotation = TestPropertySource.class)
	String[] properties() default {};

	@AliasFor(attribute = "inheritProperties", annotation = TestPropertySource.class)
	boolean inheritProperties() default true;

}

With a dedicated annotation, the following syntax could be supported:

@SpringJUnitConfig
@TestYamlPropertySource({ "classpath:test.yaml", "classpath:test1.yaml" })
@TestYamlPropertySource(locations = "classpath:test2.yaml", properties = "key:value")
class TestYamlPropertySourceIntegrationTests {

}

An alternative approach is to customize PropertySourceDescriptor in SpringBootTestContextBootstrapper by using the PropertySourceLoaderPropertySourceFactory, which leverages the existing PropertySourceLoader. With that approach, the following syntax would be possible:

@SpringBootTest
@TestPropertySource(locations = "classpath:test.yaml")
class SomeTest {
	
}

But it has several disadvantages:

  • It works only with the @SpringBootTest annotation. For @SpringJUnitConfig, the factory needs to be explicitly used.

  • It does not support EncodedResource for .properties files because PropertySourceLoader only works with Resource objects. As a result, if someone is using @TestPropertySource with the encoding attribute, these changes could potentially impact them.

  • Not obvious.

The first approach is much better and straightforward, and people would not have any problem using it.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 11, 2024
@nosan nosan force-pushed the 33434 branch 2 times, most recently from cbeb1bd to b297962 Compare October 14, 2024 13:04
@nosan
Copy link
Contributor Author

nosan commented Oct 14, 2024

The third option
is similar to the first approach, but instead of supporting only .yaml files, it also
supports multi-document .properties.

@SpringJUnitConfig
@TestPropertySource(locations = { "classpath:test.yaml", "classpath:test1.yaml", "classpath:test.properties" },
		factory = PropertySourceLoaderPropertySourceFactory.class)
class PropertySourceLoaderPropertySourceFactoryIntegrationTests {

}

@philwebb
Copy link
Member

I like YamlPropertySourceFactory since it's quite obvious what it does. Once thing that makes me pause is that we don't/can't support spring.config.activate.on-profile for multi-document files. We also can't support spring.config.import. I'm not sure if this is really a problem, but folks using the annotation will need to understand the restrictions.

@philwebb philwebb added for: team-meeting An issue we'd like to discuss as a team to make progress status: on-hold We can't start working on this issue yet labels Oct 18, 2024
@nosan
Copy link
Contributor Author

nosan commented Oct 18, 2024

Once thing that makes me pause is that we don't/can't support spring.config.activate.on-profile for multi-document files.

I was trying to add this support, but to be honest, I have not found a way.

I'm not sure if this is really a problem

I think most people will use a dedicated YAML file to configure their test context, and I doubt anyone will try to use the spring.config.activate.on-profile feature

but folks using the annotation will need to understand the restrictions.

I completely agree, and it would be beneficial to include this information in the Javadoc and documentation.

@philwebb
Copy link
Member

Perhaps we should make it a hard requirement that only a single document is in the YAML and that no profile restrictions or imports are specified. We'll discuss this on a team call when can.

@philwebb
Copy link
Member

I'm wondering if basic YAML support should be something that's added to Spring Framework. Perhaps with some hook point so that we can warn if users accidentally point at YAML files that won't work. What do you think @sbrannen?

@philwebb philwebb removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Oct 22, 2024
@wilkinsona wilkinsona added the status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team label Oct 25, 2024
@nosan nosan force-pushed the 33434 branch 3 times, most recently from 96dd0a8 to aa2179a Compare November 19, 2024 10:40
… applied to a test class to add PropertySource loaded from YAML files to the Environment

This commit introduces new functionality to load properties
from YAML files in tests. The @TestYamlPropertySource annotation
is added to allow test classes to specify YAML files as property sources.

The @TestYamlPropertySource provides a convenient alternative for @TestPropertySource(factory=YamlPropertySourceFactory.class).

The YamlPropertySourceFactory class is also introduced to enable the loading of YAML files into the Environment through @TestPropertySource.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: on-hold We can't start working on this issue yet status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants