-
Notifications
You must be signed in to change notification settings - Fork 830
Add test matrix for different java versions #1781
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
129b3d0 to
3f2e73b
Compare
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
dd5fe55 to
16d922a
Compare
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
|
@jaydeluca I think we don't need to build the release for every version - it has to tests anyways |
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
zeitlinger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great!
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| java: [17, 21, 25] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not 8, 11?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i couldnt get them to work, can't remember why, let me try again to refresh my memory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
theres a bunch of test code that uses switch statements and """...""" text blocks. I can refactor them all if we think it's worth it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also, JUnit Jupiter 6.0.2 requires Java 17+ to run
|
|
||
| mise run set-version "$VERSION" | ||
| mvn -B package -P 'release,!default' -Dmaven.test.skip=true | ||
| mvn -B package -P 'release,!default,!examples-and-integration-tests' -Dmaven.test.skip=true -Dgpg.skip=true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding the profiles has added some issues to the way some of these other tasks resolve everything.
from:
https://github.com/prometheus/client_java/actions/runs/21207964631/job/61008982563?pr=1781
Error: Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.14.1:testCompile (default-testCompile) on project prometheus-metrics-exposition-formats: Compilation failure: Compilation failure:
Error: /home/runner/work/client_java/client_java/prometheus-metrics-exposition-formats-shaded/target/metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/ProtobufExpositionFormatsTest.java:[11,45] cannot find symbol
Error: symbol: class ExpositionFormatsTest
Error: /home/runner/work/client_java/client_java/prometheus-metrics-exposition-formats-shaded/target/metrics-exposition-formats/src/test/java/io/prometheus/metrics/expositionformats/ProtobufExpositionFormatsTest.java:[13,3] method does not override or implement a method from a supertype
the shaded module's tests need a test-jar dependency that's explicitly excluded from the release profile, so thats why i added the !examples-and-integration-tests to disable them (i assumed that would be ok to skip for testing the release build)
I was also encountering gpg issues both in CI and locally, not sure if this is a "me" thing, but didn't think it was necessary for this test step
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a test matrix to enable testing across different Java versions (17, 21, and 25) while maintaining Java 8 compatibility for the main codebase. The changes restructure Maven profiles and dependencies to support building and testing on older JDK versions.
Changes:
- Added new GitHub Actions workflow for multi-version Java compatibility testing (Java 17, 21, 25)
- Restructured Maven configuration: moved test dependencies out of profile to global scope, renamed
defaultprofile toexamples-and-integration-tests(activated only for Java 25+), and created newerrorproneprofile (activated for Java 21+) - Added reflection-based compatibility layer in HTTPServerTest for Subject API differences between Java 17 and Java 18+
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/multi-version-test.yml | New workflow for testing on Java 17, 21, and 25 |
| pom.xml | Restructured profiles and dependencies for multi-version support; added test.java.version property |
| prometheus-metrics-parent/pom.xml | Moved spotless plugin to java17-plus profile |
| integration-tests/it-spring-boot-smoke-test/pom.xml | Moved spotless and native-maven-plugin to java17-plus profile |
| benchmarks/pom.xml | Added compiler args override to prevent ErrorProne inheritance |
| prometheus-metrics-exporter-httpserver/src/test/java/io/prometheus/metrics/exporter/httpserver/HTTPServerTest.java | Added reflection-based compatibility for Subject API across Java versions |
| .mise/tasks/build-release.sh | Updated profile references to match renamed profile |
| .editorconfig | Increased max line length for pom.xml; added configuration for build-release.sh |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
12f9b51 to
efc9c9f
Compare
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
Signed-off-by: Jay DeLuca <jaydeluca4@gmail.com>
No description provided.