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

Investigate troubles caused by Spock integration #72

Closed
phejl opened this issue Jul 28, 2021 · 5 comments · Fixed by #73
Closed

Investigate troubles caused by Spock integration #72

phejl opened this issue Jul 28, 2021 · 5 comments · Fixed by #73

Comments

@phejl
Copy link
Contributor

phejl commented Jul 28, 2021

Reported here junit-team/junit5@eaab286. I need to check in more depth but perhaps one nice idea would be to have the support not dependant on the JfrEventTest annotation (global spock extension). If I understand that correctly Quarkus support works like that. It would make usage even simpler.

cc @gunnarmorling @sbrannen

@sbrannen
Copy link
Contributor

FYI: The reason the JUnit 5 build failed is due to a "feature" of the Java compiler.

Many people (including me) consider this an outright bug in the Java compiler (because types are allowed to be annotated with annotation types that are not present in the current classpath at runtime), but the Java team at Oracle apparently do not see it that way (at least not when we discussed it with them at Devoxx back in 2017 (I believe that was the year)).

In any case, if you would like some more background information on the issue, see the following.

@sbrannen
Copy link
Contributor

Please keep in mind that having @JfrEventTest meta-annotated with @ExtendWith(JfrEventTestExtension.class) will result in the same error encountered in the JUnit build for projects that use JfrJunit with Spock but without JUnit Jupiter on their classpath.

@gunnarmorling
Copy link
Member

types are allowed to be annotated with annotation types that are not present in the current classpath at runtime

Just cannot not comment on this one :) I think the current behavior is desirable, as it allows to re-use annotated classes in different contexts. The canonical example being JPA entities which can be used as data structures on the client-side, without the need for having the persistence API on the classpath there.

@sbrannen
Copy link
Contributor

sbrannen commented Jul 28, 2021

@gunnarmorling, I think you misunderstood the point I was trying to make (likely because I was too vague).

I think the current behavior is desirable, as it allows to re-use annotated classes in different contexts. The canonical example being JPA entities which can be used as data structures on the client-side, without the need for having the persistence API on the classpath there.

Yes, that's a great feature of Java. No doubt. We love it!

But... the Java compiler emits a warning if you try to use a third-party annotation in your own code when that third-party annotation is meta-annotated with a second third-party annotation that is not present in the class path.

And... if you compile with -Werror (which many projects do), your build fails.

We argued with the Java team that such a condition should not result in a warning that can fail the build, because of the fact that the exact same code (once compiled) works fine at runtime without the second third-party annotation on the classpath.

Does that make sense now?


Hmmm... maybe I should say fourth-party instead of "second third-party". 🤔

@gunnarmorling
Copy link
Member

Ah yes, got it now. Indeed I understood exactly the opposite of what you meant to say :) So yes, I agree there should be a way for compiling the code of this situation without any warnings (and without supressing other, unrelated, warnings), if that code can be run just fine later on.

phejl pushed a commit that referenced this issue Jul 28, 2021
…entTest and makes usage simple at the same time
gunnarmorling pushed a commit that referenced this issue Jul 29, 2021
…entTest and makes usage simple at the same time



Co-authored-by: Petr Hejl <petr.hejl@memsource.com>
Co-authored-by: Sam Brannen <sbrannen@vmware.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants