Repository navigation
ci: strip zero-instruction lines before Codecov upload - #260
Conversation
Kover's IntelliJ engine writes <line mi="0" ci="0" mb="0" cb="0"> for return-only lines, and Codecov's JaCoCo parser counts them as misses. kotlinx-kover #699 is closed as won't fix, so CI removes these entries from the Kover XML before the upload. Counters stay unchanged.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds a script that removes zero-instruction ChangesKover coverage filtering
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant LocalTests as local-tests job
participant Filter as zero-instruction line script
participant Report as Kover XML report
participant Codecov
LocalTests->>Filter: Process report before upload
Filter->>Report: Remove zero-instruction lines and write XML
LocalTests->>Codecov: Upload filtered report
Merge Risk: ⚪ Minimal · up to No actionable coverage-report regression is established, and the filter runs before the Codecov upload. The exact Kover 0.9.11 XML shape could not be verified, but no known merge blocker remains. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change is confined to coverage reporting and does not expand CI permissions or change the upload destination. Coverage verification still precedes filtering, and filter failures normally prevent upload. Risk is low because compatibility with the precise generated report format remains unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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. A rabbit checks each line with care, Comment |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| for sourcefile in tree.iter("sourcefile"): | ||
| for line in sourcefile.findall("line"): | ||
| if is_zero_instruction_line(line): | ||
| sourcefile.remove(line) | ||
| removed += 1 | ||
| else: | ||
| kept += 1 | ||
| tree.write(path, encoding="UTF-8", xml_declaration=True) |
There was a problem hiding this comment.
<line>. That leaves the new removal behavior untested. A small Kover XML fixture could check that only the intended line is removed and that retained lines and counters survive the rewrite.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/scripts/strip-zero-instruction-lines.py
Line: 17-24
Comment:
**Filtering case has no test** The reported local run removed zero lines, and there is no test fixture with an all-zero `<line>`. That leaves the new removal behavior untested. A small Kover XML fixture could check that only the intended line is removed and that retained lines and counters survive the rewrite.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Declining. The script is ~20 lines of CI glue that fails loudly (exit 1) on a missing or malformed report. Its removal behavior was verified against a fixture with 364 injected all-zero lines (exactly those removed, every counter unchanged) when it was introduced; this repo carries a byte-identical copy enforced by a parity check. A separate Python test suite plus a CI step in every Android repo would cost more than the script it guards.
There was a problem hiding this comment.
That’s reasonable. The injected-fixture verification covers the removal path and counter preservation, while the parity check prevents this copy from drifting. Given the script’s small scope and fail-loudly behavior, adding a separate fixture/test suite here would be disproportionate. I’m withdrawing the test-coverage concern.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1 participant
Summary
<line mi="0" ci="0" mb="0" cb="0">for return-only lines..github/scripts/strip-zero-instruction-lines.py) removes these<line>entries fromreportDebug.xml. CI runs it before the Codecov upload, and all counters stay unchanged.CLAUDE.mdgains one line on the Kover tool entry that documents the CI filter step.Commit:
afd06acci: strip zero-instruction lines before Codecov uploadVerification
reportDebug.xml: 0 lines removed, 1495 kept, all 1845 counters unchanged. This repo has no affected lines today.Notes
🤖 Generated with Claude Code
https://claude.ai/code/session_012g8rnoGyH6eeKMXGqyc7hz
Adds a filter that removes zero-instruction JaCoCo lines before Codecov upload. Documents the CI step. Test results were not provided.