RFR 8136933: Additional tests for Solaris SO_FLOW_SLA socket option in JDK 9

Svetlana Nikandrova svetlana.nikandrova at oracle.com
Tue May 24 16:21:35 UTC 2016


I hate being annoying, but may be you can find another minute to review 
updated diff?

Thank you,
Svetlana

On 23.05.2016 19:36, Svetlana Nikandrova wrote:
> 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