Skip to content

TEZ-4693: Migrate from proprietary atlassian clover code coverage plugin to JaCoCo - #465

Merged
abstractdog merged 1 commit into
apache:masterfrom
Aggarwal-Raghav:TEZ-4693
Mar 19, 2026
Merged

TEZ-4693: Migrate from proprietary atlassian clover code coverage plugin to JaCoCo#465
abstractdog merged 1 commit into
apache:masterfrom
Aggarwal-Raghav:TEZ-4693

Conversation

@Aggarwal-Raghav

@Aggarwal-RaghavAggarwal-Raghav commented Mar 16, 2026

Copy link
Copy Markdown
Contributor

Code coverage using jacoco:

GO to tez-api/target/site/jacoco and run python -m http.server 8080 to see the report on http://localhost:8080/

Screenshot 2026-03-16 at 10 19 53 PM

Comment threadtez-dag/pom.xml Outdated
<configuration>
<argLine>${test.jvm.args}</argLine>
<environmentVariables>
<LOG_DIRS>${test.log.dir}</LOG_DIRS>

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no need to explicit surefire plugins in child pom. Moved LOG_DIRS to parent pom and used <environmentVariables combine.children="append"> to ensure the argline and other env variables defined in parent pom under surefire plugins are available to child pom.

Comment threadpom.xml Outdated
<forkedProcessTimeoutInSeconds>900</forkedProcessTimeoutInSeconds>
<testFailureIgnore>true</testFailureIgnore>
<argLine>-Xmx1024m -XX:+HeapDumpOnOutOfMemoryError</argLine>
<argLine>${test.jvm.args}</argLine>

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having 2 argline is a BUG. The test.jvm.args is overwriting the above argLine i.e. Xmx one
Screenshots:
Before:
Image

After:
Image

Comment threadpom.xml Outdated
<testFailureIgnore>true</testFailureIgnore>
<argLine>-Xmx1024m -XX:+HeapDumpOnOutOfMemoryError</argLine>
<argLine>${test.jvm.args}</argLine>
<argLine>@{argLine} -Xmx1024m -XX:+HeapDumpOnOutOfMemoryError -XX:+EnableDynamicAgentLoading ${test.jvm.args}</argLine>

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added @{argLine} because of https://www.jacoco.org/jacoco/trunk/doc/prepare-agent-mojo.html
and -XX:+EnableDynamicAgentLoading because of jdk 21 + mockito (even though I have not seen the warning in tez) but adding it is shouldn't be a problem.

Comment threadpom.xml
<configuration>
<rules>
<rule>
<element>BUNDLE</element>

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 11sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+0 🆗xmllint0m 0sxmllint was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+0 🆗mvndep2m 17sMaven dependency ordering for branch
+1 💚mvninstall7m 40smaster passed
+1 💚compile2m 34smaster passed
+1 💚javadoc1m 50smaster passed
_ Patch Compile Tests _
+0 🆗mvndep0m 10sMaven dependency ordering for patch
+1 💚mvninstall3m 56sthe patch passed
+1 💚codespell0m 51sNo new issues.
+1 💚compile2m 36sthe patch passed
+1 💚javac2m 36sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚javadoc1m 41sthe patch passed
_ Other Tests _
+1 💚unit0m 30stez-dag in the patch passed.
+1 💚unit0m 24stez-tests in the patch passed.
+1 💚unit0m 19stez-ext-service-tests in the patch passed.
+1 💚unit2m 4sroot in the patch passed.
+1 💚asflicense1m 5sThe patch does not generate ASF License warnings.
29m 19s
SubsystemReport/Notes
DockerClientAPI=1.54 ServerAPI=1.54 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-465/1/artifact/out/Dockerfile
GITHUB PR#465
Optional Testsdupname asflicense javac javadoc unit codespell detsecrets xmllint compile
unameLinux 7ef166f4d872 5.15.0-173-generic #183-Ubuntu SMP Fri Mar 6 13:29:34 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personality/home/jenkins/jenkins-home/workspace/tez-multibranch_PR-465/src/.yetus/personality.sh
git revisionmaster / b57f87e
Default JavaUbuntu-21.0.10+7-Ubuntu-124.04
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-465/1/testReport/
Max. process+thread count117 (vs. ulimit of 5500)
modulesC: tez-dag tez-tests tez-ext-service-tests . U: .
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-465/1/console
versionsgit=2.43.0 maven=3.8.7 codespell=2.4.1
Powered byApache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@abstractdog

