Change looks fine. On PKCS11Test.java:453, you can use Files::newInputStream.
Thanks Max > On Sep 27, 2018, at 5:34 PM, sha.ji...@oracle.com 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, sha.ji...@oracle.com 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, sha.ji...@oracle.com 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, sha.ji...@oracle.com >>>>>> 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 >>>>>> >>>>>> >>> >> >> >
