Skip to content

Add JaCoCo coverage measurement and Codecov reporting - #1553

Merged
papegaaij merged 1 commit into
masterfrom
add-code-coverage
Aug 24, 2026
Merged

Add JaCoCo coverage measurement and Codecov reporting#1553
papegaaij merged 1 commit into
masterfrom
add-code-coverage

Conversation

@papegaaij

Copy link
Copy Markdown
Contributor

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 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

The 'coverage' profile produced no data at all. maven-surefire-plugin's
pluginManagement set a literal <argLine> for the --add-opens flags, which
overrode the argLine property that jacoco:prepare-agent sets, so the agent
never attached to the forked test JVMs.

Surefire now consumes @{jacoco.argLine}, substituted at fork time. That
placeholder property must stay declared even though it is empty: surefire
only substitutes @{x} for properties that exist, and would otherwise hand
the literal token to the JVM and break every test module whenever the
profile is inactive.

Per-module reports would have been misleading too, because most tests live
in a module other than the code they exercise: wicket-core has no tests of
its own, and wicket-core-tests has no production classes. The new
wicket-coverage module aggregates the reactor with jacoco:report-aggregate
instead, using dependency scope to say what belongs in the report -- compile
contributes classes and sources, test contributes execution data only. The
per-module 'report' execution is dropped.

Coverage is measured on the JDK 21 leg of the existing build and uploaded to
Codecov for every push and pull request. It is reported, never enforced:
codecov.yml marks both status checks informational, so neither can fail a
build or block a merge.

check-coverage-report.py guards the measurement itself. Should a future
<argLine> override drop the placeholder, or a dependency scope change,
coverage would silently fall to zero rather than fail; the script asserts
the module set and non-zero coverage for the three cross-module cases, but
never a percentage.

Current aggregate: 67.5% of instructions, 67.4% of lines.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@papegaaij
papegaaij merged commit 5de29d1 into master Aug 24, 2026
3 checks passed
@papegaaij
papegaaij deleted the add-code-coverage branch August 24, 2026 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant