Code Review Request JDK-8161106 Improve SSLSocket test template

[Artem Smotrakov]

Hi Xuelei,

I am not an official reviewer, but I have a couple of comments.

1. line 149: would it be better to check this condition in a loop?

2. Using try-with-resources blocks might simplify doServerSide() a 
little bit (no need to call close() on sockets, and a couple of "try" 
blocks might be probably removed)

3. Minor: it might be better to define timeout values as constants with 
meaningful names.

4. Minor: lines 268-273 seem to work, but I am wondering if it would be 
better to use File.separator instead of "/" (someone may run the test on 
Windows, but without Cygwin).

5. SSLSocketSample() constructor, startServer() and startClient() methods:

Recently we've had a couple of test issue where no exception was printed 
on client/server sides. It happened because exceptions were caught and 
stored, but then tests waited infinitely for another thread to finish. 
Then, jtreg just killed the tests. As a result, no exception was printed 
out. It might be better to simplify this code to print out an exception 
right after it's caught, and then throw a runtime exception if 
serverException or clientException are not null. Probably there is a 
side effect that logs may be a little messy sometimes because of lack of 
synchronization, but maybe something like synchronized(System.out) may help.

6. Minor: it might be better to make class constants uppercase

http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-135099.html#367

Artem

On 07/10/2016 10:16 PM, Xuelei Fan wrote:
> There is a nice catch of the timeout miss-match during the handling of
> serverIsReady in doClientSide().   Here is the updated webrev:
>
>      http://cr.openjdk.java.net/~xuelei/8161106/webrev.01/
>
> Thanks,
> Xuelei
>
> On 7/11/2016 11:44 AM, Xuelei Fan wrote:
>> Hi,
>>
>> Please review this enhancement of SSLSocket test template:
>>      http://cr.openjdk.java.net/~xuelei/8161106/webrev.00/
>>
>> There are some known issues with the current SSLSocket test template
>> (test/javax/net/ssl/templates/SSLSocketTemplate.java) in the automation
>> testing environment:
>> 1. the client or server can be blocked if the peer run into problems, as
>> may result in intermittent timeout failure.
>> 2. the server accepted connection may be not linked to the expected
>> client. For example, some other test case may try to use and connect to
>> a free server socket port, unfortunately this port can be actually
>> opened by the server of SSLSocketTemplate because there is no
>> synchronization about the free socket port between test cases.  It's OK
>> to run the test singly, but may result in weird intermittent failure in
>> the automation testing environment.
>>
>> The new test template in this update considers the noises above.
>>
>> Thanks,
>> Xuelei
>>