On 06/05/2020 21:27, Patrick Concannon wrote:
Hi Alan,
With regards to the SetGetSendBufferSize.java test: yes, it was
created to increase the test coverage for the DatagramSocket class.
However, it was decided that it should be handled separately and was
pushed to the mainline in advance of this RFR - (JDK-8243488
<https://bugs.openjdk.java.net/browse/JDK-8243488>).
Please find the updated webrev below:
http://cr.openjdk.java.net/~pconcannon/8241072/webrevs/webrev.05/
I think this is a good shape and you (or Daniel) should be able to push
soon after JEP 373 has been targeted and the code review is completed here.
I don't have any further comments on the implementation changes.
My only issue with the tests is that I don't think SendBufCheck should
be pushed to jdk/jdk. Test SetGetSendBufferSize has
testInitialSendBufferSize to test the initial size of the send buffer so
you have test coverage for the initial size. My problem with
SendBufCheck is that it is checks that bug JDK-8242885 exists in the old
implementation. That just adds technical debt to the test suite. I'd
prefer to see JDK-8242885 fixed and
SetGetSendBufferSize/testInitialSendBufferSize updated to test a
maximally sized IPv6 packet (we should probably have done that in
advance of the JEP). At some point I think we should also make sure that
we have a good set of tests that send large datagrams on the network
(not the loopback) to ensure that we don't have any other issues.
-Alan.