abstractdog commented Mar 17, 2026

Copy link
Copy Markdown
Contributor

@Aggarwal-Raghav : awesome contribution again! am I right to assume that the surefire argline related patches are not related to the introduction of the jacoco plugin? I'm struggling to find the connection: could you move argline fixes to a separate jira/patch?

@Aggarwal-Raghav

Copy link
Copy Markdown
ContributorAuthor

@Aggarwal-Raghav : awesome contribution again! am I right to assume that the surefire argline related patches are not related to the introduction of the jacoco plugin? I'm struggling to find the connection: could you move argline fixes to a separate jira/patch?

Thanks @abstractdog , if you mean <argLine>@{argLine} then they are related. Please check

  1. https://www.jacoco.org/jacoco/trunk/doc/prepare-agent-mojo.html
  2. https://maven.apache.org/surefire/maven-surefire-plugin/faq.html#late-property-evaluation

Without that the tez-dag tests

[INFO] --- jacoco:0.8.14:prepare-agent (jacoco-prepare-agent) @ tez-dag ---
[INFO] argLine set to -javaagent:/Users/raghav/.m2/repository/org/jacoco/org.jacoco.agent/0.8.14/org.jacoco.agent-0.8.14-runtime.jar=destfile=/Users/raghav/Desktop/apache/tez/master/tez-dag/target/jacoco.exec,excludes=**/generated/**
....
....
[INFO] Results:
[INFO]
[WARNING] Tests run: 562, Failures: 0, Errors: 0, Skipped: 9
[INFO]
[INFO]
[INFO] --- jacoco:0.8.14:report (jacoco-report) @ tez-dag ---
[INFO] Skipping JaCoCo execution due to missing execution data file.

The javaagent is not picked up

Screenshot 2026-03-17 at 7 47 55 PM

@abstractdog

Copy link
Copy Markdown
Contributor

@Aggarwal-Raghav : awesome contribution again! am I right to assume that the surefire argline related patches are not related to the introduction of the jacoco plugin? I'm struggling to find the connection: could you move argline fixes to a separate jira/patch?

Thanks @abstractdog , if you mean <argLine>@{argLine} then they are related. Please check

  1. https://www.jacoco.org/jacoco/trunk/doc/prepare-agent-mojo.html
  2. https://maven.apache.org/surefire/maven-surefire-plugin/faq.html#late-property-evaluation

Without that the tez-dag tests

[INFO] --- jacoco:0.8.14:prepare-agent (jacoco-prepare-agent) @ tez-dag ---
[INFO] argLine set to -javaagent:/Users/raghav/.m2/repository/org/jacoco/org.jacoco.agent/0.8.14/org.jacoco.agent-0.8.14-runtime.jar=destfile=/Users/raghav/Desktop/apache/tez/master/tez-dag/target/jacoco.exec,excludes=**/generated/**
....
....
[INFO] Results:
[INFO]
[WARNING] Tests run: 562, Failures: 0, Errors: 0, Skipped: 9
[INFO]
[INFO]
[INFO] --- jacoco:0.8.14:report (jacoco-report) @ tez-dag ---
[INFO] Skipping JaCoCo execution due to missing execution data file.

The javaagent is not picked up

Screenshot 2026-03-17 at 7 47 55 PM

I mean all the <environmentVariables combine.children="append"> and surefire plugin removal from submodules makes sense without the jacoco introduction (and a good improvement to pom.xmls in itself) , and only <argLine>@{argLine} -Xmx1024m -XX:+HeapDumpOnOutOfMemoryError -XX:+EnableDynamicAgentLoading ${test.jvm.args}</argLine> is where the agent loading kicks in

@Aggarwal-Raghav

Copy link
Copy Markdown
ContributorAuthor

yes, we can do that but then I have to add that {@argline} in 4 child pom's as well. Let me know if you want 2 JIRA, will make that change. Just confirm one more time.

@Aggarwal-Raghav

Copy link
Copy Markdown
ContributorAuthor

Will rebase once #467 is merged

@abstractdog

Copy link
Copy Markdown
Contributor

Will rebase once #467 is merged

makes sense, thanks! please update BUILDING.txt as well:

tez/BUILDING.txt

Lines 43 to 52 in 9725e38

* Run clover : mvn test -Pclover [-Dclover.license=${user.home}/clover.license]
* Run Rat : mvn apache-rat:check
* Build javadocs : mvn javadoc:javadoc
* Build distribution : mvn package[-Dhadoop.version=2.7.0]
* Visualize state machines : mvn compile -Pvisualize -DskipTests=true
Build options:
* Use -Dpackage.format to create distributions with a format other than .tar.gz (mvn-assembly-plugin formats).
* Use -Dclover.license to specify the path to the clover license file

Comment threadBUILDING.txt
* Install JAR in M2 cache : mvn install
* Deploy JAR to Maven repo : mvn deploy
* Run clover : mvn test -Pclover [-Dclover.license=${user.home}/clover.license]
* Run jacoco : mvn test -Pjacoco

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would appreciate a quick help here about where to find the jacoco report, othen than that, this looks good

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you mean add that info in BUILDING.txt that it's present in target/site/jacoco/ and to render run
python3 -m http.server 8080

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah nevermind, it would break this list, it's fine this way
we can still add info later to a the apache webpage if we want to advertise this in Tez project

@abstractdog

Copy link
Copy Markdown
Contributor

looks good to me, tried it in tez-api, +1, pending tests

@abstractdog
abstractdog self-requested a review March 19, 2026 08:59
@tez-yetus

Copy link
Copy Markdown

💔 -1 overall

VoteSubsystemRuntimeLogfileComment
+0 🆗reexec0m 11sDocker mode activated.
_ Prechecks _
+1 💚dupname0m 0sNo case conflicting files found.
+0 🆗detsecrets0m 0sdetect-secrets was not available.
+0 🆗xmllint0m 0sxmllint was not available.
+1 💚@author0m 0sThe patch does not contain any @author tags.
-1 ❌test4tests0m 0sThe patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch.
_ master Compile Tests _
+1 💚mvninstall9m 51smaster passed
+1 💚compile1m 45smaster passed
+1 💚javadoc1m 29smaster passed
_ Patch Compile Tests _
+1 💚mvninstall3m 9sthe patch passed
+1 💚codespell1m 14sNo new issues.
+1 💚compile1m 41sthe patch passed
+1 💚javac1m 42sthe patch passed
+1 💚blanks0m 0sThe patch has no blanks issues.
+1 💚javadoc0m 48sthe patch passed
_ Other Tests _
+1 💚unit2m 17sroot in the patch passed.
+1 💚asflicense0m 17sThe patch does not generate ASF License warnings.
23m 40s
SubsystemReport/Notes
DockerClientAPI=1.54 ServerAPI=1.54 base: https://ci-hadoop.apache.org/job/tez-multibranch/job/PR-465/2/artifact/out/Dockerfile
GITHUB PR#465
Optional Testsdupname asflicense codespell detsecrets javac javadoc unit xmllint compile
unameLinux e2357d259e1c 5.15.0-173-generic #183-Ubuntu SMP Fri Mar 6 13:29:34 UTC 2026 x86_64 x86_64 x86_64 GNU/Linux
Build toolmaven
Personality/home/jenkins/jenkins-home/workspace/tez-multibranch_PR-465/src/.yetus/personality.sh
git revisionmaster / 68e3457
Default JavaUbuntu-21.0.10+7-Ubuntu-124.04
Test Resultshttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-465/2/testReport/
Max. process+thread count104 (vs. ulimit of 5500)
modulesC: . U: .
Console outputhttps://ci-hadoop.apache.org/job/tez-multibranch/job/PR-465/2/console
versionsgit=2.43.0 maven=3.8.7 codespell=2.4.1
Powered byApache Yetus 0.15.1 https://yetus.apache.org

This message was automatically generated.

@abstractdog
abstractdog self-requested a review March 19, 2026 12:48
@abstractdog
abstractdog merged commit 30932e9 into apache:masterMar 19, 2026
4 checks passed
@Aggarwal-Raghav
Aggarwal-Raghav deleted the TEZ-4693 branch May 1, 2026 05:52
Sign up for freeto 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.

3 participants

@Aggarwal-Raghav@tez-yetus@abstractdog