Add java25 checks - #1244
Conversation
797036e to
3fe4fac
Compare
| import org.junit.jupiter.api.extension.RegisterExtension; | ||
| import org.slf4j.event.Level; | ||
|
|
||
| @DisabledForJreRange(max = JRE.JAVA_16) |
There was a problem hiding this comment.
It it the same as @EnabledForJreRange(min = JRE.JAVA_17)?
Frankly, I find enabled easier to read.
| @DisabledForJreRange(max = JRE.JAVA_16) | |
| @EnabledForJreRange(min = JRE.JAVA_17) |
| // allow java17 test dependencies | ||
| configurations.testCompileClasspath.configure { | ||
| attributes.attribute( | ||
| org.gradle.api.attributes.Attribute | ||
| .of("org.gradle.jvm.version", Int::class.javaObjectType), | ||
| 17, | ||
| ) | ||
| } | ||
| configurations.testRuntimeClasspath.configure { | ||
| attributes.attribute( | ||
| org.gradle.api.attributes.Attribute | ||
| .of("org.gradle.jvm.version", Int::class.javaObjectType), | ||
| 17, | ||
| ) | ||
| } |
There was a problem hiding this comment.
These changes look odd. Why do we need explicit attributes?
There was a problem hiding this comment.
There's a few options here. This just allowed the mockoidcclient (min java17) to be allowed on the classpath even if the testjdk was java11.
I think it can be cleaned up a little syntax wise. The alternatives are to isolate out those tests in other ways, or to just move our minimum to java17.
There was a problem hiding this comment.
We can't pretend test sources to be "compatible wit java 11" while Mockito requires 17, can we?
If Mockito requires Java 17, we will might get compilation or runtime exceptions any time soon..
If we use Mockito in a few tests only, we might consider moving them to a separate module and/or sourceset (so it has its own compilation unit with its own target release=17).
There was a problem hiding this comment.
I think this is the trick for it. We're telling gradle to just allow it, and it seems to fine doing so. If the classes were ever loaded it would fail, but they're gated by the junit annotations.
There was a problem hiding this comment.
I think moving to java17 as the minimum might be fine, we can remove java11 from our standard tests and just run it all through conformance testing.
There was a problem hiding this comment.
I'm afraid compilation might fail. The code should be compiled first, and the compiler needs to parse the bytecode. It shouldn't be able to recognize Java 17 bytecode if it targets Java 11.
There was a problem hiding this comment.
I mean it's not failing right now -- but I can rework this to just be java17. I would also like to explore updating to junit6 (in another PR)
There was a problem hiding this comment.
I think I may be misunderstanding what's going on though.
There was a problem hiding this comment.
oh right our build system is jdk21 targetting java11 bytecode, so it doesn't appear to fail on the java 17 reference.
There was a problem hiding this comment.
that being said, it sounds like this is confusing and weird. So I'll try to avoid it.
Upgrade mock-oauth2-server to 5.0.2 and skip tests on JDK16 and below using JUnit 5 annotations for mock-oauth-server Signed-off-by: Appu <appu@google.com>
3fe4fac to
d505f6c
Compare
Summary
Release Note
Documentation