[9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

[Artem Smotrakov]

Hi John,

Please see inline.


On 11/01/2016 11: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.
Correct, AnonCipherWithWantClientAuth can work fine without extending 
SSLSocketTemplate.

I make those classes to extend SSLSocketTemplate to make lines shorter 
(we keep lines shorter than 80 symbols).
>
> 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.
I think it may be better to keep them static which also means stateless 
here. It would be easier to re-use in my opinion.
>
> 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).
I see your point, and agree with that. I would prefer to add this 
extension when we update a test which really needs it. If you have such 
a bug, please feel free to do that.

We have more than one hundred SSL/TLS tests, and they may need some 
other extensions. I am not sure that it's possible to provide a 
universal SSLSocketTemplate which fits all test's needs.

Artem
> 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
>>
>