On Wed, 24 Mar 2021 16:19:53 GMT, Jonathan Gibbons <j...@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
