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

[Martin Balao]

Hi Alexey,

Thanks for proposing a new Webrev.

Comments inline.

Regards,
Martin.-


On 8/10/20 10:16 AM, Alexey Bakhtin wrote:
>>>> * GenSSLConfigs
>>>>  * main.java
>>> Yes, removed
>>
>> Could not verify this removal. Missing?
> Yes. removed again

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.

Good.

>>>> * 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.

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

Ok.

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

Ok.

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

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

Good, thanks.
>>>>  * 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.

Good, thanks!

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

Ok. Verified in Step 9 (8245477) - Webrev 04:
https://mail.openjdk.java.net/pipermail/jdk8u-dev/2020-August/012395.html

>> 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:
> * OriginServer.java
>

Was OriginServer.java removal missing from Webrev.02? I could not verify
this change.

>> * test/sun/security/ssl
>>  * SSLEngineImpl
>>   * CloseInboundException.java
> This test was removed as part of JDK-8196584: TLS 1.3 Implementation
> Removed

Ok. Yes, I think that's better.

>>  * SessionIdCollisionTest.java
> This test was added as part of JDK-8203190 and not applicable to JDK11 implementation. Removed

Good.