From 47a13a1bfd262bcebe504833223b678b972e9c34 Mon Sep 17 00:00:00 2001 From: Ross Addison Date: Sun, 13 Sep 2026 23:43:31 +0100 Subject: [PATCH 1/2] fix(codecov): populate branch/condition data in Clover reports 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 #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 Assisted-By: Claude Fable 5.1 --- .../src/Internal/BranchCoverageAggregator.php | 69 +++++++++ plugin/codecov/src/Report/CloverReport.php | 55 +++++-- plugin/codecov/src/Report/CoberturaReport.php | 58 +------ .../tests/Unit/Report/CloverReportTest.php | 144 ++++++++++++++++++ 4 files changed, 259 insertions(+), 67 deletions(-) create mode 100644 plugin/codecov/src/Internal/BranchCoverageAggregator.php diff --git a/plugin/codecov/src/Internal/BranchCoverageAggregator.php b/plugin/codecov/src/Internal/BranchCoverageAggregator.php new file mode 100644 index 00000000..57d96b23 --- /dev/null +++ b/plugin/codecov/src/Internal/BranchCoverageAggregator.php @@ -0,0 +1,69 @@ +, int<0, max>} [total edges, covered edges] + */ + public static function countBranches(FileCoverage $fileCoverage): array + { + $total = 0; + $covered = 0; + + foreach ($fileCoverage->functions as $function) { + foreach ($function->branches as $branch) { + $total += \count($branch->outHit); + $covered += \count(\array_filter($branch->outHit)); + } + } + + return [$total, $covered]; + } + + /** + * Maps line number to [total edges, covered edges] for branch decision points only: + * a branch with a single outgoing edge is a linear jump, not a decision. + * Branches starting on the same line are summed. + * + * @return array, int<0, max>}> + */ + public static function buildLineBranchMap(FileCoverage $fileCoverage): array + { + $map = []; + + foreach ($fileCoverage->functions as $function) { + foreach ($function->branches as $branch) { + if (\count($branch->out) < 2) { + continue; + } + + $line = $branch->lineStart; + $total = \count($branch->outHit); + $covered = \count(\array_filter($branch->outHit)); + + $map[$line] ??= [0, 0]; + + $map[$line][0] += $total; + $map[$line][1] += $covered; + } + } + + return $map; + } +} diff --git a/plugin/codecov/src/Report/CloverReport.php b/plugin/codecov/src/Report/CloverReport.php index de246513..49359b38 100644 --- a/plugin/codecov/src/Report/CloverReport.php +++ b/plugin/codecov/src/Report/CloverReport.php @@ -5,6 +5,7 @@ namespace Testo\Codecov\Report; use Internal\Path; +use Testo\Codecov\Internal\BranchCoverageAggregator; use Testo\Codecov\Result\CoverageResult; use Testo\Codecov\Result\FileCoverage; use Testo\Codecov\Result\LineStatus; @@ -46,12 +47,16 @@ public function generate(CoverageResult $result): void $totalStatements = 0; $totalCovered = 0; + $totalConditionals = 0; + $totalCoveredConditionals = 0; $fileCount = 0; foreach ($result->files as $fileCoverage) { - [$statements, $covered] = $this->writeFile($xml, $fileCoverage); + [$statements, $covered, $conditionals, $coveredConditionals] = $this->writeFile($xml, $fileCoverage); $totalStatements += $statements; $totalCovered += $covered; + $totalConditionals += $conditionals; + $totalCoveredConditionals += $coveredConditionals; $fileCount++; } @@ -60,10 +65,10 @@ public function generate(CoverageResult $result): void 'files' => $fileCount, 'statements' => $totalStatements, 'coveredstatements' => $totalCovered, - 'elements' => $totalStatements, - 'coveredelements' => $totalCovered, - 'conditionals' => 0, - 'coveredconditionals' => 0, + 'conditionals' => $totalConditionals, + 'coveredconditionals' => $totalCoveredConditionals, + 'elements' => $totalStatements + $totalConditionals, + 'coveredelements' => $totalCovered + $totalCoveredConditionals, ]); $xml->endElement(); // project @@ -82,7 +87,8 @@ public function info(): ReportInfo } /** - * @return array{int<0, max>, int<0, max>} [statements, covered] + * @return array{int<0, max>, int<0, max>, int<0, max>, int<0, max>} + * [statements, covered, conditionals, covered conditionals] */ private function writeFile(\XMLWriter $xml, FileCoverage $fileCoverage): array { @@ -91,8 +97,17 @@ private function writeFile(\XMLWriter $xml, FileCoverage $fileCoverage): array $statements = 0; $covered = 0; + $conditionals = 0; + $coveredConditionals = 0; + + // Only decision points count as conditionals, so the file metrics equal + // the sum of truecount/falsecount over the cond lines written below. + $lineBranches = BranchCoverageAggregator::buildLineBranchMap($fileCoverage); + foreach ($lineBranches as [$branchTotal, $branchCovered]) { + $conditionals += $branchTotal; + $coveredConditionals += $branchCovered; + } - // Sort lines by number $lines = $fileCoverage->lines; \ksort($lines); @@ -107,23 +122,35 @@ private function writeFile(\XMLWriter $xml, FileCoverage $fileCoverage): array $xml->startElement('line'); $xml->writeAttribute('num', (string) $lineNumber); - $xml->writeAttribute('type', 'stmt'); - $xml->writeAttribute('count', (string) $count); + + if (isset($lineBranches[$lineNumber])) { + [$branchTotal, $branchCovered] = $lineBranches[$lineNumber]; + + // A branch may have more than two outgoing edges (`match` arms), so + // truecount/falsecount carry covered/uncovered edge counts, not a literal pair. + $xml->writeAttribute('type', 'cond'); + $xml->writeAttribute('truecount', (string) $branchCovered); + $xml->writeAttribute('falsecount', (string) ($branchTotal - $branchCovered)); + } else { + $xml->writeAttribute('type', 'stmt'); + $xml->writeAttribute('count', (string) $count); + } + $xml->endElement(); } $this->writeMetrics($xml, [ 'statements' => $statements, 'coveredstatements' => $covered, - 'elements' => $statements, - 'coveredelements' => $covered, - 'conditionals' => 0, - 'coveredconditionals' => 0, + 'conditionals' => $conditionals, + 'coveredconditionals' => $coveredConditionals, + 'elements' => $statements + $conditionals, + 'coveredelements' => $covered + $coveredConditionals, ]); $xml->endElement(); // file - return [$statements, $covered]; + return [$statements, $covered, $conditionals, $coveredConditionals]; } /** diff --git a/plugin/codecov/src/Report/CoberturaReport.php b/plugin/codecov/src/Report/CoberturaReport.php index 2a376140..1adb7a56 100644 --- a/plugin/codecov/src/Report/CoberturaReport.php +++ b/plugin/codecov/src/Report/CoberturaReport.php @@ -5,6 +5,7 @@ namespace Testo\Codecov\Report; use Internal\Path; +use Testo\Codecov\Internal\BranchCoverageAggregator; use Testo\Codecov\Result\CoverageResult; use Testo\Codecov\Result\FileCoverage; use Testo\Codecov\Result\LineStatus; @@ -50,7 +51,7 @@ public function generate(CoverageResult $result): void [$s, $c] = self::countLines($fileCoverage); $totalLines += $s; $totalLinesCovered += $c; - [$b, $bc] = self::countBranches($fileCoverage); + [$b, $bc] = BranchCoverageAggregator::countBranches($fileCoverage); $totalBranches += $b; $totalBranchesCovered += $bc; } @@ -112,55 +113,6 @@ private static function countLines(FileCoverage $fileCoverage): array return [$statements, $covered]; } - /** - * @return array{int<0, max>, int<0, max>} [total branches, covered branches] - */ - private static function countBranches(FileCoverage $fileCoverage): array - { - $total = 0; - $covered = 0; - - foreach ($fileCoverage->functions as $function) { - foreach ($function->branches as $branch) { - $total += \count($branch->outHit); - $covered += \count(\array_filter($branch->outHit)); - } - } - - return [$total, $covered]; - } - - /** - * Builds a map of line number => [total_branches, covered_branches] - * for lines that are branch decision points. - * - * @return array, int<0, max>}> - */ - private static function buildLineBranchMap(FileCoverage $fileCoverage): array - { - $map = []; - - foreach ($fileCoverage->functions as $function) { - foreach ($function->branches as $branch) { - // Only mark lines with multiple outgoing edges as branch points - if (\count($branch->out) < 2) { - continue; - } - - $line = $branch->lineStart; - $total = \count($branch->outHit); - $covered = \count(\array_filter($branch->outHit)); - - $map[$line] ??= [0, 0]; - - $map[$line][0] += $total; - $map[$line][1] += $covered; - } - } - - return $map; - } - private static function rate(int $covered, int $total): string { return $total === 0 ? '0' : \sprintf('%.4f', $covered / $total); @@ -205,7 +157,7 @@ private function writePackage(\XMLWriter $xml, string $packageName, array $files [$s, $c] = self::countLines($file['coverage']); $pkgLines += $s; $pkgLinesCovered += $c; - [$b, $bc] = self::countBranches($file['coverage']); + [$b, $bc] = BranchCoverageAggregator::countBranches($file['coverage']); $pkgBranches += $b; $pkgBranchesCovered += $bc; } @@ -228,7 +180,7 @@ private function writePackage(\XMLWriter $xml, string $packageName, array $files private function writeClass(\XMLWriter $xml, string $relativePath, FileCoverage $fileCoverage): void { [$statements, $covered] = self::countLines($fileCoverage); - [$branches, $branchesCovered] = self::countBranches($fileCoverage); + [$branches, $branchesCovered] = BranchCoverageAggregator::countBranches($fileCoverage); $className = \basename($relativePath, '.php'); @@ -240,7 +192,7 @@ private function writeClass(\XMLWriter $xml, string $relativePath, FileCoverage $xml->writeAttribute('complexity', '0'); // Build per-line branch map - $lineBranches = self::buildLineBranchMap($fileCoverage); + $lineBranches = BranchCoverageAggregator::buildLineBranchMap($fileCoverage); $xml->startElement('lines'); diff --git a/plugin/codecov/tests/Unit/Report/CloverReportTest.php b/plugin/codecov/tests/Unit/Report/CloverReportTest.php index 46bc40e4..06ad58a2 100644 --- a/plugin/codecov/tests/Unit/Report/CloverReportTest.php +++ b/plugin/codecov/tests/Unit/Report/CloverReportTest.php @@ -5,14 +5,18 @@ namespace Tests\Codecov\Unit\Report; use Testo\Assert; +use Testo\Codecov\Covers; +use Testo\Codecov\Result\BranchCoverage; use Testo\Codecov\Result\CoverageResult; use Testo\Codecov\Result\FileCoverage; +use Testo\Codecov\Result\FunctionCoverage; use Testo\Codecov\Result\LineCoverage; use Testo\Codecov\Result\LineStatus; use Testo\Codecov\Report\CloverReport; use Testo\Test; #[Test] +#[Covers(CloverReport::class)] final class CloverReportTest { public function generatesValidXml(): void @@ -95,6 +99,146 @@ public function writesLineElements(): void \unlink($path); } + public function branchDataFillsConditionals(): void + { + $result = new CoverageResult([ + '/src/Foo.php' => new FileCoverage('/src/Foo.php', [ + 5 => new LineCoverage(5, LineStatus::Executed), + 6 => new LineCoverage(6, LineStatus::NotExecuted), + ], [ + 'Foo->bar' => new FunctionCoverage('Foo->bar', [ + 0 => new BranchCoverage(0, 3, 5, 6, hit: true, out: [4, 7], outHit: [true, false]), + ], []), + ]), + ]); + $path = self::tmpPath(); + + (new CloverReport($path))->generate($result); + + $xml = \simplexml_load_file($path); + + $fileMetrics = $xml->project->file->metrics; + Assert::same((string) $fileMetrics['conditionals'], '2'); + Assert::same((string) $fileMetrics['coveredconditionals'], '1'); + + $projectMetrics = $xml->project->metrics; + Assert::same((string) $projectMetrics['conditionals'], '2'); + Assert::same((string) $projectMetrics['coveredconditionals'], '1'); + + \unlink($path); + } + + public function elementsIncludeConditionalsAlongsideStatements(): void + { + $result = new CoverageResult([ + '/src/Foo.php' => new FileCoverage('/src/Foo.php', [ + 5 => new LineCoverage(5, LineStatus::Executed), + 6 => new LineCoverage(6, LineStatus::NotExecuted), + ], [ + 'Foo->bar' => new FunctionCoverage('Foo->bar', [ + 0 => new BranchCoverage(0, 3, 5, 6, hit: true, out: [4, 7], outHit: [true, false]), + ], []), + ]), + ]); + $path = self::tmpPath(); + + (new CloverReport($path))->generate($result); + + $xml = \simplexml_load_file($path); + $metrics = $xml->project->file->metrics; + + Assert::same((string) $metrics['elements'], '4'); + Assert::same((string) $metrics['coveredelements'], '2'); + + \unlink($path); + } + + public function writesConditionalLineTypeWithTrueAndFalseCounts(): void + { + $result = new CoverageResult([ + '/src/Foo.php' => new FileCoverage('/src/Foo.php', [ + 5 => new LineCoverage(5, LineStatus::Executed), + ], [ + 'Foo->bar' => new FunctionCoverage('Foo->bar', [ + 0 => new BranchCoverage(0, 3, 5, 5, hit: true, out: [4, 7], outHit: [true, false]), + ], []), + ]), + ]); + $path = self::tmpPath(); + + (new CloverReport($path))->generate($result); + + $xml = \simplexml_load_file($path); + $line = $xml->project->file->line; + + Assert::same((string) $line['type'], 'cond'); + Assert::same((string) $line['truecount'], '1'); + Assert::same((string) $line['falsecount'], '1'); + Assert::false(isset($line['count'])); + + \unlink($path); + } + + public function branchesOnALineWithOnlyOneOutgoingEdgeStayPlainStatements(): void + { + $result = new CoverageResult([ + '/src/Foo.php' => new FileCoverage('/src/Foo.php', [ + 5 => new LineCoverage(5, LineStatus::Executed), + ], [ + 'Foo->bar' => new FunctionCoverage('Foo->bar', [ + 0 => new BranchCoverage(0, 3, 5, 5, hit: true, out: [4], outHit: [true]), + ], []), + ]), + ]); + $path = self::tmpPath(); + + (new CloverReport($path))->generate($result); + + $xml = \simplexml_load_file($path); + $line = $xml->project->file->line; + + Assert::same((string) $line['type'], 'stmt'); + Assert::false(isset($line['truecount'])); + Assert::false(isset($line['falsecount'])); + + $metrics = $xml->project->file->metrics; + Assert::same((string) $metrics['conditionals'], '0'); + Assert::same((string) $metrics['coveredconditionals'], '0'); + + \unlink($path); + } + + public function noBranchDataKeepsConditionalsAtZero(): void + { + $result = new CoverageResult([ + '/src/Foo.php' => new FileCoverage('/src/Foo.php', [ + 5 => new LineCoverage(5, LineStatus::Executed), + 6 => new LineCoverage(6, LineStatus::NotExecuted), + ]), + ]); + $path = self::tmpPath(); + + (new CloverReport($path))->generate($result); + + $xml = \simplexml_load_file($path); + + $fileMetrics = $xml->project->file->metrics; + Assert::same((string) $fileMetrics['conditionals'], '0'); + Assert::same((string) $fileMetrics['coveredconditionals'], '0'); + Assert::same((string) $fileMetrics['elements'], (string) $fileMetrics['statements']); + Assert::same((string) $fileMetrics['coveredelements'], (string) $fileMetrics['coveredstatements']); + + $projectMetrics = $xml->project->metrics; + Assert::same((string) $projectMetrics['conditionals'], '0'); + Assert::same((string) $projectMetrics['coveredconditionals'], '0'); + + foreach ($xml->project->file->line as $line) { + Assert::same((string) $line['type'], 'stmt'); + } + + \unlink($path); + } + public function statesItsFormatAndTheFileItWrites(): void { $info = (new CloverReport('build/logs/clover.xml'))->info(); From 8ad0366345b7a7160cffa8e935cd5f31a40cb54a Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 14 Sep 2026 17:15:38 +0400 Subject: [PATCH 2/2] chore(codecov): declare `@api/@internal` on the plugin's public classes Result and Exception classes gain `@api`, CoverageDriver under the Internal namespace switches to `@internal`, which clears the plugin's own StructArmed findings from #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 --- plugin/codecov/src/Exception/BranchCoverageUnsafeInFiber.php | 2 ++ plugin/codecov/src/Exception/CoverageDriverNotAvailable.php | 2 ++ plugin/codecov/src/Internal/CoverageDriver.php | 2 +- plugin/codecov/src/Result/BranchCoverage.php | 2 ++ plugin/codecov/src/Result/CoverageResult.php | 2 ++ plugin/codecov/src/Result/FileCoverage.php | 2 ++ plugin/codecov/src/Result/FunctionCoverage.php | 2 ++ plugin/codecov/src/Result/LineCoverage.php | 2 ++ plugin/codecov/src/Result/LineStatus.php | 2 ++ plugin/codecov/src/Result/PathCoverage.php | 2 ++ 10 files changed, 19 insertions(+), 1 deletion(-) diff --git a/plugin/codecov/src/Exception/BranchCoverageUnsafeInFiber.php b/plugin/codecov/src/Exception/BranchCoverageUnsafeInFiber.php index ddf16760..4d78cd3b 100644 --- a/plugin/codecov/src/Exception/BranchCoverageUnsafeInFiber.php +++ b/plugin/codecov/src/Exception/BranchCoverageUnsafeInFiber.php @@ -14,6 +14,8 @@ * when a fiber switch happens under an open coverage window. Testo closes the window around every * switch, which is enough from 3.4.5 on; older builds fault regardless, so the run is stopped with * this exception instead. + * + * @api */ final class BranchCoverageUnsafeInFiber extends \RuntimeException { diff --git a/plugin/codecov/src/Exception/CoverageDriverNotAvailable.php b/plugin/codecov/src/Exception/CoverageDriverNotAvailable.php index 30bf1b16..274f095b 100644 --- a/plugin/codecov/src/Exception/CoverageDriverNotAvailable.php +++ b/plugin/codecov/src/Exception/CoverageDriverNotAvailable.php @@ -6,6 +6,8 @@ /** * Thrown when no supported coverage driver extension is available. + * + * @api */ final class CoverageDriverNotAvailable extends \RuntimeException { diff --git a/plugin/codecov/src/Internal/CoverageDriver.php b/plugin/codecov/src/Internal/CoverageDriver.php index ae96e4af..710b4a88 100644 --- a/plugin/codecov/src/Internal/CoverageDriver.php +++ b/plugin/codecov/src/Internal/CoverageDriver.php @@ -11,7 +11,7 @@ /** * Interface for code coverage collection engines. * - * @api + * @internal */ interface CoverageDriver { diff --git a/plugin/codecov/src/Result/BranchCoverage.php b/plugin/codecov/src/Result/BranchCoverage.php index 30e80ec4..a01f55af 100644 --- a/plugin/codecov/src/Result/BranchCoverage.php +++ b/plugin/codecov/src/Result/BranchCoverage.php @@ -6,6 +6,8 @@ /** * A single branch within a function's control flow graph. + * + * @api */ final readonly class BranchCoverage { diff --git a/plugin/codecov/src/Result/CoverageResult.php b/plugin/codecov/src/Result/CoverageResult.php index f1de75a8..6583912d 100644 --- a/plugin/codecov/src/Result/CoverageResult.php +++ b/plugin/codecov/src/Result/CoverageResult.php @@ -6,6 +6,8 @@ /** * Aggregated code coverage data across multiple files. + * + * @api */ final readonly class CoverageResult { diff --git a/plugin/codecov/src/Result/FileCoverage.php b/plugin/codecov/src/Result/FileCoverage.php index 0bcaa7a3..36641f40 100644 --- a/plugin/codecov/src/Result/FileCoverage.php +++ b/plugin/codecov/src/Result/FileCoverage.php @@ -12,6 +12,8 @@ * Line coverage is always present. Branch and path coverage (via {@see $functions}) * is optional and only populated when collected with {@see CoverageLevel::Branch} * or {@see CoverageLevel::Path}. + * + * @api */ final readonly class FileCoverage { diff --git a/plugin/codecov/src/Result/FunctionCoverage.php b/plugin/codecov/src/Result/FunctionCoverage.php index 530fdd57..e7670046 100644 --- a/plugin/codecov/src/Result/FunctionCoverage.php +++ b/plugin/codecov/src/Result/FunctionCoverage.php @@ -6,6 +6,8 @@ /** * Branch and path coverage data for a single function or method. + * + * @api */ final readonly class FunctionCoverage { diff --git a/plugin/codecov/src/Result/LineCoverage.php b/plugin/codecov/src/Result/LineCoverage.php index 0535b57b..36cb66e3 100644 --- a/plugin/codecov/src/Result/LineCoverage.php +++ b/plugin/codecov/src/Result/LineCoverage.php @@ -10,6 +10,8 @@ * Carries the line's coverage status and the list of tests that executed it. * Per-test attribution is populated by {@see \Testo\Codecov\Internal\Middleware\CoverageTestInterceptor} * via {@see CoverageResult::withTestMethod()} and merged across runs by {@see self::merge()}. + * + * @api */ final readonly class LineCoverage { diff --git a/plugin/codecov/src/Result/LineStatus.php b/plugin/codecov/src/Result/LineStatus.php index ce593c25..925721aa 100644 --- a/plugin/codecov/src/Result/LineStatus.php +++ b/plugin/codecov/src/Result/LineStatus.php @@ -6,6 +6,8 @@ /** * Represents the coverage status of a single source code line. + * + * @api */ enum LineStatus: int { diff --git a/plugin/codecov/src/Result/PathCoverage.php b/plugin/codecov/src/Result/PathCoverage.php index be514c47..d43c2dbd 100644 --- a/plugin/codecov/src/Result/PathCoverage.php +++ b/plugin/codecov/src/Result/PathCoverage.php @@ -9,6 +9,8 @@ * * A path is an ordered sequence of branches (identified by their opcode start indices) * that were followed during execution. + * + * @api */ final readonly class PathCoverage {