Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

On Thu, Jul 14, 2016 at 4:04 PM, Mandy Chung wrote: > >> On Jul 12, 2016, at 9:54 PM, Volker Simonis wrote: >> >> Please find the new webrev at: >> >> http://cr.openjdk.java.net/~simonis/webrevs/2016/8160564.v2/ >> > > Looks good. > > Nit: maybe

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

Thanks Iris! On Tue, Jul 12, 2016 at 9:01 PM, Iris Clark wrote: > Hi. > >>> Please find the new webrev at: >>> >>> http://cr.openjdk.java.net/~simonis/webrevs/2016/8160564.v2/ >> >> +1 >> >>> Any other comments? > >> Only to note that this adds a validation check that we

RE: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

Hi. >> Please find the new webrev at: >> >> http://cr.openjdk.java.net/~simonis/webrevs/2016/8160564.v2/ > > +1 > >> Any other comments? > Only to note that this adds a validation check that we don't have > trailing zeros, which I was recently made aware of is being > reconsidered, see

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

On 07/12/2016 03:54 PM, Volker Simonis wrote: Please find the new webrev at: http://cr.openjdk.java.net/~simonis/webrevs/2016/8160564.v2/ +1 Any other comments? Only to note that this adds a validation check that we don't have trailing zeros, which I was recently made aware of is being

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

On 2016-07-11 18:18, Volker Simonis wrote: Hi, here comes a new version of this change. I've tried to address most of the concerns/suggestions: http://cr.openjdk.java.net/~simonis/webrevs/2016/8160564.v1/ Looks good. As I'm currently obsessing about startup performance, I did wish we

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

> On Jul 12, 2016, at 12:18 AM, Volker Simonis wrote: > > Hi, > > here comes a new version of this change. I've tried to address most of > the concerns/suggestions: > > http://cr.openjdk.java.net/~simonis/webrevs/2016/8160564.v1/ > This version looks okay in

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

Hi, here comes a new version of this change. I've tried to address most of the concerns/suggestions: http://cr.openjdk.java.net/~simonis/webrevs/2016/8160564.v1/ 1. Added a private 'check()' method to the VersionProps class which ensures that every single part of a version number starts with a

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

On Jul 7, 2016, at 11:05 AM, Martin Buchholz wrote: > private static final jdk.internal.misc.Unsafe > java.util.concurrent.ConcurrentHashMap.U Unless the security manager is turned on, you can do setAcc to pick up all sorts of private fields. As Alan points out, it would

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

Hi Volker, Thanks for adding a new test for it. Nit: can you wrap the long lines. As for the bad version, one possible change is to add assert Runtime.Version.parse(versionNumber) in parseVersionNumbers method and add -esa in @run tag. It’d be good to convert this to testng test where the

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

On 07/07/2016 19:05, Martin Buchholz wrote: When jdk9 is released, an army of white, black, grey, and red hats will try to keep their old Unsafe hacks alive and maybe get their hands on a jdk.internal.misc.Unsafe. I assume these Unsafe usages are sun.misc.Unsafe so they should continue to

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

When jdk9 is released, an army of white, black, grey, and red hats will try to keep their old Unsafe hacks alive and maybe get their hands on a jdk.internal.misc.Unsafe. Here's some code that tries to do that. The call to setAccessible succeeds! And the code succeeds in getting hold of

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

On 2016-07-07 18:08, Volker Simonis wrote: Not sure how error checking could or should be improved: >VersionProps.parseVersionNumbers(String) will throw a NFE on most malformed >data, technically an IllegalArgumentException. Same thing would happen if >you ran an invalid string through

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

On Thu, Jul 7, 2016 at 5:33 PM, Claes Redestad wrote: > Hi Volker, > > On 2016-07-07 15:59, Volker Simonis wrote: >> >> Hi, >> >> can I please have a review for the following change which makes >> VersionProps.versionNumbers() testable and the corresponding >>

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

Hi Andrew, thanks a lot for the detailed explanation! Regards, Volker On Thu, Jul 7, 2016 at 4:54 PM, Andrew Dinn wrote: > On 07/07/16 14:59, Volker Simonis wrote: >> - I was a little bit surprised that I could reflectively access and >> execute

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

On 07/07/16 14:59, Volker Simonis wrote: > - I was a little bit surprised that I could reflectively access and > execute java.lang.VersionProps.parseVersionNumbers() where both the > class and the method are package private. Maybe this is related to > Jigsaw issue

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

On Thu, Jul 7, 2016 at 4:26 PM, Alan Bateman wrote: > On 07/07/2016 14:59, Volker Simonis wrote: > >> : >> >> - I was a little bit surprised that I could reflectively access and >> execute java.lang.VersionProps.parseVersionNumbers() where both the >> class and the method

Re: RFR(S): 8160564: TEST: Add a test to check the implementation of VersionProps.versionNumbers()

On 07/07/2016 14:59, Volker Simonis wrote: : - I was a little bit surprised that I could reflectively access and execute java.lang.VersionProps.parseVersionNumbers() where both the class and the method are package private. Maybe this is related to Jigsaw issue