Code Review Request JDK-8170329 New SSLSocket testing template

Xuelei Fan xuelei.fan at oracle.com
Tue Dec 6 03:53:14 UTC 2016


New webrev:
     http://cr.openjdk.java.net/~xuelei/8170329/webrev.01/

On 12/5/2016 10:16 AM, Artem Smotrakov wrote:
>
> On 12/02/2016 09:48 PM, Xue-Lei Fan wrote:
>>
>>>>>>> - Exceptions are printed out in startServer/startClient methods, it
>>>>>>> doesn't look necessary to use suppressed exceptions and initCause()
>>>>>>> method. What was wrong with the code in runTest() method? The
>>>>>>> code in
>>>>>>> runTest() method looks shorter and cleaner to me.
>>>>>>>
>>>>>> The main issue of runTest() is that it does not throw the original
>>>>>> exception.  Exception tacks have more information for debugging. I
>>>>>> want to keep the good side of Brad's previous hardness on this point.
>>>>> It is not clear what's the original exception should be because you
>>>>> have
>>>>> client and server. If you print all exceptions in
>>>>> startServer/startClient (right after they occurred), then you don't
>>>>> hide
>>>>> anything helpful for debugging.
>>>>>
>>>> The exception dump may have more information than the single exception
>>>> track, for example the nested cause stacks.
>>> Can't it be printed out in startServer/startClient accordingly?
>> Throw the right exception is important even for test case.  Having the
>> Java handling the exception is more nature.  Reading the debug log
>> dump is not as straightforward as read the exception at the end of
>> each run. Wrapping into RuntimeException is very confusing sometimes.
>>
>> Comparing the following two log:
>>    java.net.XXException:  this is an exception message.
>>    cased by A
>>    caused by B.
>> and
>>    java.lang.RuntimeException:  xxxx
>>   caused by java.net.XXException:  this is an exception message.
>>   caused by A.
>>   (cause by B may be swallowed)
>>
>> I don't like the later one, which introduce unnecessary exception
>> stacks and take more time for the debugging.
> I am not suggesting neither #2.
>
> I am suggesting not to use initCause because it modifies the original
> exception, and not to use addSupressed() because it's not clear where
> the suppressed exception came from. Instead, I am suggesting to print
> all original exceptions on that sides where they occurred. I am not
> suggesting to wrap anything into a runtime exception. A runtime
> exception can be used just to indicate a test failure.
I see your points.  But most of the people would first read the 
exception stacks at first.  It's a natural behavior to check the failure 
exception at first for developers and customers.  Failure with the right 
exception helps a lot for debugging.  The use of supressed and caused 
exception is also for this consideration.

Xuelei



More information about the security-dev mailing list