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

Thank you, Chris! On 25.05.2016 16:08, Chris Hegarty wrote: On 25 May 2016, at 14:04, Svetlana Nikandrova wrote: 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/~snikandrov

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

On 25 May 2016, at 14:04, Svetlana Nikandrova wrote: > > 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/ >

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

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/ Tested as standalone 1 test ma

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

> On 24 May 2016, at 17:21, Svetlana Nikandrova > 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.

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

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

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

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/

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

> On 20 May 2016, at 14:10, Alan Bateman 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

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

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) 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

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

Alan, another test related to this option is on the same level (test/jdk/net/SocketFlow) 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 confusi

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

On 20/05/2016 11:16, Svetlana Nikandrova wrote: Hi Artem, thank you for your comments. Please see updated review: http://cr.openjdk.java.net/~snikandrova/8136933/webrev.01/ I've moved version check to OSInfo.java and used Process

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

Hi Artem, thank you for your comments. Please see updated review: http://cr.openjdk.java.net/~snikandrova/8136933/webrev.01/ I've moved version check to OSInfo.java and used ProcessTools. Also made a little change in ProcessTools

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

Hi Svetlana, Please see a couple of comments below. I'll leave the final review to official reviewers. 1. You may use test/lib/testlibrary/jdk/testlibrary/OSInfo.java to check OS type. I think it also may be good to add getSolarisVersion() to OSInfo.java (see getWindowsVersion() method) Yo