fix(codecov): populate branch/condition data in Clover reports - #337
Open
rossaddison wants to merge 1 commit into
Open
fix(codecov): populate branch/condition data in Clover reports#337rossaddison wants to merge 1 commit into
rossaddison wants to merge 1 commit into
Conversation
refactor(codecov): extract branch aggregation shared by Clover and Cobertura test(codecov): cover Clover's new conditionals/type=cond output CloverReport hardcoded conditionals/coveredconditionals to 0 and never read FileCoverage::$functions at all, even when branch data was genuinely collected (CoverageLevel::Branch/Path via XdebugDriver) -- confirmed by contrast with CoberturaReport in this same package, which already consumed that data correctly. Reported as php-testo#334. - BranchCoverageAggregator (new, @internal) extracts CoberturaReport::countBranches()/buildLineBranchMap() into a shared helper, per the issue discussion's suggested scope, so the two report writers stop duplicating the same reduction over FileCoverage::$functions. - CloverReport now populates conditionals/coveredconditionals (file and project level) from countBranches(), and folds them into elements/coveredelements alongside statements, mirroring how php-code-coverage's own Clover writer sums methods + statements + branches into elements. - Per-line output: a line that's a genuine branch decision point (>=2 outgoing edges, from buildLineBranchMap()) now writes type="cond" with truecount/falsecount instead of type="stmt"; ordinary statement lines are unaffected. - CloverReportTest: 5 new cases -- conditionals/coveredconditionals filled from branch data, elements including them, a per-line type="cond" case with truecount/falsecount, a single-outgoing-edge branch staying type="stmt" (not a decision point), and the "no branch data at all" Line-level shape keeping every zero at zero. Verified: `vendor/bin/testo --suite=Codecov/Unit` (136/136, up from 131), `composer psalm` (no errors), `composer cs:diff` (0 of 4 files need fixing), `composer rector:ci` (no changes) -- all against this PR's own touched files specifically, alongside a full-suite run (2211/2211 excluding pre-existing, unrelated Bench/Self timing flakiness under TESTO_CI=1). Assisted-By: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
Closes #334.
What was wrong
CloverReporthardcodedconditionals/coveredconditionalsto0at both file and project level, and never readFileCoverage::$functionsat all — even when branch data was genuinely collected (CoverageLevel::Branch/PathviaXdebugDriver). Confirmed by contrast withCoberturaReportin this same package, which already consumes that data correctly (branch-rate,condition-coverage).What this does
BranchCoverageAggregator(new,@internal): extractsCoberturaReport's owncountBranches()/buildLineBranchMap()into a shared helper, per the suggested scope in the issue, so both report writers reduceFileCoverage::$functionsthe same one way instead of drifting.CloverReport: now populatesconditionals/coveredconditionals(file and project level) fromcountBranches(), and folds them intoelements/coveredelementsalongside statements — mirroring howphp-code-coverage's own Clover writer sums methods + statements + branches intoelements.buildLineBranchMap()) now writestype="cond"withtruecount/falsecountinstead oftype="stmt"; ordinary statement lines are unaffected. Since a branch can have more than 2 outgoing edges (amatcharm per case),truecount/falsecountare populated as covered/uncovered edge counts rather than a literal true/false pair — documented inline.CloverReportTest— conditionals/coveredconditionals filled from branch data, elements including them, a per-linetype="cond"case withtruecount/falsecount, a single-outgoing-edge branch stayingtype="stmt"(not a decision point), and the "no branch data at all" Line-level shape keeping every zero at zero (per the issue's explicit ask).Verification
vendor/bin/testo --suite=Codecov/Unit: 136/136 (up from 131).composer psalm: no errors.composer cs:diff: 0 of 4 touched files need fixing.composer rector:ci(dry-run against the touched files): no changes.TESTO_CI=1: 2211/2211, excluding 3 pre-existing, unrelatedTests\Bench\Self\BenchAttrtiming-sensitive failures reproduced identically on a clean checkout of1.xbefore this change.Assisted-By: Claude Sonnet 5 noreply@anthropic.com