Skip to content

ci(coverage): disable jacoco PR comments, relax overall delta threshold to 0.4%#6695

Closed
bladehan1 wants to merge 1 commit intotronprotocol:developfrom
bladehan1:fix/ci_coverage
Closed

ci(coverage): disable jacoco PR comments, relax overall delta threshold to 0.4%#6695
bladehan1 wants to merge 1 commit intotronprotocol:developfrom
bladehan1:fix/ci_coverage

Conversation

@bladehan1
Copy link
Copy Markdown
Collaborator

What

Three targeted fixes to the coverage-gate CI job in pr-build.yml:

  1. Add comment-type: summary to both jacoco-report steps — routes output to the workflow step summary instead of PR comments.
  2. Relax MAX_DROP from -0.1% to -0.4%.

Why

Issue 1 — 403 errors on PR comment posting

The coverage-gate job declares only permissions: contents: read. The madrapps/jacoco-report action defaults to comment-type: pr_comment, which calls the GitHub PR comment API and requires pull-requests: write. Every CI run was producing a 403 error in the logs, though it did not block the gate because the action's own continue-on-error default is true.

Setting comment-type: summary makes the action write to $GITHUB_STEP_SUMMARY (a local file write, no API call), so the 403 disappears entirely without needing to grant broader token permissions.

Issue 2 — coverage-changed-files reports INSTRUCTION coverage, not LINE coverage

The value shown as Changed Files Coverage is bytecode instruction coverage, not source-line coverage. This is a fixed behavior of madrapps/jacoco-report@v1.7.2 — the action has no coverage-type parameter and internally reads only the INSTRUCTION counter from JaCoCo XML.

Why INSTRUCTION ≠ LINE and why it matters:

  • JaCoCo XML records six counter types per source file: INSTRUCTION, BRANCH, LINE, COMPLEXITY, METHOD, CLASS.
  • A single source line compiles to multiple bytecode instructions. For example, a null-check if (x != null) may emit 3–5 instructions; a fluent chain on one line may emit 10+.
  • INSTRUCTION coverage is therefore sensitive to compiler expansion: generated code (constructors, getters/setters, toString, enum boilerplate) inflates the instruction count without adding meaningful semantic coverage.
  • When a PR touches one method in a class that also has many generated accessors, the changed-file INSTRUCTION coverage can read significantly lower than the LINE coverage a reviewer would intuitively expect.
  • Concretely: the 93.3% figure visible in recent CI runs is INSTRUCTION coverage of the diff files, not line coverage.

To switch to LINE coverage the action would need to be replaced with a custom script that reads the LINE counter (<counter type="LINE" covered="N" missed="M"/>) from each matching <sourcefile> node. The action itself does not expose this as a configuration option in any released version through v1.7.2.

Issue 3 — -0.1% delta threshold causes false failures

Normal variance in test execution timing, minor refactoring (renaming, extracting constants), or adding tests to previously uncovered edge-case paths can shift overall INSTRUCTION coverage by ±0.2%. A threshold of -0.1% triggered spurious gate failures in these scenarios. -0.4% still catches real coverage regressions while eliminating the noise.

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