HADOOP-15190. Replace Clover with JaCoCo for code coverage#8606
Open
ferdelyi wants to merge 2 commits into
Open
HADOOP-15190. Replace Clover with JaCoCo for code coverage#8606ferdelyi wants to merge 2 commits into
ferdelyi wants to merge 2 commits into
Conversation
OpenClover (the open-sourced fork of Atlassian Clover) is incompatible
with Java 17+ because it relies on source-level instrumentation via a
javac annotation processor that triggers split-package violations in the
module system. JaCoCo uses bytecode instrumentation via a Java agent
and has no such constraint.
Changes:
- pom.xml: remove com.cenqua.clover dependency management, replace
clover-maven-plugin in pluginManagement with jacoco-maven-plugin
0.8.15, add the hadoop-coverage aggregate module, drop the
<profile id="clover"> block entirely, and declare
hadoop.skip-jacoco=true here so modules that inherit directly from
hadoop-main (e.g. hadoop-coverage) resolve the property too
- hadoop-project/pom.xml: add hadoop.skip-jacoco=true property (opt-in
by default), append @{argLine} to the surefire and failsafe argLines
so the JaCoCo agent is injected into every forked test JVM, and wire
in jacoco:prepare-agent with excludes for generated/proto/example
classes so they are not instrumented
- per-module poms that override the surefire argLine (hadoop-common,
hadoop-hdfs, hadoop-hdfs-rbf, hadoop-registry, hadoop-aws,
hadoop-azure, hadoop-distcp, hadoop-federation-balance, hadoop-gcp,
hadoop-tos, hadoop-yarn-applications-catalog-webapp): append
@{argLine} so instrumentation is not dropped
- hadoop-coverage/pom.xml (new): aggregate module that depends on the
test-bearing modules and runs jacoco:report-aggregate at the verify
phase to produce a combined HTML/XML report under
hadoop-coverage/target/site/jacoco-aggregate/; deploy and install are
skipped so this coverage-only pom is never published
- hadoop-maven-plugins/pom.xml,
hadoop-yarn-server-nodemanager/pom.xml: remove stale clover skip
overrides that are no longer needed
- test-libhdfs.sh, hadoop-mapreduce-client-jobclient/pom.xml,
hadoop-yarn-applications/pom.xml: drop remaining clover references
report-aggregate only sees modules within the same Maven reactor, so
the report must be produced by a single full-reactor build (not with
-pl hadoop-coverage, which would leave the other modules out of the
reactor and yield an empty report):
mvn verify -Dhadoop.skip-jacoco=false -Dmaven.test.failure.ignore=true
The aggregate report lands at
hadoop-coverage/target/site/jacoco-aggregate/index.html.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ferdelyi
marked this pull request as ready for review
July 17, 2026 12:52
K0K0V0K
reviewed
Jul 17, 2026
Contributor
There was a problem hiding this comment.
Thanks @ferdelyi for this fix!
Overall seems good, i just had some question.
Also can you please check other parts of the code where clover is mentioned?
If i see correctly the BUILDING.txt still has a line like
- Run clover : mvn test -Pclover
UPDATE:
- sry, i checked wrong code, now i see BUILDING.txt is updated also
| <groupId>org.apache.hadoop</groupId> | ||
| <artifactId>hadoop-coverage</artifactId> | ||
| <version>3.6.0-SNAPSHOT</version> | ||
| <description>Hadoop JaCoCo Aggregate Coverage</description> |
Contributor
There was a problem hiding this comment.
I think the artifact-id and the description may get some problem in future if we will have to migrate to a next coverage lib. In that case maybe the best option will be to create a new project like this like 'Hadoop xy Aggregate Coverage' and than the previous hadoop-coverage name could be confusing ...
What do you think?
- Can we remove the JaCoCo from the description?
- or can we rename the artifact to hadoop-jacoco-coverage?
Contributor
Author
There was a problem hiding this comment.
Thank you for your review @K0K0V0K , removed the JaCoCo from the description.
| <artifactId>hadoop-yarn-services-api</artifactId> | ||
| <version>${project.version}</version> | ||
| </dependency> | ||
| </dependencies> |
Contributor
There was a problem hiding this comment.
If i see correctly now if a new artifact created in the project it should be added here also.
I think this can be a part of the required changes what can be easily missed.
Is there any other way how we can prevent to list every artifact here?
Contributor
Author
There was a problem hiding this comment.
Great point @K0K0V0K , extended the PR with a build-time guard.
Contributor
There was a problem hiding this comment.
Thanks, @ferdelyi! This looks much better.
I noticed a very similar solution in hadoop-client-check-test-invariants. I think it would be nice to move the shell script into the new module as well, so the related code is kept together.
…e and guard its module list Follow-up to the Clover->JaCoCo change, addressing PR review comments. - hadoop-coverage/pom.xml: rename the module to the tool-agnostic "Hadoop Aggregate Code Coverage" (keeping the generic hadoop-coverage artifactId) so it need not be renamed if the coverage tool ever changes. - Add a build guard so the aggregate's module list cannot silently drift: dev-support/bin/check-coverage-modules.sh scans the source tree for test-bearing modules (jar packaging with a src/test/java directory) and fails the build if any is neither listed as a dependency in hadoop-coverage/pom.xml nor named in dev-support/coverage-modules-allowlist.txt. It is wired via exec-maven-plugin at the validate phase, independent of hadoop.skip-jacoco, so it runs on every build that includes the module. - dev-support/coverage-modules-allowlist.txt (new): document the intentional exclusions (hadoop-mapreduce-examples, already excluded via the **/examples/** class pattern; hadoop-client-integration-tests, a test-only module). - hadoop-coverage/pom.xml: fix the hadoop-yarn-applications-mawo-core dependency groupId (org.apache.hadoop -> org.apache.hadoop.applications.mawo) so it resolves the reactor module instead of a stale published artifact. - pom.xml, hadoop-project/pom.xml: manage exec-maven-plugin in the root pluginManagement (hadoop-coverage inherits from hadoop-main, which did not manage it) and drop the now-duplicate declaration from hadoop-project so the version has a single source of truth. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Trunk supports Java 17 (https://github.com/apache/hadoop-release-support). OpenClover (the open-sourced fork of Atlassian Clover) is incompatible with Java 17+ because it relies on source-level instrumentation via a javac annotation processor that triggers split-package violations in the module system. JaCoCo uses bytecode instrumentation via a Java agent and has no such constraint.
Description of PR
Changes:
hadoop.skip-jacoco=true here so modules that inherit directly from hadoop-main (e.g. hadoop-coverage) resolve the property too
hadoop-tos, hadoop-yarn-applications-catalog-webapp): append @{argLine} so instrumentation is not dropped
hadoop-coverage/target/site/jacoco-aggregate/; deploy and install are skipped so this coverage-only pom is never published
overrides that are no longer needed
report-aggregate only sees modules within the same Maven reactor, so the report must be produced by a single full-reactor build (not with -pl hadoop-coverage, which would leave the other modules out of the
reactor and yield an empty report):
mvn verify -Dhadoop.skip-jacoco=false -Dmaven.test.failure.ignore=true
The aggregate report lands at hadoop-coverage/target/site/jacoco-aggregate/index.html.
Generated-by: Claude Code
How was this patch tested?
Run on my local computer coverage report for hadoop_nfs and also an aggregate coverage report.
Screenshots provided on the Jira as attachments.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?AI Tooling
If an AI tool was used:
where is the name of the AI tool used.
https://www.apache.org/legal/generative-tooling.html