RFR: 8284047: Harmonize/Standardize the SSLSocket/SSLEngine/SSLSocketSSLEngine test templates [v3]

Rajan Halade rhalade at openjdk.org
Tue Mar 14 18:00:39 UTC 2023


On Wed, 1 Mar 2023 15:30:19 GMT, Matthew Donovan <duke at openjdk.org> wrote:

>> * Refactored SSLContextTemplate and SSLSocketTemplate to put common code in one base class (SSLContextTemplate)
>> * Updated TLS/SSL tests to extend SSLSocketTemplate where possible.
>> * Updated SSLEngineTemplate to accommodate changes in SSLContextTemplate. To keep this changeset to a reasonable size, updates to SSLEngine tests will be made under JDK-8301194.
>
> Matthew Donovan has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
> 
>  - Merge branch 'master' into templates
>  - updated certificate info
>  - Merge branch 'master' into templates
>  - reverted changes, removed SSLSocketSSLEngineTemplate, updated SSLEngineBadBufferArrayAccess.java
>  - fixed compilation errors
>  - Merge branch 'master' into templates
>  - 8284047: Harmonize/Standardize the SSLSocket/SSLEngine/SSLSocketSSLEngine test templates

Updated review looks good to me except few minor code cleanup comments.

test/jdk/javax/net/ssl/ALPN/SSLServerSocketAlpnTest.java line 84:

> 82: import java.security.KeyStore;
> 83: import java.util.Arrays;
> 84: 

`java.security.KeyStore` is unused

test/jdk/javax/net/ssl/ALPN/SSLServerSocketAlpnTest.java line 144:

> 142:      * to avoid infinite hangs.
> 143:      */
> 144:     protected void runServerApplication(SSLSocket sslSocket) throws Exception {

Missing `@Override`

test/jdk/javax/net/ssl/ALPN/SSLServerSocketAlpnTest.java line 230:

> 228:      * to avoid infinite hangs.
> 229:      */
> 230:     protected void runClientApplication(SSLSocket sslSocket) throws Exception {

Missing `@Override`. Comment applies to few other files in this review.

test/jdk/javax/net/ssl/TLSv12/DisabledShortRSAKeys.java line 58:

> 56: import java.util.Base64;
> 57: 
> 58: 

With the update to this file, many imports are unused.

test/jdk/sun/security/ssl/CipherSuite/NamedGroupsWithCipherSuite.java line 122:

> 120:     }
> 121: 
> 122:     public SSLContext createClientSSLContext() throws Exception {

please revert unnecessary change to this file

test/jdk/sun/security/ssl/SSLContextImpl/MultipleChooseAlias.java line 71:

> 69: 
> 70:     @Override
> 71:     public ContextParameters getClientContextParameters() {

Please revert unnecessary change to this file

test/jdk/sun/security/ssl/SSLContextImpl/TrustTrustedCert.java line 116:

> 114: 
> 115:     @Override
> 116:     public SSLContext createServerSSLContext() throws Exception {

Same here, no change is needed in this file.

-------------

Changes requested by rhalade (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12555


More information about the net-dev mailing list