This repository was archived by the owner on Apr 7, 2026. It is now read-only.
test: simplifying Java 8 test via jvm property#2440
Merged
suztomo merged 9 commits intogoogleapis:mainfrom May 19, 2023
Merged
Conversation
|
Warning: This pull request is touching the following templated files:
|
product-auto-label
Bot
added
the
api: spanner
suztomo
added
the
owlbot:run
gcf-owl-bot
Bot
removed
the
owlbot:run
suztomo
commented
May 19, 2023
Comment on lines
+188
to
+189
| // This is dummy statement to touch the Version class that is compiled for Java 11 | ||
| System.out.println("org.graalvm.home.Version: " + org.graalvm.home.Version.parse("1.2.3")); |
Member
Author
There was a problem hiding this comment.
Test to confirm the Java 8 test fails with Java 8-incompatible class.
Member
Author
There was a problem hiding this comment.
The test successfully detected the incompatible class file in Java 8:
Error: com.google.cloud.spanner.RetryOnInvalidatedSessionTest Time elapsed: 0 s <<< ERROR!
java.lang.UnsupportedClassVersionError: org/graalvm/home/Version has been compiled by a more recent version of the Java Runtime (class file version 55.0), this version of the Java Runtime only recognizes class file versions up to 52.0
This reverts commit 7752fa4.
suztomo
added
the
owlbot:run
gcf-owl-bot
Bot
removed
the
owlbot:run
Member
Author
|
@rajatbhatta Would you review this? |
rajatbhatta
approved these changes
May 19, 2023
Contributor
rajatbhatta
left a comment
rajatbhatta
left a comment
There was a problem hiding this comment.
LGTM, apart from one comment.
| requiresCodeOwnerReviews: true | ||
| requiresStrictStatusChecks: false | ||
| requiredStatusCheckContexts: | ||
| - dependencies (8) |
Contributor
There was a problem hiding this comment.
Shall we do this for other patterns too? 4.0.x and 5.2.x.
Contributor
Also, looks like the dependencies github checks are stuck. |
suztomo
commented
May 19, 2023
Member
Author
suztomo
left a comment
suztomo
left a comment
There was a problem hiding this comment.
I'll remove the dependencies checks from required checks. We only need one.
| requiredStatusCheckContexts: | ||
| - dependencies (8) | ||
| - dependencies (11) | ||
| - dependencies (17) |
Member
Author
There was a problem hiding this comment.
Background: we don't need to repeatedly check the dependencies via different JDKs.
Comment on lines
-86
to
-94
| # For Java 8 tests, use JDK 11 to compile | ||
| - if: ${{matrix.java}} == '8' | ||
| uses: actions/setup-java@v3 | ||
| with: | ||
| java-version: 11 | ||
| distribution: zulu | ||
| - if: ${{matrix.java}} == '8' | ||
| run: echo "JAVA11_HOME=${JAVA_HOME}" >> $GITHUB_ENV | ||
| shell: bash |
Member
Author
There was a problem hiding this comment.
This removal is the simplification
Comment on lines
-54
to
-55
| - run: echo "JAVA11_HOME=${JAVA_HOME}" >> $GITHUB_ENV | ||
| shell: bash |
Member
Author
There was a problem hiding this comment.
This removal is the simplification
| requiresCodeOwnerReviews: true | ||
| requiresStrictStatusChecks: false | ||
| requiredStatusCheckContexts: | ||
| - dependencies (8) |
suztomo
added
the
owlbot:run
gcf-owl-bot
Bot
removed
the
owlbot:run
suztomo
added
automerge
gcf-owl-bot
Bot
removed
the
owlbot:run
gcf-merge-on-green
Bot
removed
the
automerge
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I found we can simplify the Java 8 test via surefire's jvm property (document: https://maven.apache.org/surefire/maven-surefire-plugin/test-mojo.html#jvm)
Changing the required checks to use "dependencies (17)" only. There's no point running the dependencies checks for different JDK versions.
Once approved, I will change the required checks in repository settings to merge this pull request.