Skip to content

Introduce getMergedAnnotation() in TestContextAnnotationUtils #34456

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Introduce getMergedAnnotation() in TestContextAnnotationUtils #34456

wants to merge 1 commit into from

Conversation

quaff
Copy link
Contributor

@quaff quaff commented Feb 20, 2025

It's equivalent to findMergedAnnotation() but return MergedAnnotation instead of synthesized annotation.

@quaff
Introduce method TestContextAnnotationUtils.getMergedAnnotation()
e34a5c7 
It's equivalent to findMergedAnnotation() but return MergedAnnotation instead of synthesized annotation.

Signed-off-by: Yanming Zhou <[email protected]>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 20, 2025
@quaff
Copy link
Contributor Author

quaff commented Feb 20, 2025

@sbrannen See the actual use case: spring-projects/spring-boot#44354 (comment)

@quaff quaff mentioned this pull request Feb 20, 2025
Open
@sbrannen sbrannen self-assigned this Feb 27, 2025
@sbrannen sbrannen added the in: test Issues in the test module label Feb 27, 2025
@sbrannen sbrannen changed the title Introduce method TestContextAnnotationUtils.getMergedAnnotation() Introduce getMergedAnnotation() in TestContextAnnotationUtils Feb 27, 2025
@sbrannen sbrannen modified the milestone: 7.0.0-M3 Feb 27, 2025
@sbrannen
Copy link
Member

sbrannen commented Mar 1, 2025

Hi @quaff,

After reviewing the proposed changes in this PR and the comments in spring-projects/spring-boot#44354 (comment), I am declining this for the following reasons.

  • as mentioned in that comment, that new (not-yet-approved) Boot feature would currently be the only client.
  • TestContextAnnotationUtils::searchEnclosingClass is intentionally public for exactly such custom use cases.
  • a getMergedAnnotation method that returns an instance of MergedAnnotation conflicts with the other methods in TestContextAnnotationUtils which return a synthesized Annotation.
  • The name getMergedAnnotation conflicts with the get/find semantics used in TestContextAnnotationUtils and defined in the class-level Javadoc for AnnotatedElementUtils.

Thanks for the PR in any case!

Sam

@sbrannen sbrannen closed this Mar 1, 2025
@sbrannen sbrannen added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@sbrannen sbrannen

Labels
in: test Issues in the test module status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@quaff @sbrannen @spring-projects-issues