> On Sep 27, 2018, at 11:18 AM, 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.

I meant JDK-8209546. My understanding is that even if it's a noreg-self change, 
the bug id should also be included.

Thanks
Max

> 
>> 
>> 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
>>>>> 
>>>>> 
>> 
> 

Reply via email to