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