[9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

[Artem Smotrakov]

Hi Xuelei,

This totally makes sense. But in my opinion we should use new features 
and APIs because:
- they are here to make code readable (okay, not to much lambdas)
- they help to avoid bugs (e.g., forgetting to close resources)
- we implicitly provide test coverage for new APIs
- we give examples how these features can be used
- finally, we encourage people to move to new Java versions

Not sure if I got correction what you mean by removing the part to 
declare the store files in each sub-classes. Do you mean getting rid of 
setting keystore/truststore in actual test files?

If so, I would prefer to do it in a separate enhancement. Most of 
SSL/TLS tests use keystores from "etc" directory. The test use 
"test.src" system property (set by jtreg) to build a path to "etc", and 
it depends on actual directory where a test stays. Also if I recall 
correctly, some tests use their own keystore. This update may be quite 
big. Would you mind if we did it separately?

8168969 is about merging helper classes to remove possible confusions 
Brad mentioned recently.

Artem

On 11/02/2016 03:57 AM, Xuelei Fan wrote:
> Please use JDK 6 only features (no Lambda, try-with-resource, 
> multi-catch, java.util.Objects, etc.) so as to expedite porting to 
> previous releases.
>
> It would be nice if removing the part to declare the store files in 
> each sub-classes.
>
> Xuelei
>
> On 11/2/2016 2:48 PM, John Jiang wrote:
>> Hi Artem,
>> Thanks for making the template to be used easily.
>>
>> The tests in your patch extend class SSLSocketTemplate, but
>> SSLSocketTemplate looks like an utility class, but not a parent class.
>> For example,
>> test/sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java
>>   67         new SSLSocketTemplate()
>> AnonCipherWithWantClientAuth still creates a SSLSocketTemplate instance,
>> but not itself.
>> In fact, if replacing "print()" with "SSLSocketTemplate.print()",
>> AnonCipherWithWantClientAuth can work fine without extending
>> SSLSocketTemplate.
>>
>> The original SSLSocketSample.java defines the core methods, such as
>> doServerSide(), runServerApplication(), doClientSide() and
>> runClientApplication(), as non-static. In my eyes, this style may be
>> better. The child class can simply override the methods if necessary.
>>
>> In addition, some tests have to configure SSLServerSocket. Why not
>> provide one more extension point in doServerSide()? Then, it unnecessary
>> to re-write the whole doServerSide() (or, set a new server peer).
>> The code talks more clearly. Please take a look at my example:
>> http://cr.openjdk.java.net/~jjiang/8168969/example/
>>
>> Best regards,
>> John Jiang
>>
>> On 2016/11/2 8:54, Artem Smotrakov wrote:
>>> Hello,
>>>
>>> Please review the following patch which merges a couple of classes in
>>> javax/net/ssl/templates.
>>>
>>> SSLTest class contains re-usable parts of SSLSocketSample.
>>> SSLSocketTemplate class is buggy (tests which follows it may fail
>>> intermittently). I basically replaced SSLSocketTemplate with SSLTest,
>>> and removed SSLSocketSample.
>>>
>>> SSL/TLS tests should use SSLSocketTemplate class. I updated test which
>>> use SSLTest to use SSLSocketTemplate.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8168969
>>> Webrev: http://cr.openjdk.java.net/~asmotrak/8168969/webrev.00/
>>>
>>> Artem
>>>
>>