The change looks fine. Thanks Max
> On Sep 28, 2018, at 8:55 AM, [email protected] wrote: > > On 2018/9/27 21:20, Weijun Wang wrote: >> Change looks fine. >> >> On PKCS11Test.java:453, you can use Files::newInputStream. >> > Just addressed it, please take a look at this updated webrev: > http://cr.openjdk.java.net/~jjiang/8209546/webrev.03/ > It also renames the test in ProblemList.txt accordingly. > > Best regards, > John Jiang >> Thanks >> Max >> >> >>> On Sep 27, 2018, at 5:34 PM, [email protected] >>> wrote: >>> >>> Hi Max, >>> Please review this new webrev: >>> http://cr.openjdk.java.net/~jjiang/8209546/webrev.02/ >>> >>> You previous points, except #1, are addressed. >>> >>> Best regards, >>> John Jiang >>> >>> On 2018/9/27 11:18, >>> [email protected] >>> wrote: >>> >>>> On 2018/9/27 10:34, Weijun Wang wrote: >>>> >>>>> Hi John >>>>> >>>>> 1. Please add @bug to all tests. >>>>> >>>> Which issue should be linked? JDK-8209546? >>>> I suppose @bug should indicate a product issue here. >>>> At least, JDK-8209546 looks have no much association with this test. >>>> >>>> >>>>> 2. Are getLibPath() and findLib() in AutoTest.java really necessary? It >>>>> looks like PKCS11Test::getNSSLibDir is doing something similar. >>>>> >>>> I'll modify PKCS11Test.java a bit to help this point. >>>> >>>> >>>>> 3. Looks like Standard.java is not necessary now. You can just make >>>>> KeyToolTest.java a @test and add a @run line there, something like >>>>> >>>>> @run main/othervm/timeout=600 -Dfile KeyToolTest >>>>> >>>> Will address. >>>> >>>> >>>>> 4. Maybe we can rename AutoTest.java to NssTest.java. The old name says >>>>> nothing. >>>>> >>>> Will address. >>>> >>>> Best regards, >>>> John Jiang >>>> >>>>> Thanks >>>>> Max >>>>> >>>>> >>>>>> On Sep 27, 2018, at 9:25 AM, [email protected] >>>>>> wrote: >>>>>> >>>>>> Hi Max, >>>>>> Please review the updated webrev: >>>>>> http://cr.openjdk.java.net/~jjiang/8209546/webrev.01/ >>>>>> >>>>>> All your comments are addressed, though this test is moved to problem >>>>>> list for windows due to JDK-8204203. >>>>>> >>>>>> Best regards, >>>>>> John Jiang >>>>>> On 2018/9/25 22:30, Weijun Wang wrote: >>>>>> >>>>>>> Some questions: >>>>>>> >>>>>>> 1. Do we still need the OS check on lines 47-49? As long as >>>>>>> getLibPath() can return something, does it mean the test should just >>>>>>> run? Especially, does the test run on Windows? >>>>>>> >>>>>>> 2. Is launching a separate process necessary? Can we just call >>>>>>> KeyToolTest::main after setting system properties and copying the files. >>>>>>> >>>>>>> 3. Is it possible to include standard.sh? >>>>>>> >>>>>>> Thanks >>>>>>> Max >>>>>>> >>>>>>> >>>>>>> >>>>>>>> On Sep 25, 2018, at 6:30 PM, [email protected] >>>>>>>> >>>>>>>> wrote: >>>>>>>> >>>>>>>> Hi, >>>>>>>> JDK-8164639 removed NSS libs from repo, so >>>>>>>> sun/security/tools/keytool/autotest.sh has to download NSS libs from >>>>>>>> artifactory on macosx. >>>>>>>> This patch also refactors this shell test to a Java test. >>>>>>>> >>>>>>>> Webrev: >>>>>>>> >>>>>>>> http://cr.openjdk.java.net/~jjiang/8209546/webrev.00/ >>>>>>>> >>>>>>>> >>>>>>>> Issue: >>>>>>>> >>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8209546 >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Best regards, >>>>>>>> John Jiang >>>>>>>> >>>>>>>> >>>>>>>> >
