code review request: 7081411: Change more keytool -genkeypair to RSA

[Sean Mullan]

I agree that we should not remove all tests for DSA keypair generation - this
could also hide any new regressions in that area.

It's my understanding that there are still some tests that test DSA keypair
generation, just not as many, so the likelihood of encountering the Solaris bug
is smaller. Is that right, Max? As long as we still have some test coverage of
DSA keypair generation - I think it is ok.

--Sean

On 8/31/11 8:16 AM, Xuelei Fan wrote:
> I may argue more in a different viewpoint from you. ;-)
> 
> 1. I don't think the failure is noise, it is valuable information to
> tell us that there is problem in our implementation or underlying
> platform. If it happens again and again, it means that we don't solve
> the problem from build to build. If we decrease the possibility to
> reproduce the problem, we also also decrease the possibility to find the
> underlying problem.
> 
> 2. I don't concern too much about how you avoid the problem in keytool.
> I'm against the approach to cut the test to fit problematic platform.
> The "best" result of the approach at last would be that we will not
> likely to find any problem by running regression test, because we have
> already filtered out nearly all known issues by cutting the test.
> 
>    It is a good test if it can expose problems, it can be anything but a
> test if it is expected to pass in all situation. Test is not designed to
> pass, at least I think so.
> 
> 3. If we do find the platform problem, and it do impact a lot of tests,
> and it cannot be fixed in a short team. I think we may be able to
> isolate the platform problem in the tests in a short run. But we should
> not isolate the function in all platform.
> 
>    For example, we know that there are PKCS11 padding problems in
> solaris 11. If we do want to cut the tests, We may only need to isolate
> PKCS11 provider in solaris 11, but not PKCS11 provider problem in all
> platforms, or even worse to isolate all padding operations.
> 
> Just my very few sense about the approach. It is up to you to make the
> final decision.
> 
> Thanks,
> Xuelei
> 
> On 8/31/2011 4:37 PM, Weijun Wang wrote:
>> More:
>>
>> You can see that I simply added "-keyalg rsa". Most of these tests are
>> not designed to test on any algorithm, instead, they test various
>> functions/commands of the tool. Maybe this change will reveal some RSA
>> problems. Who knows? :)
>>
>> (I admit if it really finds out any, I don't know what my next change
>> should be)
>>
>> For a few tests that do test key algorithms, I have not changed them.
>> For example, in KeyToolTest.java, some lines check the matching of
>> -keyalg and -sigalg. Those still have a chance to fail on Solaris but I
>> haven't removed them.
>>
>> Thanks
>> Max
>>
>> On 08/31/2011 04:23 PM, Weijun Wang wrote:
>>> My personal view was not faraway from yours -- I am not so frightened by
>>> test failures.
>>>
>>> However, zero tolerance of any test failure is becoming a common sense
>>> of the team and the whole JPG [1]. Evaluating test failures is consuming
>>> too much time for both release engineers, SQE and us. Especially in this
>>> case, other people might not easily find out it's the Solaris DSA bug
>>> that causes the failure.
>>>
>>> Therefore, my current opinion is that once the reason of a test failure
>>> is known, we should take actions immediately. Either fix it if we can,
>>> or fix the test (or problem list it) if we cannot fix the bug. There is
>>> no benefit in leaving them there making noises from time to time.
>>>
>>> In this case, I certainly do not want to add all of them to problems
>>> list. Also, since the changeset is there, we always have a chance to
>>> backout the changes when we want to bring the old tests back.
>>>
>>> In fact, if you are worried that changing the tests might hide the bug,
>>> I can add a new test that detects this bug. I'll make sure the test
>>> always fails on Solaris.
>>>
>>> Thanks
>>> Max
>>>
>>> [1] http://wiki.se.oracle.com/display/JPG/Home
>>>
>>> On 08/30/2011 10:36 PM, Xuelei Fan wrote:
>>>> The update, in both open and closed repositories, looks fine to me.
>>>>
>>>> However, the following reason cannot convince me of the necessity to
>>>> make the change.
>>>>
>>>>> Because of the Solaris DSA bug described in 7041639, we keep seeing
>>>>> tests generating DSA key pairs failing. Therefore I'm changing most
>>>>> keypair generation to use RSA instead.
>>>>
>>>> Most of the updated tests using the default key algorithm ("DSA"). I'm
>>>> thinking, shall we replace "RSA" back with "DSA" again when there is a
>>>> RSA bug in the native libraries in the future? It may be not a good
>>>> choice to cut the tests to fit problematic platform. These tests are
>>>> also very good test to find the potential problems, right? When we
>>>> change the test to be able to passed on all platform, the test may lost
>>>> it function to find potential issues partially.
>>>>
>>>> Just my very personal view.
>>>>
>>>> Thanks,
>>>> Xuelei
>>>>
>>>>
>>>> On 8/30/2011 8:26 PM, Weijun Wang wrote:
>>>>> Hi All
>>>>>
>>>>> 7081411: Change more keytool -genkeypair to RSA
>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7081411
>>>>>
>>>>> Webrev at http://cr.openjdk.java.net/~weijun/7081411/webrev.01/
>>>>>
>>>>> Because of the Solaris DSA bug described in 7041639, we keep seeing
>>>>> tests generating DSA key pairs failing. Therefore I'm changing most
>>>>> keypair generation to use RSA instead.
>>>>>
>>>>> In all code changes, KeyToolTest.java is called by standard.sh, which
>>>>> makes so many "keytool -genkeypair" calls that I decide to add
>>>>> "-keysize
>>>>> 512" to make it fast. Please note that in this test there are still
>>>>> explicit calls to "-genkeypair -keyalg dsa". These still have a chance
>>>>> to fail on Solaris, but I like to keep them there to make the test
>>>>> complete.
>>>>>
>>>>> Code changes in the closed repo will be sent in another mail.
>>>>>
>>>>> Thanks
>>>>> Max
>>>>>
>>>>
>