Allow parsing the value of java.version sysprop #44017
Conversation
We often start testing with early access versions of new Java versions and this have caused minor issues in our tests (i.e. elastic#43141) because the version string that the JVM reports cannot be parsed as it ends with the string `-ea`. This commit changes how we parse and compare Java versions to allow correct parsing of early access versions. Early access versions of the same exact Java version are considered, well, earlier when comparing. It also changes a number of tests that would attempt to parse `java.specification.version` in order to get the full version of Java. `java.specification.version` only contains the major version and is thus inappropriate when trying to compare against a version that might contain a minor, patch or an early access part. We know parse `java.version` that can be consistently parsed.
jkakavas
added
>non-issue
:Core/Infra/Core
|
Pinging @elastic/es-core-infra |
|
@elasticmachine update branch |
|
Thanks for looking into this @jkakavas! While I understand the need to handle The difference here with what you have implemented is we shouldn't try to discern any meaning out of
So, I think we can get the same behavior you are after, but without having any "-ea" special string or comparison. |
|
Got it , makes sense @rjernst. I didn't meant to attach any special meaning to the early access flag, |
This commit changes the behavior to enable more generic parsing of the value of java.version system property as this is described in https://openjdk.java.net/jeps/223. In short it handles a version number part, like before, but additionally a PRE part that matches ([a-zA-Z0-9]+).
|
Changed this to be able to handle the value of |
jkakavas
changed the title
|
No rush, just pinging for visibility @rjernst. |
| numericComponents = parts[0].split("\\."); | ||
| prePart = parts[1]; | ||
| } else { | ||
| throw new IllegalArgumentException("value"); |
There was a problem hiding this comment.
We should have a more descriptive failure message, which includes the actual value, not the literal string value. ;)
|
Both ci/1 and ci/2 have completed successfully but the notification failed to reach ci/1: https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+pull-request-1/3323/console |
because of the service hiccups.We often start testing with early access versions of new Java versions and this have caused minor issues in our tests (i.e. #43141) because the version string that the JVM reports cannot be parsed as it ends with the string -ea. This commit changes how we parse and compare Java versions to allow correct parsing and comparison of the output of java.version system property that might include an additional alphanumeric part after the version numbers (see [JEP 223[(https://openjdk.java.net/jeps/223)). In short it handles a version number part, like before, but additionally a PRE part that matches ([a-zA-Z0-9]+). It also changes a number of tests that would attempt to parse java.specification.version in order to get the full version of Java. java.specification.version only contains the major version and is thus inappropriate when trying to compare against a version that might contain a minor, patch or an early access part. We know parse java.version that can be consistently parsed. Resolves #43141
We often start testing with early access versions of new Java versions and this have caused minor issues in our tests (i.e. #43141) because the version string that the JVM reports cannot be parsed as it ends with the string -ea. This commit changes how we parse and compare Java versions to allow correct parsing and comparison of the output of java.version system property that might include an additional alphanumeric part after the version numbers (see [JEP 223[(https://openjdk.java.net/jeps/223)). In short it handles a version number part, like before, but additionally a PRE part that matches ([a-zA-Z0-9]+). It also changes a number of tests that would attempt to parse java.specification.version in order to get the full version of Java. java.specification.version only contains the major version and is thus inappropriate when trying to compare against a version that might contain a minor, patch or an early access part. We know parse java.version that can be consistently parsed. Resolves #43141
We often start testing with early access versions of new Java
versions and this have caused minor issues in our tests
(i.e. #43141) because the version string that the JVM reports
cannot be parsed as it ends with the string
-ea.This commit changes how we parse and compare Java versions to
allow correct parsing and comparison of the output of
java.versionsystem property that might include an additional alphanumeric
part after the version numbers (see [JEP 223[(https://openjdk.java.net/jeps/223)).
It also changes a number of tests that would attempt to parse
java.specification.versionin order to get the full versionof Java.
java.specification.versiononly contains the majorversion and is thus inappropriate when trying to compare against
a version that might contain a minor, patch or an early access
part. We know parse
java.versionthat can be consistentlyparsed.
Resolves #43141