Hi,
WRT to testing - one thing that could be done (possibly in a
followup patch) would be:
class VersionProps {
...
static List<Integer> parseVersionNumbers(String versionNumber) {
// parsing code goes there
}
static List<Integer> versionNumbers() {
return parseVersionNumbers(VERSION_NUMBER);
}
...
}
that would allow writing/adding a unit test for the algorithm
implemented in parseVersionNumbers (either using white-box
with -Xpatch or using reflection and the proper
incantations to invoke parseVersionNumbers from outside the
package).
cheers,
-- daniel
On 28/06/16 15:57, Volker Simonis wrote:
Hi,
can somebody please review this trivial fix:
http://cr.openjdk.java.net/~simonis/webrevs/2016/8160457/
https://bugs.openjdk.java.net/browse/JDK-8160457
The problem is that VersionProps.versionNumbers() incorrectly parses a
Java version string because it doesn't skip the separating dots. The
current version only works for one digit versions like "9" but will
fail for any longer version string like for example "9.0.0.1" with:
at
java.lang.NumberFormatException.forInputString(java.base@9.0.0.1-internal/NumberFormatException.java:65)
at
java.lang.Integer.parseInt(java.base@9.0.0.1-internal/Integer.java:791)
at
java.lang.VersionProps.versionNumbers(java.base@9.0.0.1-internal/VersionProps.java:76)
at
java.lang.Runtime.version(java.base@9.0.0.1-internal/Runtime.java:940)
This also breaks the build which uses a newly built jdk for
bootstrapping if we set '--with-version-patch=1' for example.
An can you PLEASE, PLEASE finally do your internal/early access builds
with '--with-version-patch=1'. It seems really careless to me that you
introduce a new, up to four digit versioning schema but only test the
shortcut version with one digit. I wouldn't be surprised if a version
like "9.0.0.1" breaks more Java applications than the sun.misc.Unsafe
removal :)
Thank you and best regards,
Volker
