[fix][build] Dump Jacoco coverage data to file with JMX interface in TestNG listener

[lhotari]

Fixes #19931

Motivation

See #19931. Sometimes the default Jacoco shutdown hook doesn't run and there's no coverage data. This causes the Codecov upload to fail since there's no Jacoco coverage report available.

Modifications

• enable Jacoco agent's JMX interface
• Dump Jacoco coverage to file with JMX interface in TestNG listener

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: lhotari#145

[tisonkun]

It seems personal CI failed before upload the report - https://github.com/lhotari/pulsar/actions/runs/4540821034/jobs/8002229561?pr=145

[lhotari]

It seems personal CI failed before upload the report - https://github.com/lhotari/pulsar/actions/runs/4540821034/jobs/8002229561?pr=145

@tisonkun it didn't fail because of the changes made in the PR. The building step took 55 minutes and the workflow timed out because of that. I don't see what changes you are requesting.

[tisonkun]

The building step took 55 minutes and the workflow timed out because of that. I don't see what changes you are requesting.

Yes, that's the case. Are your going to rerun the workflow in your fork and notify here once it passed, or we tag this PR as ready-to-test and rerun here?

[lhotari]

Yes, that's the case. Are your going to rerun the workflow in your fork and notify here once it passed, or we tag this PR as ready-to-test and rerun here?

@tisonkun re-running it. in progress at https://github.com/lhotari/pulsar/actions/runs/4540821034/jobs/8006055251?pr=145 . This time build step completed properly. Waiting for the whole build job to complete.

[lhotari]

@tisonkun I revisited the dumping logic once more. It turns out that with maven-surefire-plugin each test is run in it's own TestNG suite. TestNG has a separate listener interface for the completion of all tests.

[lhotari]

/pulsarbot rerun-failure-checks

outdated

[tisonkun]

Cool!

[tisonkun]

Hmmm...Now we can pass the flaky tests job, but it seems the integration tests upload job is still suffering:

[lhotari]

Hmmm...Now we can pass the flaky tests job, but it seems the integration tests upload job is still suffering:

this PR doesn't improve upload at all. It's just about dumping the Jacoco exec data.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.81%. Comparing base (cda2827) to head (c5672e1).
Report is 1764 commits behind head on master.

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #19947       +/-   ##
=============================================
+ Coverage     58.92%   72.81%   +13.89%     
- Complexity    25921    31385     +5464     
=============================================
  Files          1846     1859       +13     
  Lines        136607   136808      +201     
  Branches      15033    15047       +14     
=============================================
+ Hits          80498    99623    +19125     
+ Misses        48645    29273    -19372     
- Partials       7464     7912      +448     

Flag	Coverage Δ
inttests	24.42% <ø> (?)
systests	25.07% <ø> (?)
unittests	72.09% <ø> (+13.16%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 705 files with indirect coverage changes

[tisonkun]

@lhotari the error info says "not_found" and I read it as file not created. But it seems different from the previous one.

Do we now always dump the file, but the upload action itself is still unstable? IIRC there is an upstream issue for the latter one.

[lhotari]

@lhotari the error info says "not_found" and I read it as file not created. But it seems different from the previous one.

Do we now always dump the file, but the upload action itself is still unstable? IIRC there is an upstream issue for the latter one.

Exactly. For the upload issue, it seems that Codecov suggests to not consider the upload token as a secret:

Public repositories that rely on PRs via forks will find that they cannot effectively use Codecov if the token is stored as a GitHub secret. The scope of the Codecov token is only to confirm that the coverage uploaded comes from a specific repository, not to pull down source code or make any code changes.

For this reason, we recommend that teams with public repositories that rely on PRs via forks consider the security ramifications of making the Codecov token available as opposed to being in a secret.

I think we need to ask ASF infra to create the token.