[9] RFR: JDK-8164322: sun/security/pkcs11/PKCS11Test.java shall be updated to run on ARM platforms

[Xuelei Fan]

Looks fine to me.

Xuelei

On 9/30/2016 4:39 PM, Tim Du wrote:
> Thanks Xuelei review it.
> Latest webrev available here
> :http://cr.openjdk.java.net/~tidu/8164322/webrev.02/ ,Please help to
> review it. Thanks
>
> Regards
> Tim
> On 2016/9/29 18:16, Xuelei Fan wrote:
>> On 9/29/2016 5:36 PM, Tim Du wrote:
>>> Thank you for reviewing the codes.
>>> Remove the codes which throw exception and add codes could show machine
>>> not installed NSS library information.
>>> The latest code is here:
>>> http://cr.openjdk.java.net/~tidu/8164322/webrev.01/ ,please help to
>>> review it.
>>>
>> Looks fine to me.  A very minor comment:
>>  303   + ",please initialize ...
>>
>> Please add a white space between ",' and 'please".
>>
>> Thanks,
>> Xuelei
>>
>>> For whether throw exception caused by OS/machine issue ,I file an
>>> Enhancement https://bugs.openjdk.java.net/browse/JDK-8166893 , we can
>>> keep discussing by it ,and then the test
>>> sun/security/pkcs11/PKCS11Test.java could be supported to run on ARM
>>> platform firstly. Thanks
>>>
>>> Regards
>>> Tim
>>> On 2016/9/29 13:05, Xuelei Fan wrote:
>>>> On 9/29/2016 9:26 AM, Artem Smotrakov wrote:
>>>>> Hi Xuelei,
>>>>>
>>>>> This is not a problem with machine configuration. The actual test
>>>>> cases
>>>>> are not going to be run (even if there are NSS libs on a test machine)
>>>>> until "osMap" contains an entry for specific platform.
>>>>>
>>>> OK.  Please fix for the purpose accordingly.
>>>>
>>>> Tim, the current fix has two parts.  The 1st part, which throw
>>>> exception is a machine configuration problem, is not in the scope of
>>>> Artem's consideration, please remove it.  The 2nd part, which add new
>>>> items to the osMap, is fine to me.  I think we are on the same page
>>>> now.  Please update the webrev accordingly.  I will approve the 2nd
>>>> part fix.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>
>>>>> Artem
>>>>>
>>>>>
>>>>> On 09/28/2016 05:52 PM, Xuelei Fan wrote:
>>>>>> Hi Artem,
>>>>>>
>>>>>> What do you think fix the testing infrastructure (JPRT/Mach5) with
>>>>>> proper configuration?  I think it is a easier than update a large
>>>>>> bunch of test codes.
>>>>>>
>>>>>> I understand the concerns of yours, but I don't want add additional
>>>>>> effort for those who need to run the test on their own environment.
>>>>>> Some people run the test locally before submit JPRT jobs (or no
>>>>>> access
>>>>>> to JPRT systems for external people).
>>>>>>
>>>>>> Update the testing infrastructure may solve both of your concerns and
>>>>>> my concerns.
>>>>>>
>>>>>> I'm not sure the @requires tag would work or not.  It would be great
>>>>>> if you can find a solution with the tag.
>>>>>>
>>>>>> Thanks,
>>>>>> Xuelei
>>>>>>
>>>>>> On 9/29/2016 4:32 AM, Artem Smotrakov wrote:
>>>>>>> Hi Xuelei,
>>>>>>>
>>>>>>> I understand your concerns. But I'd prefer to be aware of situations
>>>>>>> when a test reports that it passed when it actually did nothing.
>>>>>>>
>>>>>>> How about using @requires key? We can try to specify all expected
>>>>>>> platforms. If I understand correctly, jtreg won't run tests if
>>>>>>> specified
>>>>>>> requirements are not met. In this case, you need to look at the
>>>>>>> report
>>>>>>> to make sure all tests you are interested in were run.
>>>>>>>
>>>>>>> Artem
>>>>>>>
>>>>>>>
>>>>>>> On 09/28/2016 08:00 AM, Xuelei Fan wrote:
>>>>>>>> On 9/28/2016 9:20 PM, Denis Kononenko wrote:
>>>>>>>>> There're 60+ tests related to PKCS11. Two years they have been
>>>>>>>>> "passed" on 3 unsupported platforms on hosts where usually no NSS
>>>>>>>>> libraries were installed. How can we rely on these results?
>>>>>>>> ;-) The words of "unsupported platforms" are very confusing in this
>>>>>>>> mail thread.
>>>>>>>>
>>>>>>>> Let's think more about what if a test failed.  What one will do
>>>>>>>> if a
>>>>>>>> test failed?
>>>>>>>> 1. Test fail means source code problems for developers. One cannot
>>>>>>>> submit a change-set if a test failed.  He need to pay additional
>>>>>>>> effort and analysis the failure.  It take one developer a lot
>>>>>>>> effort
>>>>>>>> to know the root cause.  I would never like this unnecessary cost.
>>>>>>>> 2. In order to get the test pass, he need to install the NSS libs
>>>>>>>> although NSS has nothing to do with his changeset.  It may be a
>>>>>>>> very
>>>>>>>> very hard step or even impossible (for example licenses issues)
>>>>>>>> step
>>>>>>>> for him.
>>>>>>>>
>>>>>>>> TBH, I did not see much benefits to fail on unsupported
>>>>>>>> platforms.  I
>>>>>>>> agree that skip for pass is not a good idea, but fail to warn is
>>>>>>>> worse.
>>>>>>>>
>>>>>>>> I think the root cause if "unsupported platforms" actually are
>>>>>>>> supported platforms, but by accident the NSS libraries are not
>>>>>>>> installed or not installed properly.
>>>>>>>>
>>>>>>>> If one is not interested in NSS, the test get ignored (passed). If
>>>>>>>> one
>>>>>>>> is interested in NSS, he should install the NSS libs and the
>>>>>>>> test get
>>>>>>>> checked.
>>>>>>>>
>>>>>>>> What do you think if fix the testing infrastructure with properly
>>>>>>>> installed NSS libs?
>>>>>>>>
>>>>>>>> > The problem is the tests report they passed but actually they
>>>>>>>> were
>>>>>>>> > skipped. I have no objections against skipping tests but it would
>>>>>>>> > be better to give a hint somehow how many tests were skipped and
>>>>>>>> why.
>>>>>>>> Agreed.  Unfortunately, there are only two options, "pass" or
>>>>>>>> "fail",
>>>>>>>> at present.  It would be nice if there is a grey area. Any idea to
>>>>>>>> make summary of skipped tests and reasons?
>>>>>>>>
>>>>>>>> Xuelei
>>>>>>>
>>>>>
>>>
>