Hi John 1. Please add @bug to all tests.
2. Are getLibPath() and findLib() in AutoTest.java really necessary? It looks like PKCS11Test::getNSSLibDir is doing something similar. 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 4. Maybe we can rename AutoTest.java to NssTest.java. The old name says nothing. 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 >>> >>> >