[8u] TLSv1.3 RFR: 8245653: Remove 8u TLS tests
Martin Balao
mbalao at redhat.com
Mon Aug 10 19:46:36 UTC 2020
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.
More information about the jdk8u-dev
mailing list