On Tue, 25 Feb 2025 08:57:00 GMT, SendaoYan <s...@openjdk.org> wrote:
> Hi all, > > Two java/net/InetAddress tests fails "java.net.UnknownHostException" on some > special machines. The machine cannot connect to the Internet directly and > needs to be connected to the Internet through a jump machine. The > java/net/InetAddress tests report "java.net.UnknownHostException: > bugs.openjdk.org: Temporary failure in name resolution" failure do not caused > by JVM bug but environmentalk issue, so this PR match the > java.net.UnknownHostException and throw jtreg.SkippedException instead of > report test fails. > > And run the tests with JVM options `-Dhttp.proxyHost=192.168.50.1 > -Dhttp.proxyPort=10991 -Dhttps.proxyHost=192.168.50.1 -Dhttps.proxyPort=10991 > -DsocksProxyHost=192.168.50.1 -DsocksProxyPort=10991` can not work around. So > I create this PR to fix the test bug. > > Command wget shows that the machine should connect to internet through a > jumper machine(192.168.50.1:10991): > >> wget www.bing.com > --2025-02-24 17:56:25-- http://www.bing.com/ > Connecting to 192.168.50.1:10991... connected. > Proxy request sent, awaiting response... 301 Moved Permanently > Location: http://cn.bing.com/ [following] > --2025-02-24 17:56:25-- http://cn.bing.com/ > Reusing existing connection to 192.168.50.1:10991. > Proxy request sent, awaiting response... 200 OK > Length: 13006 (13K) [text/html] > Saving to: ‘index.html’ > > > Change has been verified locally, test-fix only and make tests more > robustness, no risk. Overall looks good to me, just minor cleanup comments and a question about `@build jtreg.SkippedException` test/jdk/java/net/InetAddress/IsReachableViaLoopbackTest.java line 26: > 24: import java.io.*; > 25: import java.net.*; > 26: import java.util.*; I know, this is not a part of your change, but this import is not used. Could you please remove it? test/jdk/java/net/InetAddress/IsReachableViaLoopbackTest.java line 35: > 33: * @key intermittent > 34: * @library /test/lib > 35: * @build jtreg.SkippedException AFAIK,`@build jtreg.SkippedException` is not needed. Tried to remove and it worked fine locally, does it work for you? Also this is the same for the line 30 in the `getOriginalHostName.java` test/jdk/java/net/InetAddress/IsReachableViaLoopbackTest.java line 64: > 62: } catch (java.net.UnknownHostException e) { > 63: e.printStackTrace(); > 64: throw new SkippedException("Network setup issue, skip this > test"); Minor: Do you think it would make the comment cleaner if it would be just `"Network setup issue"`? It seems to be repeating itself for me (skip exception already states, that the test is skipped). Don't have a strong opinion, so it's completely up you :) Also this is the same for the line 60 in the `getOriginalHostName.java` ------------- PR Review: https://git.openjdk.org/jdk/pull/23767#pullrequestreview-2653693064 PR Review Comment: https://git.openjdk.org/jdk/pull/23767#discussion_r1977263320 PR Review Comment: https://git.openjdk.org/jdk/pull/23767#discussion_r1977254299 PR Review Comment: https://git.openjdk.org/jdk/pull/23767#discussion_r1977283080