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

@TestPropertySource not recognizing yaml files #33434

Open
DataWorm opened this issue Dec 1, 2022 · 9 comments
Open

@TestPropertySource not recognizing yaml files #33434

DataWorm opened this issue Dec 1, 2022 · 9 comments
Labels
type: enhancement A general enhancement
Milestone

Comments

@DataWorm
Copy link

DataWorm commented Dec 1, 2022

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?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 1, 2022
@philwebb
Copy link
Member

philwebb commented Dec 2, 2022

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?

@philwebb philwebb added for: team-meeting An issue we'd like to discuss as a team to make progress status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Dec 2, 2022
@philwebb philwebb removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Dec 14, 2022
@sbrannen
Copy link
Member

sbrannen commented Aug 2, 2023

@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
Copy link
Member

sbrannen commented Aug 2, 2023

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
Copy link
Member

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
Copy link
Member

sbrannen commented Aug 2, 2023

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.

  • org.springframework.core.io.support.PropertySourceDescriptor

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
Copy link
Member

sbrannen commented Aug 2, 2023

@wilkinsona
Copy link
Member

Thanks, @sbrannen.

Closing in favour of the Framework issue.

@wilkinsona wilkinsona closed this as not planned Won't fix, can't repro, duplicate, stale Aug 2, 2023
@wilkinsona wilkinsona added for: external-project For an external project and not something we can fix and removed status: waiting-for-triage An issue we've not yet triaged status: waiting-for-internal-feedback An issue that needs input from a member or another Spring Team labels Aug 2, 2023
@sbrannen
Copy link
Member

sbrannen commented Aug 4, 2023

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

spring-projects/spring-framework@04cce0b

@philwebb
Copy link
Member

philwebb commented Nov 7, 2023

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

@philwebb philwebb reopened this Nov 7, 2023
@philwebb philwebb added type: enhancement A general enhancement and removed for: external-project For an external project and not something we can fix labels Nov 7, 2023
@philwebb philwebb added this to the 3.x milestone Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants