From 22db75ec00f6f32c35f59f3c652acd4c36c4da0e Mon Sep 17 00:00:00 2001 From: Eljees <3.14hell@gmail.com> Date: Sun, 2 Aug 2026 15:20:33 +0300 Subject: [PATCH 1/2] Support PHPUnit Clover reports without metadata --- .../violations_reporter.py | 28 +++++++++++++++---- tests/test_violations_reporter.py | 15 ++++++++++ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/diff_cover/violationsreporters/violations_reporter.py b/diff_cover/violationsreporters/violations_reporter.py index 69a83dab..0bb369e7 100644 --- a/diff_cover/violationsreporters/violations_reporter.py +++ b/diff_cover/violationsreporters/violations_reporter.py @@ -129,11 +129,20 @@ def get_src_path_line_nodes_clover(xml_document, src_path): If file is not present in `xml_document`, return None """ - files = [ - file_tree - for file_tree in xml_document.findall(".//file") - if GitPathTool.relative_path(file_tree.get("path")) == src_path - ] + files = [] + normalized_src_path = util.to_unix_path(src_path) + for file_tree in xml_document.findall(".//file"): + file_path = file_tree.get("path") or file_tree.get("name") + if not file_path: + continue + + normalized_file_path = util.to_unix_path(file_path) + relative_file_path = util.to_unix_path(GitPathTool.relative_path(file_path)) + if ( + relative_file_path == normalized_src_path + or normalized_file_path.endswith(f"/{normalized_src_path}") + ): + files.append(file_tree) if not files: return None lines = [] @@ -142,6 +151,13 @@ def get_src_path_line_nodes_clover(xml_document, src_path): lines.append(file_tree.findall('./line[@type="cond"]')) return list(itertools.chain(*lines)) + @staticmethod + def _is_clover_report(xml_document): + return ( + bool(xml_document.findall(".[@clover]")) + or xml_document.find(".//file/line[@num][@count]") is not None + ) + def _measured_source_path_matches(self, package_name, file_name, src_path): # find src_path in any of the source roots if not src_path.endswith(util.to_unix_path(file_name)): @@ -204,7 +220,7 @@ def _cache_file(self, src_path): # Loop through the files that contain the xml roots for i, xml_document in enumerate(self._xml_roots): - if xml_document.findall(".[@clover]"): + if self._is_clover_report(xml_document): # see etc/schema/clover.xsd at https://bitbucket.org/atlassian/clover/src line_nodes = self.get_src_path_line_nodes_clover( xml_document, src_path diff --git a/tests/test_violations_reporter.py b/tests/test_violations_reporter.py index 2def2371..ff636e2b 100644 --- a/tests/test_violations_reporter.py +++ b/tests/test_violations_reporter.py @@ -497,6 +497,21 @@ def test_violations(self): result = coverage.violations("file1.java") assert result == violations + def test_phpunit_clover_without_clover_attribute(self): + xml = self._coverage_xml( + ["/workspace/project/subdir/file.java"], + self.FEW_VIOLATIONS, + self.FEW_MEASURED, + ) + del xml.attrib["clover"] + file_node = xml.find(".//file") + file_node.set("name", file_node.attrib.pop("path")) + + coverage = XmlCoverageReporter([xml]) + + assert coverage.violations("subdir/file.java") == self.FEW_VIOLATIONS + assert coverage.measured_lines("subdir/file.java") == self.FEW_MEASURED + def test_two_inputs_first_violate(self): # Construct the XML report file_paths = ["file1.java"] From 9a842f3e936a6acf35a3f6022b978261e7c11aaf Mon Sep 17 00:00:00 2001 From: Eljees <3.14hell@gmail.com> Date: Sat, 8 Aug 2026 09:40:53 +0300 Subject: [PATCH 2/2] Address review: classify each report once, and keep method lines Two points from the review of #617. Detecting the report format searched the whole document, and it was done once per source file inside _cache_file. The answer cannot change after load, so classify each root once in __init__ and look the answer up. This also folds in the JaCoCo probe, which had the same shape. Clover marks an executable line as method, stmt or cond. PHPUnit emits method for the declaration line of every function it measured, and leaving that type out reported those lines as unmeasured. This part is not specific to PHPUnit - it applies to any Clover writer. --- .../violations_reporter.py | 39 +++++++++++---- tests/test_violations_reporter.py | 50 ++++++++++++++++++- 2 files changed, 79 insertions(+), 10 deletions(-) diff --git a/diff_cover/violationsreporters/violations_reporter.py b/diff_cover/violationsreporters/violations_reporter.py index 0bb369e7..48d5ea11 100644 --- a/diff_cover/violationsreporters/violations_reporter.py +++ b/diff_cover/violationsreporters/violations_reporter.py @@ -46,6 +46,11 @@ def __init__( # Values are output of `self._get_xml_classes()` self._xml_cache = [{} for i in range(len(xml_roots))] + # Which format each report is in. Classifying costs a search through the + # document, and the answer cannot change, so do it once here rather than + # once per source file in `_cache_file`. + self._report_formats = [self._detect_report_format(root) for root in xml_roots] + self._src_roots = src_roots or [""] self._expand_coverage_report = expand_coverage_report self._branch_coverage = branch_coverage @@ -147,16 +152,34 @@ def get_src_path_line_nodes_clover(xml_document, src_path): return None lines = [] for file_tree in files: + # Clover marks an executable line as one of these three types. PHPUnit's + # writer emits `method` for the declaration line of every function it + # measured; leaving it out reported those lines as unmeasured. + # https://github.com/sebastianbergmann/php-code-coverage/blob/main/src/Report/Clover.php + lines.append(file_tree.findall('./line[@type="method"]')) lines.append(file_tree.findall('./line[@type="stmt"]')) lines.append(file_tree.findall('./line[@type="cond"]')) return list(itertools.chain(*lines)) @staticmethod - def _is_clover_report(xml_document): - return ( - bool(xml_document.findall(".[@clover]")) + def _detect_report_format(xml_document): + """ + Return which of the supported formats `xml_document` is in. + + Clover writes a `clover` attribute on the root, but PHPUnit's Clover + writer does not, so fall back to the shape of its line elements. + """ + if ( + xml_document.findall(".[@clover]") or xml_document.find(".//file/line[@num][@count]") is not None - ) + ): + # see etc/schema/clover.xsd at https://bitbucket.org/atlassian/clover/src + return "clover" + if xml_document.findall(".[@name]"): + # https://github.com/jacoco/jacoco/blob/master/org.jacoco.report/src/org/jacoco/report/xml/report.dtd + return "jacoco" + # https://github.com/cobertura/web/blob/master/htdocs/xml/coverage-04.dtd + return "cobertura" def _measured_source_path_matches(self, package_name, file_name, src_path): # find src_path in any of the source roots @@ -220,22 +243,20 @@ def _cache_file(self, src_path): # Loop through the files that contain the xml roots for i, xml_document in enumerate(self._xml_roots): - if self._is_clover_report(xml_document): - # see etc/schema/clover.xsd at https://bitbucket.org/atlassian/clover/src + report_format = self._report_formats[i] + if report_format == "clover": line_nodes = self.get_src_path_line_nodes_clover( xml_document, src_path ) _number = "num" _hits = "count" - elif xml_document.findall(".[@name]"): - # https://github.com/jacoco/jacoco/blob/master/org.jacoco.report/src/org/jacoco/report/xml/report.dtd + elif report_format == "jacoco": line_nodes = self.get_src_path_line_nodes_jacoco( xml_document, src_path ) _number = "nr" _hits = "ci" else: - # https://github.com/cobertura/web/blob/master/htdocs/xml/coverage-04.dtd line_nodes = self.get_src_path_line_nodes_cobertura( i, xml_document, src_path ) diff --git a/tests/test_violations_reporter.py b/tests/test_violations_reporter.py index ff636e2b..b67c099a 100644 --- a/tests/test_violations_reporter.py +++ b/tests/test_violations_reporter.py @@ -512,6 +512,45 @@ def test_phpunit_clover_without_clover_attribute(self): assert coverage.violations("subdir/file.java") == self.FEW_VIOLATIONS assert coverage.measured_lines("subdir/file.java") == self.FEW_MEASURED + def test_method_lines_are_measured(self): + # PHPUnit's Clover writer emits type="method" for the declaration line of + # every measured function. Those lines carry a count like any other and + # must not be reported as unmeasured. + xml = self._coverage_xml( + ["file1.java"], self.FEW_VIOLATIONS, self.FEW_MEASURED, method_lines={42} + ) + + coverage = XmlCoverageReporter([xml]) + + assert 42 in coverage.measured_lines("file1.java") + assert coverage.violations("file1.java") == self.FEW_VIOLATIONS + + def test_method_line_can_be_a_violation(self): + xml = self._coverage_xml( + ["file1.java"], self.FEW_VIOLATIONS, self.FEW_MEASURED, method_lines={42} + ) + method_line = xml.find('.//line[@type="method"]') + method_line.set("count", "0") + + coverage = XmlCoverageReporter([xml]) + + assert 42 in coverage.measured_lines("file1.java") + assert Violation(42, None) in coverage.violations("file1.java") + + def test_report_format_is_detected_once(self): + # Classifying the report is a search through the whole document; doing it + # per source file was the review comment on #617. + xml = self._coverage_xml( + ["file1.java", "file2.java"], self.FEW_VIOLATIONS, self.FEW_MEASURED + ) + + coverage = XmlCoverageReporter([xml]) + + assert coverage._report_formats == ["clover"] + coverage.violations("file1.java") + coverage.violations("file2.java") + assert coverage._report_formats == ["clover"] + def test_two_inputs_first_violate(self): # Construct the XML report file_paths = ["file1.java"] @@ -635,7 +674,7 @@ def test_no_such_file(self): result = coverage.violations("file.java") assert result == set() - def _coverage_xml(self, file_paths, violations, measured): + def _coverage_xml(self, file_paths, violations, measured, method_lines=None): """ Build an XML tree with source files specified by `file_paths`. Each source fill will have the same set of covered and @@ -645,6 +684,9 @@ def _coverage_xml(self, file_paths, violations, measured): `line_dict` is a dictionary with keys that are line numbers and values that are True/False indicating whether the line is covered + `method_lines` is an optional set of line numbers to emit as + `type="method"` instead of `type="stmt"`, the way PHPUnit's Clover + writer marks the declaration line of a measured function This leaves out some attributes of the Cobertura format, but includes all the elements. @@ -669,6 +711,12 @@ def _coverage_xml(self, file_paths, violations, measured): line.set("count", str(hits)) line.set("num", str(line_num)) line.set("type", "stmt") + + for line_num in method_lines or (): + line = etree.SubElement(src_node, "line") + line.set("count", "1") + line.set("num", str(line_num)) + line.set("type", "method") return root