RFR: JDK-8262899: TestRedirectLinks fails [v4]

[Pavel Rappo]

On Wed, 24 Mar 2021 16:19:53 GMT, Jonathan Gibbons <jjg at openjdk.org> wrote:

>> Please review a trivial change to fix this test when run behind a firewall with a proxy set.
>> 
>> Previously, the test used `InetAddress.getLocalHost` which returns an IP address for the current host.  It runs a temporary local web server on that address, but since the address is typically not on the `nonProxyHosts` list, the attempted access to the web server goes to the proxy, and fails.
>> 
>> The fix is to explicitly use `127.0.0.1`, which should never go to a proxy, if one is set.
>
> Jonathan Gibbons has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Disable problem test case by default, unless enabled by system property

Marked as reviewed by prappo (Reviewer).

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java line 388:

> 386: 
> 387:         Fault(String msg, Exception cause) {
> 388:             super(msg + (cause == null ? "" : " (" + cause + ")"), cause);

Although this message concatenation captures the immediate cause of the `Fault`, it doesn't capture those deeper exceptions that might be present in the chain. Moreover, such concatenation is required in all places where the cause is to be preserved.

A better way to do it would be to derive diagnostic information from all the exceptions in the chain on the handling site (i.e. wherever this exception is caught). Later we should consider improving `jdk.javadoc.doclet.Reporter.print` instead of introducing this concatenation.

test/langtools/jdk/javadoc/doclet/testLinkOption/TestRedirectLinks.java line 91:

> 89:     public void testRedirects() throws Exception {
> 90:         // This test relies on access to an external resource, which may or may not be
> 91:         // reliably available, depending on the shost system configuration and other

Typo: shost

test/langtools/jdk/javadoc/doclet/testLinkOption/TestRedirectLinks.java line 110:

> 108:         } catch (UnknownHostException e) {
> 109:             out.println("Setup failed (" + testURLHost + " not found); this test skipped");
> 110:             return;

L98 and L110: can we use `jtreg.SkippedException` instead? That way the results of the test test run might look more clearly.

test/langtools/jdk/javadoc/doclet/testLinkOption/TestRedirectLinks.java line 196:

> 194:                 "--module-source-path", libSrc.toString(),
> 195:                 "--module", "mA,mB",
> 196:                 "-Xdoclint:none" );

Why did you add `-Xdoclint:none`?

-------------

PR: https://git.openjdk.java.net/jdk/pull/3137