RFR[12] JDK-8209546: Make sun/security/tools/keytool/autotest.sh to support macosx

[Weijun Wang]

The change looks fine.

Thanks
Max

> On Sep 28, 2018, at 8:55 AM, sha.jiang at oracle.com 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, sha.jiang at 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.jiang at 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.jiang at 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.jiang at 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
>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>