ascheman opened a new pull request, #1334:
URL: https://github.com/apache/maven-javadoc-plugin/pull/1334

   ## Summary
   
   The `MJAVADOC-427` IT verifies that the plugin follows HTTP redirects when
   resolving an external `-link` target. It does so against the live
   `http://www.slf4j.org/`, whose URL is `http://` and whose CDN redirects to
   `https://`. That external dependency has been a persistent CI flake.
   
   This PR replaces the slf4j.org probe with a localhost
   `com.sun.net.httpserver.HttpServer` started by `setup.groovy` on an
   OS-allocated free port (bind-to-0). The mock issues a `301` from
   `/element-list` to `/redirected/element-list` and serves a minimal
   `element-list` containing `org.slf4j`. A hit-counter written by the
   mock's handlers lets `verify.groovy` directly assert that both endpoints
   were probed — i.e. that the plugin actually followed the redirect, which
   is a *stronger* signal than the original IT's "a link appears in the
   HTML" (which only inferred redirect-following from absence-of-broken-link).
   
   ## Why
   
   The `Error fetching link: http://www.slf4j.org/apidocs. Ignored it.` flake
   has been masters' problem for some time. Recent reproductions on the fork's
   full CI matrix:
   
   | Run | Result | Failing cells |
   |---|---|---|
   | master `c684cb50` (current upstream tip) | 3 failures / 51 | `windows + 
jdk-17-temurin`, `windows + jdk-21-adopt-openj9`, `windows + jdk-21-liberica` |
   | this branch (`d0863d6d`) | **51 / 51 green** | — |
   | this branch (`04fa2d7b`, fixed-port earlier iteration) | **51 / 51 green** 
| — |
   
   The failures span OpenJ9 *and* mainstream HotSpot distributions (Temurin,
   Zulu, Liberica) — i.e. not a JVM-specific quirk, but a CDN-egress problem
   hitting whoever's runner pool draws an unlucky network slot. Historical
   master runs that hit the same `MJAVADOC-427` failure (for evidence the
   flake is pre-existing, not introduced here):
   
   - https://github.com/apache/maven-javadoc-plugin/actions/runs/26077453173 
(2026-05-19)
   - https://github.com/apache/maven-javadoc-plugin/actions/runs/25874617489 
(2026-05-14)
   
   Local fork validation runs (matching the upstream matrix exactly):
   
   - master baseline (slf4j.org dep): 
https://github.com/aschemaven/maven-javadoc-plugin/actions/runs/27460422862 (3 
failures)
   - this branch dynamic port: 
https://github.com/aschemaven/maven-javadoc-plugin/actions/runs/27464388990 
(51/51 green)
   
   ## Coverage trade-off
   
   | | Original IT | This IT |
   |---|---|---|
   | Redirect (`301`) followed by the plugin's `-link` resolver | ✓ (inferred) 
| **✓ (directly asserted via hit log)** |
   | Cross-reference URL constructed from the redirected `element-list` | ✓ | ✓ 
|
   | `http://` → `https://` protocol switch specifically | ✓ (incidental) | ✗ 
(intentionally dropped) |
   | Deterministic across all CI cells | ✗ (CDN flake) | ✓ |
   
   The `http → https` coverage is intentionally dropped. That transition
   happens inside the JDK's `HttpURLConnection` implementation, not in
   `maven-javadoc-plugin`. The plugin is protocol-agnostic at its own layer
   and delegates redirect transport to the JDK. Reproducing the protocol
   switch on a localhost mock would require an HTTPS listener with a
   self-signed certificate and JVM truststore plumbing — a lot of
   scaffolding for coverage of code that is not the plugin's responsibility.
   
   ## Implementation notes
   
   ### Why `com.sun.net.httpserver.HttpServer`?
   
   Despite the `com.sun.*` package prefix, `com.sun.net.httpserver` is a
   **public, supported** JDK API — not an internal one. It lives in the
   `jdk.httpserver` module (since Java 9), is documented at oracle.com under
   the standard module reference, and has had a stable API contract since
   Java 6 (2006). The `com.sun.*` prefix is purely historical naming —
   Oracle never moved the package to avoid breaking existing users.
   
   It is shipped in every JDK distribution we tested on (Temurin, Zulu,
   Liberica, Microsoft, **adopt-openj9**) across JDK 8, 17, 21, and 25 — see
   the 51/51 green run on `d0863d6d`. Using it keeps the IT self-contained
   (zero added Maven dependencies, no embedded Jetty/Netty/etc.) and avoids
   writing bespoke HTTP request parsing in Groovy.
   
   ### Server lifecycle
   
   The mock server is **never explicitly stopped**; cleanup is handled by
   JVM-exit semantics:
   
   - Worker threads: `setup.groovy` installs a `ThreadFactory` that marks
     every executor thread as daemon.
   - Dispatcher (accept) thread: the JDK's `ServerImpl` marks its own
     `HTTP-Dispatcher` thread daemon since Java 9.
   
   When `mvn verify` finishes and the invoker JVM exits, all daemon threads
   terminate and the port is released. No hangs observed across the 51-cell
   matrix run.
   
   Explicit `server.stop()` would require sharing the `server` reference
   between `setup.groovy` and `verify.groovy`. The two scripts run in
   separate `GroovyShell` invocations with separate bindings; passing state
   between them would need either a static holder helper or a `metaClass`
   trick, both more invasive than the daemon-thread pattern they'd replace.
   
   ### Other design choices
   
   - **Dynamic port** via `HttpServer.create(new InetSocketAddress(0), 0)` —
     no fixed-port collision risk on developer machines or CI runners.
   - **Idempotent setup**: `maven-invoker-plugin` runs each IT twice (once
     for the `integration-test` mojo, once for `verify`). The second
     invocation detects the already-patched `pom.xml` placeholder and returns
     silently; the daemon-threaded server from the first invocation
     remains live.
   - **Hit log**: `${basedir}/redirect-hits.log` accumulates `base ...` and
     `redirected ...` lines from each handler. `verify.groovy` asserts both
     are present.
   
   ## Test plan
   
   - [x] `mvn -P run-its -Dinvoker.test=MJAVADOC-427 verify` passes locally
         on macOS Temurin JDK 17 with the OS allocating different free ports
         on successive runs.
   - [x] Hit log on local run contains both `base /element-list` and
         `redirected /redirected/element-list`.
   - [x] Fork CI: 51/51 green on this branch (no regressions across the
         full matrix — JDK 8/17/21/25, all 5 vendors, Linux/macOS/Windows).
   - [x] Fork CI on `master` reproduces the slf4j.org flake (3 failures on
         `MJAVADOC-427`) — confirms the change actually addresses the
         flake, not just hides it.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to