[9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

[Artem Smotrakov]

I don't like it, but no lambdas, no nothing in SSLSocketTemplate.java

http://cr.openjdk.java.net/~asmotrak/8168969/webrev.01/

Xuelei,

Could you please take a look?

Artem


On 11/02/2016 09:14 AM, Artem Smotrakov wrote:
> 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
>>>>
>>>
>