RFR 8234823: java/net/Socket/Timeouts.java testcase testTimedConnect2() fails on Windows 10
Could I get the following two trivial test case fixes reviewed please? They are both caused by a recent configuration change to Windows 10 where socket connect requests are attempted four times, at 500ms intervals, on connection refused errors, which means that such connects will take around 2 seconds to return to the caller. The two tests need to increase their timeout from 2 sec to 3 sec. 8234823: java/net/Socket/Timeouts.java testcase testTimedConnect2() fails on Windows 10 http://cr.openjdk.java.net/~michaelm/8234823/webrev.1/ 8234823: java/nio/channels/SocketChannel/AdaptSocket.java fails on Windows 10 http://cr.openjdk.java.net/~michaelm/8234824/webrev.1/ There are links to the bug reports from the webrevs. Thanks, Michael.
Re: RFR 8234823: java/net/Socket/Timeouts.java testcase testTimedConnect2() fails on Windows 10
On 03/12/2019 09:59, Michael McMahon wrote: : 8234823: java/net/Socket/Timeouts.java testcase testTimedConnect2() fails on Windows 10 http://cr.openjdk.java.net/~michaelm/8234823/webrev.1/ For this one, it should be okay to increase the timeout to a much larger timeout if you want. For example, if the connect timeout is 10s then it will still fail with ConnectException, you'll only get the SocketTimeoutException if it were to exceed 10s. 8234823: java/nio/channels/SocketChannel/AdaptSocket.java fails on Windows 10 http://cr.openjdk.java.net/~michaelm/8234824/webrev.1/ I think this one should be okay to increase to a much larger value too. -Alan
Re: RFR 8234823: java/net/Socket/Timeouts.java testcase testTimedConnect2() fails on Windows 10
Thanks Alan. Yes, I think it's reasonable to set a much larger timeout. The actual delay experienced (annoying as it is) will still be only two seconds. I'll go ahead with that. Michael. On 03/12/2019 11:08, Alan Bateman wrote: On 03/12/2019 09:59, Michael McMahon wrote: : 8234823: java/net/Socket/Timeouts.java testcase testTimedConnect2() fails on Windows 10 http://cr.openjdk.java.net/~michaelm/8234823/webrev.1/ For this one, it should be okay to increase the timeout to a much larger timeout if you want. For example, if the connect timeout is 10s then it will still fail with ConnectException, you'll only get the SocketTimeoutException if it were to exceed 10s. 8234823: java/nio/channels/SocketChannel/AdaptSocket.java fails on Windows 10 http://cr.openjdk.java.net/~michaelm/8234824/webrev.1/ I think this one should be okay to increase to a much larger value too. -Alan
Re: RFR: 8235141: Specify the required standard socket options for the socket types in the java.net package
Thanks for the review Alan! new webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8235141/webrev.01/ On 02/12/2019 15:53, Alan Bateman wrote: 8235141: Specify the required standard socket options for the socket types in the java.net package https://bugs.openjdk.java.net/browse/JDK-8235141 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8235141/webrev.00/ A few suggestion for wording: "offers some convenience methods to get and set some commonly used options" -> "defines convenience methods to set and get several socket options". Done. "However, socket options can be more generally configured using the setOption method" -> "This class also defines the setOption and getOption methods to set and query socket options". Done. The tables looks right. The RequiredOptions appears to overlap significantly with existing tests, e.g. test/jdk/java/nio/channels already has a SocketOptionTests test for each of the network channel types. It's true that there might be some overlap. This test is more about proving consistency between the doc and the different implementations provided by the JDK. It's too easy for something to fall through the gaps otherwise. best regards, -- daniel -Alan
Re: RFR: 8235141: Specify the required standard socket options for the socket types in the java.net package
On 03/12/2019 16:39, Daniel Fuchs wrote: It's true that there might be some overlap. This test is more about proving consistency between the doc and the different implementations provided by the JDK. It's too easy for something to fall through the gaps otherwise. The javadoc update looks good. I guess the test is okay but it's not really the location to test that the implementations of NetworkChannel support the socket options that they specify. The tests in nio/channels already do this, additionally testing extended socket options. -Alan
