fix(ci): gate changed-line coverage with diff-cover#27
fix(ci): gate changed-line coverage with diff-cover#27
Conversation
…col#6614) * test(framework): merge duplicate CredentialsTest coverage Consolidate the misplaced keystroe CredentialsTest into org.tron.keystore.CredentialsTest. - remove the duplicate test under the misspelled keystroe package - add explicit equals behavior coverage for address and cryptoEngine - normalize assertions to JUnit Assert and remove legacy TestCase usage * test(framework): stabilize CredentialsTest fixtures Replace random Credentials test setup with deterministic SignInterface mocks so the suite no longer depends on platform-specific SecureRandom providers or probabilistic retries. - remove NativePRNG usage from CredentialsTest - replace random key generation with fixed address fixtures via mocked SignInterface - assert create(SignInterface) returns the expected base58check address - keep equals/hashCode contract coverage with deterministic inputs
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 34 minutes and 46 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
- Use diff-cover changed-line coverage as the changed-code gate; keep the overall coverage delta gate at -0.1% and JaCoCo reports as summary-only snapshots. - Handle non-Java PRs: when diff-cover reports no changed Java lines (workflow-only, docs-only, proto-only), emit NA and treat it as SKIPPED in the enforcement step so the gate does not wrongly fail. - Harden the diff-cover step: guard against empty SRC_ROOTS before invoking diff-cover, verify diff-cover.json was written, and surface exit codes in failure messages. - Clarify metric semantics in the step summary: Changed-line is LINE coverage (diff-cover) while Overall / Delta are INSTRUCTION coverage (jacoco-report).
bladehan1
force-pushed
the
fix/ci_coverage_gate
branch
from
b63295a to
141d786
Compare
2 participants
Summary
Replace the
coverage-changed-filesgate (whole-file INSTRUCTION coverage) with a true changed-line LINE coverage gate powered bydiff-cover, and make the gate robust against non-Java PRs and tool failures.Reference: tronprotocol/java-tron#6705 — same change set, prepared as a single squashed commit.
Commit included
diff-coveras the changed-code gate, handle non-Java PRs via anNAsentinel, and polish the step (guard emptySRC_ROOTS, verifydiff-cover.json, clarify LINE vs INSTRUCTION semantics in the metrics panel).Net change vs tronprotocol/develop base: 1 file, +90 / -27.
Why
madrapps/jacoco-report'scoverage-changed-filesoutput, which is the whole-file INSTRUCTION coverage of each changed file — not the coverage of the lines the PR actually changed.diff-coverprovides the industry-standard "changed-line" measurement.jacoco-report's default PR comment needspull-requests: write, which fork PRs do not get. Job logs kept filling up with 403 errors.diff-cover, a PR that only touches workflows / docs / proto / test code producestotal_num_lines=0, and the originaljqcall exited with 1. TheNAsentinel now maps that state toSKIPPEDin the enforce step.Design notes for reviewers
coverage-basejob runs independently ofdocker-build-debian11(they share noneeds:). Sincedocker-build-debian11is already the slowest build (~16 min withtestWithRocksDb), adding a parallelcoverage-baseof similar length does not extend the critical path. The gate job waits for both, so total PR wall-clock is unchanged.Changed-line Coverageuses LINE coverage (diff-cover), whilePR Overall Coverage/Base Overall Coverage/Deltause INSTRUCTION coverage.madrapps/jacoco-reportv1.7.2 is the latest release and hard-codes thecoverage-overalloutput to INSTRUCTION; there is nocoverage-typeinput yet — see the openmadrapps/jacoco-report#82on milestone1.8. The METRICS panel and step summary now print an explicit note so readers do not accidentally compare the two counters.diff-cover --src-rootsis limited to*/src/main/java. PRs touching onlysrc/test/java,build/generated(proto output), docs, or CI will be reported asSKIPPED. In practice proto-only PRs with non-trivial generated-code coverage shifts are rare in this repository; we accept the gap.MIN_CHANGED=60(for Changed-line LINE) andMAX_DROP=-0.1%(for Overall INSTRUCTION delta) are unchanged from the existing workflow.exit 1branches only fire in genuine error states:SRC_ROOTSis empty (unexpected workspace shape) ordiff-cover.jsonis missing (tool crash). Neither can be triggered by ordinary PR content.bladehan1/java-tron:developalready contains the first two steps of this rollout via prior in-fork merges (PRs test: validate changed-line coverage gate #23 and ci(coverage): harden diff-cover step and handle non-Java PRs #25). The diff visible on this PR (+12/-2) is therefore only thepolishsubset; the commit message and Summary describe the full scope so the single squashed commit stands alone on upstream.Follow-ups (not in this PR)
Overallto LINE oncemadrapps/jacoco-report1.8 shipscoverage-type, or by self-parsing the JaCoCo XML. Unifies the metrics panel on one counter.bats) for the enforce step, so the three-state logic (NA/ numeric / error) is covered without needing a real CI run.comment-type: summaryif reviewers prefer on-PR comment visibility; restoring comments requires grantingpull-requests: writeon thecoverage-gatejob.Test plan
Changed-line Coverage: NA,Changed-line Gate: SKIPPED, gate passes. Verified end-to-end on a prior fork PR carrying the same commits.Changed-line Coverage: X%,Changed-line Gate: PASSatX > 60. Verified end-to-end on a prior fork PR (25 changed lines @ 100%).X <= 60→FAIL. Logic reviewed; path not force-exercised.Summary by cubic
Replaced the whole-file changed-files coverage gate with true changed-line LINE coverage using
diff-cover, keeping the overall INSTRUCTION delta gate unchanged. Also consolidated and stabilizedCredentialsTestwith deterministic fixtures.New Features
diff-cover; thresholds unchanged (MIN_CHANGED=60, MAX_DROP=-0.1%).NA) so docs/workflow/proto-only PRs don’t fail.src/main/javaroots exist, fetch base ref, verifydiff-cover.json; note LINE vs INSTRUCTION semantics in the step summary.madrapps/jacoco-report@v1.7.2withcomment-type: summaryto avoid 403s on fork PRs; set up Python 3.11 and pindiff-cover==9.2.0.Refactors
org.tron.keystroe.CredentialsTestintoorg.tron.keystore.CredentialsTest.SignInterfacefixtures; assert Base58Check address and equals/hashCode behavior; normalize to JUnitAssert.Written for commit 141d786. Summary will update on new commits.