RFR 8136933: Additional tests for Solaris SO_FLOW_SLA socket option in JDK 9
Svetlana Nikandrova
svetlana.nikandrova at oracle.com
Mon May 23 16:36:18 UTC 2016
Alan, Chris,
thank you for your comments. I've decided to do as Chris suggested and
updated existing test test/jdk/net/Sockets/Test.java instead of
creating a new one.
Please see updated review:
http://cr.openjdk.java.net/~snikandrova/8136933/webrev.03/
<http://cr.openjdk.java.net/%7Esnikandrova/8136933/webrev.03/>
Thank you,
Svetlana
On 20.05.2016 18:19, Chris Hegarty wrote:
>> On 20 May 2016, at 14:10, Alan Bateman <Alan.Bateman at oracle.com> wrote:
>>
>> On 20/05/2016 14:05, Svetlana Nikandrova wrote:
>>> Alan,
>>>
>>> another test related to this option is on the same level (test/jdk/net/SocketFlow)
> I added this recently, when working on a different issue, when I
> noticed that there were no tests for the SocketFllow API, regardless
> of the underlying system support. The test directory structure
> matches the class package/name hierarchy.
>
>>> so I thought it's ok to maintain the same hierarchy. I can move test to test/jdk/net/Sockets/ExtendedSocketOptions/SO_FLOW_SLA or to test/jdk/net/Sockets, but in that case won't it be a little bit confusing?
>>>
>>> As for overlapping coverage: existing test silently exits if option is not in supported list while this one is focused on platform support check.
>> I think it would be good to put all the tests in the same place, will make it easier for further maintenance in this area.
> The proposed new test seems, somewhat, to be a replacement for
> test/jdk/net/Sockets/Test.java, but it does not assert that if set
> succeeds that get should return something useful? Also set
> typically cannot succeed unless run with privileges. The test also
> seems to focus on setting the option through the idk.net.Sockets API
> rather than through the setOption of the socket types, any reason
> for this? The jdk.net.Sockets API should be deprecated in 9, as
> we have a standard replacement.
>
> When working in the area previously, I ran
> test/jdk/net/Sockets/Test.java manually and examined the output
> to determine that the option was being set correctly.
>
> It this test is merely to check that the option is supported on
> S 11.2 +, then maybe that could be added to the existing
> test/jdk/net/Sockets/Test.java ?
>
> -Chris.
More information about the net-dev
mailing list