Skip to content

Add java25 checks - #1244

Open
loosebazooka wants to merge 1 commit into
mainfrom
add-java-25-stuff
Open

Add java25 checks#1244
loosebazooka wants to merge 1 commit into
mainfrom
add-java-25-stuff

Conversation

@loosebazooka

Copy link
Copy Markdown
Member

Summary

Release Note

Documentation

vlsi
vlsi previously approved these changes Jul 27, 2026
import org.junit.jupiter.api.extension.RegisterExtension;
import org.slf4j.event.Level;

@DisabledForJreRange(max = JRE.JAVA_16)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It it the same as @EnabledForJreRange(min = JRE.JAVA_17)?
Frankly, I find enabled easier to read.

Suggested change
@DisabledForJreRange(max = JRE.JAVA_16)
@EnabledForJreRange(min = JRE.JAVA_17)

Comment thread sigstore-java/build.gradle.kts Outdated
Comment on lines +13 to +27
// 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,
)
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look odd. Why do we need explicit attributes?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@loosebazooka loosebazooka Jul 27, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I may be misunderstanding what's going on though.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh right our build system is jdk21 targetting java11 bytecode, so it doesn't appear to fail on the java 17 reference.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
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 this pull request may close these issues.

2 participants