On 24/01/2019, 14:38, Alan Bateman wrote:
On 24/01/2019 13:09, Michael McMahon wrote:
I've updated the webrev at
http://cr.openjdk.java.net/~michaelm/8216986/webrev.2/
to add some tests and also I found the same dubious implementation of
getLocalPort()
in HttpConnectSocketImpl.java. The test infrastructure needed some
cleanup also.
The changes to SocksSocketImpl and in HttpConnectSocketImpl look good
to me.
The test changes look okay too. A potential improvement is to have
SocksServer implement Closeable and rename its terminate method to
close as that would allow the tests to use try-with-resources and
ensure connections are closed in the event of an exception. Not
important if you don't want to spend time on it of course.
-Alan
Yeah, considering the test runs in samevm mode, it would be better to
ensure full cleanup.
I will push after changing that.
Thanks!
Michael.
