[9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

[Xuelei Fan]

I don't like it, too.  It is not necessary to make the merge in a hurry. 
  Maybe, we can wait for a while so that we have time for a real and 
comfortable template.

Thanks,
Xuelei

On 11/16/2016 10:44 AM, Artem Smotrakov wrote:
> 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
>>>>>
>>>>
>>
>