test(common): verify coverage gate fails on uncovered change#28
test(common): verify coverage gate fails on uncovered change#28hzj-edu-nju wants to merge 1 commit intobladehan1:developfrom
Conversation
Intentional test: add a public utility method to DecodeUtil without any unit test so `diff-cover` reports changed-line coverage below 60% and the Coverage Gate exercises its FAIL path (Acceptance Gate G1.3 — Java PR with coverage below threshold should FAIL). The new method has ~10 executable lines and is not called from production code or tests, so the new lines appear as 0% covered. Revert before merging.
📝 WalkthroughWalkthroughA new public utility method Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
common/src/main/java/org/tron/common/utils/DecodeUtil.java (1)
44-47: Redundant zero-branch.
byteLength == 0already yields0viabyteLength * 2, so the explicit branch is unnecessary. Not a functional issue — only relevant if this method is ever kept beyond the sandbox PR.♻️ Proposed simplification
- if (byteLength < 0) { - throw new IllegalArgumentException("byteLength must be non-negative"); - } - if (byteLength == 0) { - return 0; - } - return byteLength * 2; + if (byteLength < 0) { + throw new IllegalArgumentException("byteLength must be non-negative"); + } + return byteLength * 2;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/src/main/java/org/tron/common/utils/DecodeUtil.java` around lines 44 - 47, In DecodeUtil (the method containing the shown snippet), remove the redundant if (byteLength == 0) branch and simply return byteLength * 2; so replace the conditional block with a single return statement to simplify the code while preserving behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@common/src/main/java/org/tron/common/utils/DecodeUtil.java`:
- Around line 35-48: Remove the sandbox-only helper method hexLengthForAddress
from DecodeUtil to avoid introducing dead public API; revert the added method
(public static int hexLengthForAddress(int byteLength)) from
org.tron.common.utils.DecodeUtil so callers continue to use existing
ADDRESS_SIZE/ADDRESS_SIZE/2 logic and ensure no tests or usages reference
hexLengthForAddress before merging.
---
Nitpick comments:
In `@common/src/main/java/org/tron/common/utils/DecodeUtil.java`:
- Around line 44-47: In DecodeUtil (the method containing the shown snippet),
remove the redundant if (byteLength == 0) branch and simply return byteLength *
2; so replace the conditional block with a single return statement to simplify
the code while preserving behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8a022337-d54f-4fc3-a88b-84c0180b2653
📒 Files selected for processing (1)
common/src/main/java/org/tron/common/utils/DecodeUtil.java
| /** | ||
| * Intentional uncovered helper used to exercise the Coverage Gate FAIL path | ||
| * (changed-line coverage below 60%). No unit test is added on purpose. | ||
| * Revert before merging. | ||
| */ | ||
| public static int hexLengthForAddress(int byteLength) { | ||
| if (byteLength < 0) { | ||
| throw new IllegalArgumentException("byteLength must be non-negative"); | ||
| } | ||
| if (byteLength == 0) { | ||
| return 0; | ||
| } | ||
| return byteLength * 2; | ||
| } |
There was a problem hiding this comment.
Sandbox-only change — ensure this is reverted before merge.
Per the PR description, this method is intentionally added to exercise the Coverage Gate FAIL path and must not be merged. The Javadoc correctly flags this, but it's worth double-checking that once the gate behavior is observed, this PR is closed/reverted and hexLengthForAddress is not left behind as unused public API in common. Since the method has no callers in the tree (existing address-length checks use ADDRESS_SIZE / ADDRESS_SIZE / 2 directly in JsonRpcApiUtil, ValidateAddressServlet, etc.), landing it would just add dead public surface.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@common/src/main/java/org/tron/common/utils/DecodeUtil.java` around lines 35 -
48, Remove the sandbox-only helper method hexLengthForAddress from DecodeUtil to
avoid introducing dead public API; revert the added method (public static int
hexLengthForAddress(int byteLength)) from org.tron.common.utils.DecodeUtil so
callers continue to use existing ADDRESS_SIZE/ADDRESS_SIZE/2 logic and ensure no
tests or usages reference hexLengthForAddress before merging.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="common/src/main/java/org/tron/common/utils/DecodeUtil.java">
<violation number="1" location="common/src/main/java/org/tron/common/utils/DecodeUtil.java:40">
P3: Temporary coverage-gate helper was added as a public production API; remove it or move it to test-only code before merge.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| * (changed-line coverage below 60%). No unit test is added on purpose. | ||
| * Revert before merging. | ||
| */ | ||
| public static int hexLengthForAddress(int byteLength) { |
There was a problem hiding this comment.
P3: Temporary coverage-gate helper was added as a public production API; remove it or move it to test-only code before merge.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At common/src/main/java/org/tron/common/utils/DecodeUtil.java, line 40:
<comment>Temporary coverage-gate helper was added as a public production API; remove it or move it to test-only code before merge.</comment>
<file context>
@@ -32,4 +32,19 @@ public static boolean addressValid(byte[] address) {
+ * (changed-line coverage below 60%). No unit test is added on purpose.
+ * Revert before merging.
+ */
+ public static int hexLengthForAddress(int byteLength) {
+ if (byteLength < 0) {
+ throw new IllegalArgumentException("byteLength must be non-negative");
</file context>
2 participants
Summary
Intentional test PR to exercise Acceptance Gate G1.3 of the coverage gate: a Java PR whose changed-line coverage is
<= 60%should fail the gate.The PR adds a new public utility method
DecodeUtil.hexLengthForAddress(int)with ~10 executable lines. No unit test is added on purpose, sodiff-coverwill report0%changed-line coverage and the Coverage Gate job should exit non-zero withChanged-line Gate: FAIL (<= 60%).Expected CI signals
Coverage Gatestep summary shows:Changed-line Coverage: 0%Changed-line Gate: FAIL (<= 60%)Overall Delta Gate: PASS (>= -0.1%)(no regression in overall INSTRUCTION delta)Coverage Gatejob ends withCoverage gate failed: changed-line coverage must be > 60%and exit code1.Not for merge
This PR is a sandbox test and must not be merged. Revert / close after observing the expected gate FAIL.
Summary by cubic
Adds an uncovered public helper
DecodeUtil.hexLengthForAddress(int)to intentionally verify Coverage Gate Acceptance G1.3 (changed-line coverage <= 60% should fail).CI should report 0% changed-line coverage and fail the changed-line gate; this is a sandbox test and must not be merged.
Written for commit dbe162b. Summary will update on new commits.
Summary by CodeRabbit