RFR 8241305: Add protocol specific factory creation methods to SocketChannel and ServerSocketChannel

[Alan Bateman]

On 13/05/2020 22:51, Michael McMahon wrote:
> I have updated the webrev for this at 
> http://cr.openjdk.java.net/~michaelm/8241305/webrev.4/
> based on the last round of comments.
LocalSocketAddressType - looks good but not clear why it re-runs with 
preferIPv6Addresses=true.

OpenAndConnect - looks much better, a few comments:

The use of ignoreTest in each of the parameterized tests is a bit of 
anti-pattern. The data provider should filter the test cases so that the 
test aren't invoked with untestable parameters. That should avoid lots 
of skipped tests too.

There are several places where exceptions are being caught and the 
error(Exception) method used to re-thrown them. I think error(Exception) 
can be removed and just let TestNG handle the failures.

There are a few duplicate tests: L138 and L139, L144 and L145, L195 and 
L196.

The tests that check for DONT_BIND would be a bit clear if they created 
"csa" after caddr != DONT_BIND, e.g.

if (caddr != DONT_BIND) {
     dc.bind(getSocketAddress(caddr);
}


ProtocolFamilies - looks much better, a few comments:

isWindows can be removed.

I think the data providers could be a bit clearer if they used if 
(hasIPv6 && !preferIPv4) { ... }.
Do you intend to keep the commented out test cases in the openConnect 
data provider?

getLocalIPv6Address looks like it could return a loopback address, 
filter step missing? BTW: if you change ia4 and ia6 to be InetAddress 
then it should eliminate the casts in all places.

scOpenConnect, the connectOp isn't really needed, simpler to just call 
sc.connect if expectedException is null.

dcOpen names the parameter "sfamily", typo? The other methods use "family".


I think that is all I have for this round.

-Alan.