Repository navigation
feat(coverage): read jacoco xml reports natively - #2197
Merged
Merged
Conversation
Parse sourcefile line records and detect XML by its root element so JVM coverage reports need no Cobertura converter. Expose jacoco in CLI and MCP selectors and test partial coverage, aggregate groups and shards. Preserve source-root-relative paths and document the mapping limitation tracked in #2188. Closes #2187
Reject stray text and CDATA before or after the root so malformed XML cannot silently produce an apparently valid coverage report. Add a regression test while preserving whitespace and trailing comments.
CoverageTotal: 97.95% ⚪ 0 pp vs Comparing
🔇 269 ignored region(s), 0 tolerated region(s)
Patch coveragePatch: 100% (243/243 new lines covered)
Indirect coverage changes🔴 0 lines lost coverage, 🟢 2 lines gained coverage on unchanged code. Indirect changes
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
JaCoCo reports from Java, Kotlin and Scala can now be read directly by
coverage diff,coverage mergeand the MCPcoverage_difftool. XML detection distinguishes JaCoCo's<report>root from Cobertura's<coverage>root;--report-format jacocoalso works explicitly, including for baselines.A partially covered line (
mi > 0,ci > 0) counts as covered. Instruction counts become boolean hits, and aggregate groups or separate module reports merge by covered-line union. Branch counters and class/method summaries are ignored.Type of Change
Related Issue
Closes #2187
Implementation plan
Changes Made
src/coverage/jacoco.rswith sourcefile/line parsing, default-package support, escaped names, group aggregation and malformed-input validation.Format::Jacocoand XML-root detection after declarations, comments and DTDs insrc/coverage/format.rs; retain the existing Cobertura parser.jacocoin CLI and MCP selectors and update the reviewed help snapshot.docs/coverage.md; add an[Unreleased]changelog entry.Testing
All commands ran in
/Users/jky/wrk/work-trees/omni-dev/issue-2187-jacoco-xml, with Cargo explicitly targeting that worktree's manifest. Checks were rerun after the review fix and rebase.Results: build, formatting, Clippy, marker lint, changelog and commit lint all passed. The final default suite passed 13,468 tests and the MCP suite passed 14,367 tests, with zero failures and one existing ignored test in each. The full suites cover the help snapshot. JaCoCo tests exercise multiple packages, partially covered lines, default packages, escaped attributes, aggregate groups, module/shard union, malformed or truncated XML, missing/invalid numbers, stray text outside the root and MCP gate results. Coverage percentages were not measured locally; CI computes them. No live JVM report-generation test was run.
The first sandboxed filtered test run could not bind local Wiremock ports; both full suites were rerun with that access enabled. Apple’s linker emitted a large
__eh_framewarning for a test binary; tests passed and Clippy reported no lint warnings.Review Focus Areas
com/example/App.java), which may differ from git paths such assrc/main/java/com/example/App.java.Checklist
origin/main...HEADorigin/mainPerformance Impact
Streaming XML parsing with memory proportional to coverage records and XML nesting. No JVM or converter process is invoked.
Security Considerations
External DTDs/entities are not fetched. Invalid numeric attributes and malformed/truncated documents return errors.
Deployment Notes
No special deployment requirements. Rust callers exhaustively matching the public
Formatenum must handle the added variant.Additional Notes
One review finding at
src/coverage/jacoco.rs's event loop was fixed in a separate commit: stray text/CDATA outside the root could be accepted. Added a regression test; no findings were skipped. No departure from the plan.Source-root mapping remains the existing #2188 follow-up. This PR preserves paths rather than guessing Maven/Gradle layouts. Clover and branch coverage remain out of scope.