spring-projects / spring-boot

Spring Boot helps you to create Spring-powered, production-grade applications and services with absolute minimum fuss.
https://spring.io/projects/spring-boot
Apache License 2.0
75.25k stars 40.7k forks source link

@TestPropertySource not recognizing yaml files #33434

Open DataWorm opened 1 year ago

DataWorm commented 1 year ago

Spring Boot supports not only application.properties but also application.yml files for long time now and we are using it in all our projects for better readability. But when it comes to testing with annotations like @TestPropertySource then yaml files are not supported for whatever reason. If you try to specify a yml file as source, it does not complain but the test also won't be able to resolve the properties unless you are using the properties notation format and just store it with the .yml file extension.

I wonder why this is not supported yet. Is there another annotation I haven't seen yet or is there any plan to let TestPropertySource automatically recognize the file extension and make use of the corresponding yaml file loader?

philwebb commented 1 year ago

It's currently not supported because @TestPropertySource is a Spring Framework annotation and isn't aware of YAML. We've had a request to support it previously (#11921) which resulted in Framework issue that was ultimately closed. I wonder if we should try again for Framework 6.1.

@sbrannen any thought on this? Perhaps we could have a hook point to allow @TestPropertySource to deal with different file types?

sbrannen commented 1 year ago

@sbrannen any thought on this? Perhaps we could have a hook point to allow @TestPropertySource to deal with different file types?

Yes, I'm thinking about introducing a new Class<? extends PropertySourceFactory> factory() default PropertySourceFactory.class; attribute in @TestPropertySource analogous to the one already present in @PropertySource since Spring Framework 4.3. Though, I'll need to investigate the feasibility of such a change.

@philwebb and @wilkinsona, what do you think about that?

sbrannen commented 1 year ago

Though, I'll need to investigate the feasibility of such a change.

At a first glance, the biggest challenge I foresee with supporting a custom factory per @TestPropertySource annotation is that MergedContextConfiguration.getPropertySourceLocations() contains all merged locations from all such repeatable annotations, meaning there would not be any easy way to determine which factory to use for which files.

One option would be to use the first (or last) custom configured factory to process all locations -- which would require that all locations configured via @TestPropertySource on a given test class be the same type of file format.

Another option would be to create some sort of map from file extension to factory -- though I'm not sure how we would want to support that mapping via annotation-based configuration.

Perhaps instead of an external mapping, we could introduce the following default method in the PropertySourceFactory SPI and allow factory implementations to check the supported file extensions themselves (and fall back to using DefaultPropertySourceFactory).

default boolean supportsResource(@Nullable String name, EncodedResource resource) {
    return true;
}

Thoughts?

wilkinsona commented 1 year ago

I like the idea of aligning with @PropertySource but hadn't considered the complications introduced by MergedContextConfiguration.

I wonder if it would be possible to introduce a new method on MergedContextConfiguration that returns something richer than a String[]. That richer type would provide both the locations and the factory from @TestPropertySource. That feels a little cleaner to me than changing PropertySourceFactory in spring-core to accommodate a requirement that appears to be specific to spring-test.

sbrannen commented 1 year ago

I wonder if it would be possible to introduce a new method on MergedContextConfiguration that returns something richer than a String[].

I was also pondering introducing a new method in MCC.

That richer type would provide both the locations and the factory from @TestPropertySource.

Well, it turns out that we already have such a record as of Framework 6.0.

So we could return a list of descriptors.

That feels a little cleaner to me than changing PropertySourceFactory in spring-core to accommodate a requirement that appears to be specific to spring-test.

Indeed.

sbrannen commented 1 year ago
wilkinsona commented 1 year ago

Thanks, @sbrannen.

Closing in favour of the Framework issue.

sbrannen commented 1 year ago

FYI: the feature is already available in Spring Framework 6.1 snapshots in case anyone wants to try it out.

https://github.com/spring-projects/spring-framework/commit/04cce0bafd21c3f6b8f427ab2eac75ffadefb585

philwebb commented 1 year ago

Reopening to see if we can provide a YAML PropertySourceFactory or perhaps an adapter to our own PropertySourceLoader interface.

lfvJonas commented 2 months ago

Any news on this issue?