Hi Arthur,
On the surface, this looks good.
However I have a concern about two things:
1. IPSupport needs to read system properties, attempts
to bind sockets etc... I wonder how that might interact
with tests that use a security manager, as some of these
operations may throw a SecurityException.
Maybe some double checking would be in order for those.
2. I would also be more comfortable if
skipIfCurrentConfigurationIsInvalid() (and possibly
the static initializer too?), could throw an
AssertionError if it finds that neither IPv4 nor IPv6
are available on a system. I'm concerned that a bug
that e.g. prevents to bind any socket would go unnoticed
if all tests are skipped...
best regards,
-- daniel
PS: Just a heads up that I have a pending fix [1] which I'm intending
to push later today and which also touches some of the tests from
your patch. Hopefully any merge should be trivial.
[1] https://mail.openjdk.java.net/pipermail/net-dev/2019-April/012416.html
On 01/05/2019 23:21, Arthur Eubanks wrote:
Sorry for the delay, but here's the next revision:
http://cr.openjdk.java.net/~aeubanks/8220673/webrev.01
The list of tests modified was found by searching for "preferIPv4Stack"
in the tests. The only one I didn't touch was
test/jdk/sun/net/sdp/sanity.sh, which is a shell test, and is Solaris-only.
For most tests, all I did was add
IPSupport.skipIfCurrentConfigurationIsInvalid(). Some of them also had
custom implementations for detecting if IPv6 was available, so I
replaced those with the new IPSupport version.
Also added a missing "@test" to test/jdk/java/net/Socket/RST.java.
Again, I made sure that the only time the test is skipped
with IPSupport.skipIfCurrentConfigurationIsInvalid() is when IPv4 is not
available (with LD_PRELOAD) and when java.net.preferIPv4Stack is true.
There are still tests that fail/timeout under the LD_PRELOAD that
intercepts and rejects IPv4 calls to getifaddrs(), setsockopt(), and
socket(), but those will be fixed in the future, likely with changes to
the JDK itself.
