papegaaij opened a new pull request, #1553:
URL: https://github.com/apache/wicket/pull/1553
Wicket had no coverage numbers and no way to see them. It turned out the
measurement side was already half-present but broken in two independent ways,
so this fixes both and then wires the result up to a UI.
## Why `-Pcoverage` produced nothing
**The agent never attached.** `maven-surefire-plugin`'s `pluginManagement`
set a *literal* `<argLine>` for the `--add-opens` flags. `jacoco:prepare-agent`
works by setting an `argLine` *property*, and an explicit `<argLine>` element
wins over it, so the whole suite ran uninstrumented and no `jacoco.exec` was
ever written.
Surefire now consumes `@{jacoco.argLine}`, substituted at fork time. The
placeholder property has to stay declared even though it is empty: surefire
only substitutes `@{x}` for properties that actually exist, and would otherwise
hand the literal token to the JVM and break every test module whenever the
profile is inactive. It is deliberately named `jacoco.argLine` rather than the
bare `argLine`, because `argLine` is also surefire's own parameter expression —
a reactor-wide `<argLine />` property would pin surefire's fallback everywhere
and turn `mvn -DargLine=-Xmx4g` into a silent no-op.
**Per-module reports would have been misleading anyway.** Most tests live in
a module other than the code they exercise: `wicket-core` has 842 main classes
and no tests, while `wicket-core-tests` has ~500 test classes and no production
code. Same split for `wicket-cdi`/`wicket-cdi-tests`. Per-module JaCoCo reports
`wicket-core` at 0%.
So the per-module `report` execution is dropped in favour of a new
`wicket-coverage` module that aggregates the reactor with
`jacoco:report-aggregate`. Its dependency list *is* the configuration —
`report-aggregate` reads dependency scope, where `compile` contributes classes
and sources and `test` contributes execution data only. That one distinction is
what lets `wicket-core` get credit for tests that live elsewhere.
Two subtleties in that pom, both called out in comments:
- `wicket-tester` is managed to `<scope>test</scope>` in the parent, and
scope defaulting runs *after* management injection, so it needs an explicit
`<scope>compile</scope>` or its classes silently vanish from the report.
- `report-aggregate` has no default phase, and with no execution data it
happily emits a well-formed **0%** `jacoco.xml`. It is therefore gated on the
same `coverage` profile that attaches the agent, so "the report exists" implies
"the agent ran".
## Reporting
Coverage is measured on the **JDK 21 leg of the existing build** and
uploaded to Codecov for every push and pull request. Riding on the existing
build means the only marginal cost is load-time instrumentation on one leg,
rather than a second full test run per commit on shared ASF runners. Running on
both `push` and `pull_request` is deliberate: Codecov needs coverage on the
base commit to compute a meaningful diff.
**Coverage is reported, never enforced.** `codecov.yml` marks both status
checks `informational`, so they show real numbers on a PR but cannot fail a
build or block a merge. `require_changes: true` keeps the bot quiet on PRs that
do not move coverage.
On ASF policy: `codecov/codecov-action` is already blanket-approved on the
allowlist in `apache/infrastructure-actions`, so no security review is needed,
and it is pinned to a commit SHA as [the
policy](https://infra.apache.org/github-actions-policy.html) requires. Fork PRs
get no repository secrets and so upload tokenlessly, which Codecov supports for
public upstreams — note the comment in the workflow warning against "fixing"
that with `pull_request_target`. `fail_ci_if_error` is left at its default of
`false` on purpose, so a Codecov outage cannot redden every push.
## The tripwire
If a future `<argLine>` override forgets the placeholder, or a dependency
scope changes, coverage falls silently to zero instead of failing — it would
show up only as an unexplained cliff on the trend line.
`.github/scripts/check-coverage-report.py` runs before the upload and asserts
the expected module set plus non-zero coverage for the three cross-module
cases. It deliberately checks structure, never a percentage: it is a
correctness check on the measurement, not a quality gate.
## Results
`mvn clean verify -Pcoverage` on the full reactor, JDK 21:
```
wicket-core 74.1% (88,444 / 119,289 instructions)
wicket-request 88.2% wicket-cdi 88.6%
wicket-ioc 84.3% wicket-guice 83.6%
wicket-spring 81.5% wicket-tester 80.7%
wicket-nws-tester 80.1% wicket-bean-validation 79.5%
wicket-auth-roles 70.8% wicket-util 60.6%
wicket-extensions-tester 60.0% wicket-nws-core 58.6%
wicket-velocity 56.5% wicket-extensions 43.9%
wicket-devutils 6.3% wicket-jmx / nws-javax 0.0%
─────────────────────────────────────────────────────────────────────
TOTAL 67.5% instructions · 67.4% lines · 63.6% branches
```
The two zeroes are legitimate: `wicket-jmx` and
`wicket-native-websocket-javax` contain only `ApacheLicenceHeaderTest`, which
reads license headers and never exercises its own module's classes. Their exec
files do exist, so the agent attached.
Verified locally:
- With the profile on, the forked JVM receives `-javaagent:` **and** all
five `--add-*` flags; with it off, the placeholder resolves to nothing and no
exec file appears.
- The serialization tests in `wicket-core-tests` — the ones that fail hard
without `--add-opens java.base/java.lang` — pass with the profile **off**,
which is what the JDK 25/26 legs will do.
- Full reactor build succeeds in 3:45 with 20 non-empty exec files.
- `maven-enforcer-plugin` passes on the new module with no
`dependencyManagement` pins needed; `dependencyConvergence` excludes test and
provided scopes by default, which prunes the `wicket-core-tests` subtree
entirely.
- `codecov.yml` is accepted by Codecov's own validator.
## Not done here
There is no JIRA ticket for this yet — happy to file one and retitle if the
PMC would prefer that. Left for follow-ups: the same treatment on `wicket-10.x`
(its `coverage` profile has the identical defect), a `jacoco.version` bump from
0.8.15 to 0.8.16, and a README badge.
Worth a note on dev@ either way, since this adds a third-party service to
the CI surface and puts a bot comment on pull requests.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
