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

[Svetlana Nikandrova]

Hi Chris,

thank you for your comments. Please see updated review. (I left braces 
in one line "if" blocks, hope it wasn't strong objection)

http://cr.openjdk.java.net/~snikandrova/8136933/webrev.04/ 
<http://cr.openjdk.java.net/%7Esnikandrova/8136933/webrev.04/>

Tested as standalone 1 test manual run and with JPRT.

Thank you,
Svetlana

On 24.05.2016 21:59, Chris Hegarty wrote:
>> On 24 May 2016, at 17:21, Svetlana Nikandrova <svetlana.nikandrova at oracle.com> wrote:
>>
>> I hate being annoying, but may be you can find another minute to review updated diff?
> This mainly look fine. Some comments:
>
>   - The test will need to '@build jdk.testlibrary.*’ to ensure that the library classes
>      are compiled. Please verify that the test can run successfully, by itself, in a
>      clean work directory.
>
>   - You can initialize ‘expectSupport' and make it final:
>       private static final boolean expectSupport = checkExpectedOptionSupport();
>   
>   - If you statically import SO_FLOW_SLA, then things will fit easier, e.g.
>
>     import static jdk.net.ExtendedSocketOptions.SO_FLOW_SLA;
>     …
>        if (s.supportedOptions().contains(SO_FLOW_SLA) != expectSupport) {
>
>        … and maybe drop the unnecessary braces.
>
>   - Can you please revert the indentation on SocketFlow.create(),
>     i.e. line up the dots
>
> -Chris.
>
>> 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.