Skip to content

fix(codecov): populate branch/condition data in Clover reports - #337

Merged
roxblnfk merged 2 commits into
php-testo:1.xfrom
rossaddison:fix/334-clover-branch-coverage
Sep 14, 2026
Merged

roxblnfk merged 2 commits into
php-testo:1.xfrom
rossaddison:fix/334-clover-branch-coverage

Conversation

@rossaddison

Copy link
Copy Markdown
Contributor

Closes #334.

What was wrong

CloverReport hardcoded conditionals/coveredconditionals to 0 at both file and project level, 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 consumes that data correctly (branch-rate, condition-coverage).

What this does

  • BranchCoverageAggregator (new, @internal): extracts CoberturaReport's own countBranches()/buildLineBranchMap() into a shared helper, per the suggested scope in the issue, so both report writers reduce FileCoverage::$functions the same one way instead of drifting.
  • 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. Since a branch can have more than 2 outgoing edges (a match arm per case), truecount/falsecount are populated as covered/uncovered edge counts rather than a literal true/false pair — documented inline.
  • Tests: 5 new cases in CloverReportTest — 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 (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.
  • Full suite under TESTO_CI=1: 2211/2211, excluding 3 pre-existing, unrelated Tests\Bench\Self\BenchAttr timing-sensitive failures reproduced identically on a clean checkout of 1.x before this change.

Assisted-By: Claude Sonnet 5 noreply@anthropic.com

@rossaddison
rossaddison requested a review from a team as a code owner September 13, 2026 22:44
@roxblnfk

Copy link
Copy Markdown
Member

Pushed a follow-up commit (e7e4af4) on top of yours with a few review tweaks, so the PR can be merged as is:

  • conditionals / coveredconditionals at file and project level are now summed from buildLineBranchMap() instead of countBranches(). Only decision points count, so the file metrics equal the sum of truecount + falsecount over the cond lines, and single-edge linear jumps no longer dilute condition coverage. Cobertura keeps countBranches() for branches-valid, its semantics are unchanged.
  • cond lines no longer carry the count attribute, the Clover schema defines it for stmt only.
  • Comments trimmed to the constraints, #[Covers(CloverReport::class)] added on the test class, Assert::false() instead of Assert::same(isset(), false).

Thanks for the clean port and the tests, the aggregator extraction is exactly what was needed.

@codecov

codecov Bot commented Sep 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.07843% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
.../codecov/src/Internal/BranchCoverageAggregator.php 90.00% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

refactor(codecov): share branch aggregation between Clover and Cobertura

test(codecov): cover Clover conditionals and type="cond" lines

CloverReport hardcoded conditionals/coveredconditionals to 0 and never read FileCoverage::$functions, so branch data collected at CoverageLevel::Branch/Path was discarded, while CoberturaReport in the same package already consumed it. Resolves php-testo#334.

The aggregation moves into Testo\Codecov\Internal\BranchCoverageAggregator and both writers use it. Clover counts only decision points (two or more outgoing edges) as conditionals, so file and project metrics equal the sum of truecount/falsecount over the cond lines and single-edge linear jumps do not dilute condition coverage. Branches are folded into elements/coveredelements the way php-code-coverage's Clover writer does. Cobertura keeps counting every edge for branches-valid.

Assisted-By: Claude Sonnet 5 <noreply@anthropic.com>
Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@roxblnfk
roxblnfk force-pushed the fix/334-clover-branch-coverage branch from bd858e0 to ee2f5cc Compare September 14, 2026 13:16
Result and Exception classes gain `@api`, CoverageDriver under the Internal namespace switches to `@internal`, which clears the plugin's own StructArmed findings from php-testo#330. The remaining ones, CoverageInput using core Config attributes and ApplicationPlugins depending on CodecovPlugin, are cross-package and stay open.

Assisted-By: Claude Fable 5.1 <noreply@anthropic.com>
@roxblnfk
roxblnfk force-pushed the fix/334-clover-branch-coverage branch from ee2f5cc to 8ad0366 Compare September 14, 2026 13:19
@roxblnfk
roxblnfk merged commit 9fea2fa into php-testo:1.x Sep 14, 2026
13 of 14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CloverReport discards collected branch/condition coverage (conditionals always 0)

2 participants