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 support for composed annotations to all annotations #696

Closed
eeverman opened this issue Nov 28, 2022 · 2 comments · Fixed by #698
Closed

Add support for composed annotations to all annotations #696

eeverman opened this issue Nov 28, 2022 · 2 comments · Fixed by #698

Comments

@eeverman
Copy link
Contributor

Looking through the source code of the annotations used to register extensions, it looks like none of them (or none that I could find) support composed annotations, that is, reusing the annotation in another annotation.

JUnit's user manual has a few examples of this in the declarative extension registration section. It would allow users who repeatedly use several extensions together to bundle those into a single annotation like this:

@Retention(RetentionPolicy.RUNTIME)
@Target({ TYPE, METHOD, ANNOTATION_TYPE })
@Stopwatch  @DefaultLocal("Local.UK") @SetSystemProperty(key="BROILER" value="ON")
public @interface StandardTestSetup { }

To enable that, two things would be needed:

  1. The ANNOTATION_TYPE would need to be added to the @Target of the annotations
  2. The PioneerAnnotationUtils would need to be able to find them.

Currently the PioneerAnnotationUtils class only looks for the specified annotation - It doesn't check to see what meta-annotations an annotation might have. I think that happens (or doesn't happen) right here.

@eeverman eeverman changed the title Add support for composed annotation to all annotations Add support for composed annotations to all annotations Nov 28, 2022
@Michael1993
Copy link
Member

Hi @eeverman! Thank you for opening the issue.

I've looked through our annotations and it seems we do have some of them that do not support composed annotations (namely @StdIo and @ReportEntry). All our other annotations types either have ANNOTATION_TYPE or TYPE in their @Target.

I also looked at PioneerAnnotationUtils and I can kind of see your issue? The line you are pointing out is a naïve approach and if we are trying to find only the first meta-annotation it wouldn't work. I would like to point out that the method would still work (but could take a bit longer), I'm only referring to the approach taken in the specific line you mentioned.

@eeverman
Copy link
Contributor Author

Nice fix - Thank you!

I had been looking at StdIo when I reported the issue.... and I guess forgot that TYPE included the annotation type. I also had a test that was trying to verify that composed annotation worked, but I see now that it included some other assumptions.

Thanks again!

nipafx pushed a commit that referenced this issue Nov 30, 2022
This commit fixes the annotations @ReportEntry and @stdio missing
the ANNOTATION_TYPE or TYPE from their @target. They can now be used
on composed annotations.

Closes: #696
PR: #698
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants