[8u] TLSv1.3 RFR: 8245653: Remove 8u TLS tests

[Alexey Bakhtin]

Hi Matrtin,

Thank you for review.

Please find updated patch at :
http://cr.openjdk.java.net/~abakhtin/tls1.3/8245466/8245653/webrev.v2/

Some comments below.

Regards
Alexey

> On 4 Aug 2020, at 20:07, Martin Balao <mbalao at redhat.com> wrote:
> 
> Hi Alexey,
> 
> Thanks for taking the time to thoroughly go through each file! When I
> first elaborated the list, just sampled a couple of files and was not
> going to finish it before asking. Then I decided that it might be useful.
> 
> Comments below.
> 
> Thanks,
> Martin.-
> --
> 
> On 8/3/20 1:36 PM, Alexey Bakhtin wrote:
>>> * ClientHandshaker
>>>  * RSAExport.java
>> Copyright date changed only, no reason to remove/update
> 
> Ok
> 
>>> * GenSSLConfigs
>>>  * main.java
>> Yes, removed
> 
> Could not verify this removal. Missing?
Yes. removed again
> 
>>> * HandshakeHash
>>>  * MyProvider.java
>> Test updated for new Provider API, no reason to update for JDK11
> 
> Ok.
> 
>>> * HandshakeOutStream
>>>  * NullCerts.java
>> Changes in the comment only. No reason to remove/update
> 
> Can we still delete that commented line in this step? Thanks
OK. The line is removed without file removal.
> 
>>> * InputRecord
>>>  * ClientHelloRead.java
>> Changes not required for JDK8 (@modules annotation)
> 
> Ok.
> 
>>> * ProtocolVersion
>>>  * HttpsProtocols.java
>> Copyright updated without test changes - no reason to remove/update
> 
> Ok.
> 
>>> * SSLContextImpl
>>>  * CustomizedCipherSuites.java
>> No changes in the test - no reason to remove/update
> 
> The reference to 8208350 was removed. I did not find any reference to
> DES algorithms. I suggest to remove it in this step.
Even if this test is modified as part of 8208350, it does not test 8208350 functionality.
So, reference to 8208350 should be removed. Test updated without file removal.
> 
>>> * SSLEngineImpl
>>>  * DelegatedTaskWrongException.java
>> Copyright updated without test changes - no reason to remove/update
> 
> Ok.
> 
>>> * SSLSocketImpl
>>>  * SetClientMode.java
>> This test marked @ignore in JDK11 but passed in JDK8 with new TLSv1.3 impleentation. No reason to remove
> 
> Passing does not guarantee that the test works properly. Unless we
> analyze the reasons, I suggest to stick to JDK-11's judgement.
OK. Test removed
> 
>>> * ServerHandshaker
>>>  * GetPeerHostClient.java
>> No functional changes in the test (removed unused import). I think no reason to remove/udate it
> 
> Even though it's a non-functional change, I'd either remove the import
> here or delete/add.
OK. import removed
> 
>>>  * GetPeerHostServer.java
>> No changes in the test
> 
> Ok.
> 
>>> * X509TrustManagerImpl
>>>  * CheckNullEntity.java
>> Changes not required for JDK8 (@modules annotation). No reason to remove/update
> 
> Ok.
> 
>>> * rsa
>>>  * SignatureOffsets.java
>>>  * SignedObjectChain.java
>> Changes not required for JDK8.No reason to remove/update
> 
> Ok, if they pass in Step 11 I have no objections.
> 
>>> * test/com/sun/net/ssl
>>> * SSLSecurity
>>>  * ProviderTest.java
>>>  * TruncateArray.java
>> Changes not required for JDK8 (@modules annotation). No reason to remove/update
> 
> Ok. And the reference to '8130181' is something we don't even want.
> 
>>> * test/javax/net/ssl
>>> * ALPN
>>>  * MyX509ExtendedKeyManager.java
>>>  * SSLServerSocketAlpnTest.java
>>>  * SSLSocketAlpnTest.java
>> Changed copyrigth only, no reason to remove tests
> 
> Ok.
> 
>>> * SSLParameters
>>>  * UseCipherSuitesOrder.java
>> Changed copyrigth only, no reason to remove tests
> 
> Ok.
> 
>>> * SSLSession
>>>  * CheckMyTrustedKeystore.java
>> Changes not required for JDK8 (@modules annotation). No reason to remove/update
> 
> Ok.
> 
>>> * ServerName
>>>  * SSLEngineExplorer.java
>>>  * SSLSocketExplorer.java
>> Changed copyrigth only, no reason to remove tests
> 
> Ok.
> 
>>> * TLSv11
>>>  * EmptyCertificateAuthorities.java
>>>  * GenericBlockCipher.java
>>>  * GenericStreamCipher.java
>> Changes not required for JDK8 (@modules annotation). No reason to remove/update
> 
> Ok. We have the imports but not a big deal really.
> 
>>> * ciphersuites
>>>  * DisabledAlgorithms.java
>> No functional changes in the test. No reason to remove/update
> 
> The reference to 8157035 was removed. I suggest to consider this a
> relevant change for removal.
OK. Test removed
> 
>>> * etc
>>>  * keystore
>>>  * truststore
>> Yes, these files should be removed in this step.
> 
> Ok. I'm not seeing this in jdk.patch but that's because this is a change
> in binaries. I could read the 'Binary files...' message, though.
> 
>>> * sanity
>>>  * pluggability
>>>   * CheckSSLContextExport.java
>> No changes required for JDK8 (uses new provider API)
> 
> Ok, and we don't want the reference to 8130181.
> 
>>> * templates
>>>  * SSLExplorer.java
>> No functional changes in the test (removed unused import) No reason to remove/update
> 
> Ok.
> 
>>> * GetInstance.java
>> Changes not required for JDK8 (@modules annotation). No reason to remove/update
> 
> Ok.
> 
>>> * test/sun/net/www/protocol/https
>>> * HttpsURLConnection
>>>  * B6226610.java
>>>  * CheckMethods.java
>> Changes not required for JDK8 (@modules annotation). No reason to remove/update
> 
> Ok.
> 
>>>  * DNSIdentities.java
>> No functional changes in the test (removed unused import) No reason to remove/update
> 
> Ok. It still hurts to see an used import from an internal class.
OK. Unused import is removed
> 
>>>  * HttpsCreateSockTest.java
>>>  * HttpsSocketFacTest.java
>> Changes not required for JDK8 (@modules annotation). No reason to remove/update
> 
> Ok.
> 
>>>  * IPAddressDNSIdentities.java
>>>  * IPAddressIPIdentities.java
>>>  * IPIdentities.java
>>>  * Identities.java
>> No functional changes in the test (removed unused import) No reason to remove/update
> 
> The unused import again. If you want to delete the import as part of
> this change -diverting a bit from the step spirit-, I'd be happy.
> Otherwise, okay.
OK. Unused import is removed.
> 
>>>  * PostThruProxy.java
>>>  * PostThruProxyWithAuth.java
>> Changes not required for JDK 8. No reason to remove/update
> 
> Ok.
> 
>>>  * RetryHttps.java
>> No  functional changes in the test (enabled debug output) No reason to remove/update
> 
> Ok.
> 
>>> * NewImpl
>>>  * ComHTTPSConnection.java
>>>  * ComHostnameVerified.java
>> Yes, these tests should be removed in this step.
> 
> Ok.
> 
>>> * ChunkedOutputStream.java
>> Changes not required for JDK8 (@modules annotation). No reason to remove/update
> 
> Ok.
> 
> 
> There are still a couple of thing I'd like you to have a look (from my
> previous email):
> 
> Files that should be removed because they do not exist in JDK-11, were
> relocated or renamed:
> 
> * test/sun/security/ssl
>  * SSLContextImpl
>   * DefautlCacheSize.java
>    * Relocated to SSLSessionContextImpl (there might be file changes as
> well)
> 
Yes, you are right. This test should be moved into test/sun/security/ssl/SSLSessionContextImpl
I will update JDK-8245477 accordingly

> SSL-related tests that are in JDK-8 but not in JDK-11:
> 
> * test/sun/net/www/protocol/https
>  * HttpsClient
>   * OriginServer.java

Yes, you are right. test/sun/net/www/protocol/https/HttpsClient tests were updated in JDK11.
The following files should be removed and updated in the next step:
* ProxyAuthTest.java
* ServerIdentityTest.java
* OriginServer.java

> * test/sun/security/ssl
>  * SSLEngineImpl
>   * CloseInboundException.java
This test was removed as part of JDK-8196584: TLS 1.3 Implementation
Removed
>  * SessionIdCollisionTest.java
This test was added as part of JDK-8203190 and not applicable to JDK11 implementation. Removed
> 
> Are we sure that we want to keep them?
> 
>