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

@ServiceConnection is not found when used in an interface implemented by a test class #37671

Closed
therealaleko opened this issue Oct 4, 2023 · 11 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@therealaleko
Copy link

Using the new @ServiceConnection does not work as I would expect when applied within an interface. It starts the container on a random port rather than my spring defined port.

@DynamicPropertySource does work as I expect.

public interface PostgresContainer {
  @Container
  //  @ServiceConnection
  PostgreSQLContainer<?> postgreSQLContainer = new PostgreSQLContainer<>("postgres:11.1");

  @DynamicPropertySource
  static void initialize(DynamicPropertyRegistry registry) {
    registry.add("spring.datasource.url", postgreSQLContainer::getJdbcUrl);
    registry.add("spring.datasource.username", postgreSQLContainer::getUsername);
    registry.add("spring.datasource.password", postgreSQLContainer::getPassword);
  }
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Oct 4, 2023
@eddumelendez
Copy link
Contributor

eddumelendez commented Oct 4, 2023

It starts the container on a random port rather than my spring defined port.

Can you elaborate more? I think I didn't get it.

Take into account Testcontainers starts containers in random ports and @ServiceConnection maps the properties from the container to spring boot

@philwebb
Copy link
Member

philwebb commented Oct 4, 2023

Thanks @eddumelendez, I was just writing the same thing!

@therealaleko perhaps you could provide a sample application that shows the problem?

@philwebb philwebb added the status: waiting-for-feedback We need additional information before we can continue label Oct 4, 2023
@therealaleko
Copy link
Author

therealaleko commented Oct 4, 2023

Please check the repo here. Thanks!
https://github.com/therealaleko/spring_boot_issue_37671

@philwebb @eddumelendez

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Oct 4, 2023
@eddumelendez
Copy link
Contributor

eddumelendez commented Oct 4, 2023

Thanks! So, the ServiceConnection doesn't work when using interfaces

I think this line here

should change to MergedAnnotations annotations = MergedAnnotations.from(field, SearchStrategy.TYPE_HIERARCHY);

@philwebb philwebb added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Oct 4, 2023
@philwebb philwebb added this to the 3.1.x milestone Oct 4, 2023
@wilkinsona wilkinsona changed the title @ServiceConnection does not work properly for testcontainers when used inside an interface. @DynamicPropertySource works. @ServiceConnection is not found when used in an interface Oct 4, 2023
@scottfrederick scottfrederick self-assigned this Oct 9, 2023
@scottfrederick scottfrederick changed the title @ServiceConnection is not found when used in an interface @ServiceConnection is not found when used in an interface implemented by a test class Oct 9, 2023
@scottfrederick scottfrederick modified the milestones: 3.1.x, 3.1.5 Oct 9, 2023
@andreasevers
Copy link

Would it also be possible to add support for inherited interfaces? At the moment the test class has to implement the interface directly, but ideally extending a superclass that implements the interface should work too.
I could submit a PR if this is something that could be accepted.

philwebb added a commit that referenced this issue Oct 15, 2023
Refine original fix to also search interfaces on the superclass.

Fixes gh-37671
@philwebb
Copy link
Member

Good spot @andreasevers, I've refined the search algorithm in e01e4f1

@andreasevers
Copy link

Wow, thanks Phil! This is great, using it right away!

@andreasevers
Copy link

Could this latest commit also get backported into 3.1.x perhaps?

wilkinsona pushed a commit that referenced this issue Oct 19, 2023
Refine original fix to also search interfaces on the superclass.

Fixes gh-37671
@wilkinsona
Copy link
Member

Good catch, @andreasevers. Thank you. I've done that in fcb75b6.

@andreasevers
Copy link

Much appreciated 🙏

@therealaleko
Copy link
Author

Thank you!

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

No branches or pull requests

7 participants