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
