Test on Java 21 #1326
Merged
Test on Java 21 #1326
Conversation
This file contains 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
stIncMale
force-pushed
the
JAVA-5164
branch
2 times, most recently
from
25961ac
to
b03aadd
Compare
stIncMale
commented
Mar 4, 2024
Comment on lines
56
to
+64
| // If the Java 11+ extended socket options for keep alive probes are available, check those values. | ||
| if (Arrays.stream(ExtendedSocketOptions.getDeclaredFields()).anyMatch{ f -> f.getName().equals('TCP_KEEPCOUNT') }) { | ||
| Method getOptionMethod = Socket.getMethod('getOption', SocketOption) | ||
| getOptionMethod.invoke(socket, ExtendedSocketOptions.getDeclaredField('TCP_KEEPCOUNT').get(null)) == 9 | ||
| getOptionMethod.invoke(socket, ExtendedSocketOptions.getDeclaredField('TCP_KEEPIDLE').get(null)) == 120 | ||
| getOptionMethod.invoke(socket, ExtendedSocketOptions.getDeclaredField('TCP_KEEPINTERVAL').get(null)) == 10 | ||
| Method getOptionMethod | ||
| try { | ||
| getOptionMethod = Socket.getMethod('getOption', SocketOption) | ||
| } catch (NoSuchMethodException e) { | ||
| // ignore, the `Socket.getOption` method was added in Java SE 9 and does not exist in Java SE 8 | ||
| getOptionMethod = null | ||
| } |
There was a problem hiding this comment.
This code assumed that if ExtendedSocketOptions.TCP_KEEPCOUNT is available, then so is Socket.getOption. That's not actually the case:
ExtendedSocketOptions.TCP_KEEPCOUNT(not part of the Java SE) was added in Oracle JDK 81 (this is not part of the Java SE API, so it does not have to be in all JDKs): https://docs.oracle.com/javase/8/docs/jre/api/net/socketoptions/spec/jdk/net/ExtendedSocketOptions.html#TCP_KEEPCOUNTSocket.getOptionwas added in the Java SE 9.- So when I run
sdk use java 17.0.10-librca && ./gradlew -PjavaVersion=8 clean :driver-core:testlocally,ExtendedSocketOptions.TCP_KEEPCOUNTis present, whileSocket.getOptionis not, which leads to a failure. - Why is this not a problem in Evergreen? My guess is that the JDK 8 it uses does not have
ExtendedSocketOptions.TCP_KEEPCOUNT.
1 Currently, Oracle mistakenly documents it as added in JDK 11, see https://docs.oracle.com/en/java/javase/21/docs/api/jdk.net/jdk/net/ExtendedSocketOptions.html#TCP_KEEPCOUNT.
stIncMale
commented
Mar 4, 2024
| @@ -29,7 +29,7 @@ object TestMongoClientHelper { | |||
|
|
|||
| val mongoClientURI: String = { | |||
| val uri = Properties.propOrElse(MONGODB_URI_SYSTEM_PROPERTY_NAME, DEFAULT_URI) | |||
| if (!uri.isBlank) uri else DEFAULT_URI | |||
| if (!uri.codePoints().allMatch((cp: Int) => Character.isWhitespace(cp))) uri else DEFAULT_URI | |||
There was a problem hiding this comment.
String.isBlank was added in Java SE 11, so when I run sdk use java 17.0.10-librca && ./gradlew -PjavaVersion=8 clean :driver-scala:integrationTest locally, it fails.
The are two reasons this hasn't been a problem in Evergreen:
run-tests.shdid not run theintegrationTesttask (thetesttask does not depend onintegrationTest). Update: I still does not run theintegrationTesttask because I rolled the related change back: the tests weren't running correctly when run byrun-tests.sh, and on top of that @jyemin suggested to roll the change back.run-scala-tests.shdid not useJAVA_VERSION. So, despite this script runningintegrationTestimplicitly, it was using the same JDK Gradle is run by.
* Add JDK 21 to Gradle configuration * Add JDK 21 to the test matrix * Replace JDK 17 with JDK 21 for tasks that are ony run using latest JDK * Update dependencies (some now have to be conditional) * Update `run-scala-tests.sh` to use `JAVA_VERSION` * Run the `scala-tests` task with JDK 8, 17, 21 similarly to `kotlin-tests` * Update `run-tests.sh` to run the `integrationTest` task (some Scala tests, like `ClientSideEncryptionBypassAutoEncryptionSpec` were not run by it) * Fix test code that assumed Java SE 9+ JAVA-5164 --------- Co-authored-by: Jeff Yemin <jeff.yemin@mongodb.com>
jyemin
reviewed
Mar 4, 2024
JAVA-5164
jyemin
approved these changes
Mar 5, 2024
rozza
approved these changes
Mar 5, 2024
| @@ -2037,7 +2041,8 @@ buildvariants: | |||
| - name: "test" | |||
|
|
|||
| - matrix_name: "tests-jdk-secure" | |||
| matrix_spec: { auth: "auth", ssl: "ssl", jdk: [ "jdk8", "jdk17" ], version: [ "3.6", "4.0", "4.2", "4.4", "5.0", "6.0", "7.0", "latest" ], | |||
| matrix_spec: { auth: "auth", ssl: "ssl", jdk: [ "jdk8", "jdk17", "jdk21"], | |||
There was a problem hiding this comment.
As part of MongoDBs green sustainability drive we should review how much resources the matrix takes up and if we can keep coverage without duplicating testing. I'll open a ticket.
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR builds up on #1242.
run-scala-tests.shto useJAVA_VERSIONscala-teststask with JDK 8, 17, 21 similarly tokotlin-testsJAVA-5164