diff --git a/CHANGELOG b/CHANGELOG index d2843867..c9a239ec 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,8 @@ +Unreleased + +* Add --branch-coverage flag to diff-cover to treat partially covered branches in Cobertura XML reports as uncovered + + 05/30/2026 v10.3.0 * Add --show-covered flag to highlight covered diff lines in HTML report [PR 600](https://github.com/Bachmann1234/diff_cover/pull/600) Thanks @duxiaocheng diff --git a/diff_cover/diff_cover_tool.py b/diff_cover/diff_cover_tool.py index 6bf49b6b..5d835988 100644 --- a/diff_cover/diff_cover_tool.py +++ b/diff_cover/diff_cover_tool.py @@ -52,6 +52,10 @@ EXPAND_COVERAGE_REPORT = ( "Append missing lines in coverage reports based on the hits of the previous line." ) +BRANCH_COVERAGE_HELP = ( + "Treat partially covered branches as uncovered. Requires a coverage report " + 'with branch coverage data (Cobertura branch="true" condition-coverage).' +) INCLUDE_UNTRACKED_HELP = "Include untracked files" CONFIG_FILE_HELP = "The configuration file to use" DIFF_FILE_HELP = "The diff file to use" @@ -117,6 +121,13 @@ def parse_coverage_args(argv): help=EXPAND_COVERAGE_REPORT, ) + parser.add_argument( + "--branch-coverage", + action="store_true", + default=None, + help=BRANCH_COVERAGE_HELP, + ) + parser.add_argument( "--external-css-file", metavar="FILENAME", @@ -215,6 +226,7 @@ def parse_coverage_args(argv): "diff_range_notation": "...", "quiet": False, "expand_coverage_report": False, + "branch_coverage": False, "total_percent_float": False, } @@ -237,6 +249,7 @@ def generate_coverage_report( show_uncovered=False, show_covered=False, expand_coverage_report=False, + branch_coverage=False, total_percent_float=False, ): """ @@ -265,7 +278,9 @@ def generate_coverage_report( if xml_roots and lcov_roots: raise ValueError("Mixing LCov and XML reports is not supported yet") if xml_roots: - coverage = XmlCoverageReporter(xml_roots, src_roots, expand_coverage_report) + coverage = XmlCoverageReporter( + xml_roots, src_roots, expand_coverage_report, branch_coverage + ) else: coverage = LcovCoverageReporter(lcov_roots, src_roots) @@ -417,6 +432,7 @@ def main(argv=None, directory=None): show_uncovered=arg_dict["show_uncovered"], show_covered=arg_dict["show_covered"], expand_coverage_report=arg_dict["expand_coverage_report"], + branch_coverage=arg_dict["branch_coverage"], total_percent_float=arg_dict["total_percent_float"], ) diff --git a/diff_cover/violationsreporters/violations_reporter.py b/diff_cover/violationsreporters/violations_reporter.py index 585cdc6e..69a83dab 100644 --- a/diff_cover/violationsreporters/violations_reporter.py +++ b/diff_cover/violationsreporters/violations_reporter.py @@ -24,7 +24,13 @@ class XmlCoverageReporter(BaseViolationReporter): Query information from a Cobertura|Clover|JaCoCo XML coverage report. """ - def __init__(self, xml_roots, src_roots=None, expand_coverage_report=False): + def __init__( + self, + xml_roots, + src_roots=None, + expand_coverage_report=False, + branch_coverage=False, + ): """ Load the XML coverage report represented by the cElementTree with root element `xml_root`. @@ -42,6 +48,11 @@ def __init__(self, xml_roots, src_roots=None, expand_coverage_report=False): self._src_roots = src_roots or [""] self._expand_coverage_report = expand_coverage_report + self._branch_coverage = branch_coverage + + # Pulls the "(covered/total)" pair out of Cobertura's + # condition-coverage attribute, e.g. "50% (1/2)". + self._cobertura_condition_coverage_re = re.compile(r"\((\d+)/(\d+)\)") def _get_xml_classes(self, xml_document): """ @@ -239,23 +250,19 @@ def _cache_file(self, src_path): {_hits: last_hit_number, _number: line_number} ) - # First case, need to define violations initially - if violations is None: - violations = { - Violation(int(line.get(_number)), None) - for line in line_nodes - if int(line.get(_hits, 0)) == 0 - } + new_violations = { + Violation(int(line.get(_number)), None) + for line in line_nodes + if self._is_violation(line, _hits) + } - # If we already have a violations set, - # take the intersection of the new - # violations set and its old self + # The first report defines the violations set. Each subsequent + # report narrows it, since we only keep violations that show up + # in every report. + if violations is None: + violations = new_violations else: - violations = violations & { - Violation(int(line.get(_number)), None) - for line in line_nodes - if int(line.get(_hits, 0)) == 0 - } + violations = violations & new_violations # Measured is the union of itself and the new measured measured = measured | {int(line.get(_number)) for line in line_nodes} @@ -267,6 +274,29 @@ def _cache_file(self, src_path): self._info_cache[src_path] = (violations, measured) + def _is_violation(self, line, hits_attr): + """ + Return whether a coverage `line` node counts as a violation. + + A line is a violation if it was never executed, or -- when branch + coverage is enabled -- if it is a partially covered branch. + """ + if int(line.get(hits_attr, 0)) == 0: + return True + return self._branch_coverage and self._is_partial_branch(line) + + def _is_partial_branch(self, line): + """Return whether a Cobertura `line` node is a partially covered branch.""" + if line.get("branch") != "true": + return False + match = self._cobertura_condition_coverage_re.search( + line.get("condition-coverage", "") + ) + if not match: + return False + covered, total = int(match.group(1)), int(match.group(2)) + return covered < total + def violations(self, src_path): """ See base class comments. diff --git a/tests/test_diff_cover_tool.py b/tests/test_diff_cover_tool.py index f4a0a611..72b11aa0 100644 --- a/tests/test_diff_cover_tool.py +++ b/tests/test_diff_cover_tool.py @@ -47,6 +47,16 @@ def test_show_covered_flag(): assert arg_dict["show_covered"] is True +def test_branch_coverage_defaults_false(): + arg_dict = parse_coverage_args(["reports/coverage.xml"]) + assert arg_dict["branch_coverage"] is False + + +def test_branch_coverage_flag(): + arg_dict = parse_coverage_args(["reports/coverage.xml", "--branch-coverage"]) + assert arg_dict["branch_coverage"] is True + + def test_parse_with_multiple_reports(): argv = [ "reports/coverage.xml", diff --git a/tests/test_violations_reporter.py b/tests/test_violations_reporter.py index 4b6ccba9..2def2371 100644 --- a/tests/test_violations_reporter.py +++ b/tests/test_violations_reporter.py @@ -333,6 +333,75 @@ def test_expand_unreported_lines_without_measured(self): assert set() == coverage.violations("file1.java") + def test_partial_branch_is_violation_when_branch_coverage_enabled(self): + # A line that executed (hits=1) but only exercised 1 of its 2 + # branches. With branch coverage enabled this should count as a + # violation, even though the statement itself ran. + xml = etree.fromstring(""" + + + + + + + + + + + + """) + + coverage = XmlCoverageReporter([xml], branch_coverage=True) + + # Line 2 is a partially covered branch -> violation. + # Line 3 is a fully covered branch, line 1 a plain statement -> covered. + assert coverage.violations("file1.py") == {Violation(2, None)} + assert coverage.measured_lines("file1.py") == {1, 2, 3} + + def test_partial_branch_ignored_by_default(self): + # Without branch coverage enabled, an executed-but-partially-covered + # branch stays covered, preserving the historical behaviour. + xml = etree.fromstring(""" + + + + + + + + + + + """) + + coverage = XmlCoverageReporter([xml]) + + assert coverage.violations("file1.py") == set() + assert coverage.measured_lines("file1.py") == {1, 2} + + def test_uncovered_branch_line_is_single_violation(self): + # A line that is both never executed (hits=0) and a partial branch is + # reported once, not twice. + xml = etree.fromstring(""" + + + + + + + + + + """) + + coverage = XmlCoverageReporter([xml], branch_coverage=True) + + assert coverage.violations("file1.py") == {Violation(1, None)} + def _coverage_xml(self, file_paths, violations, measured, source_paths=None): """ Build an XML tree with source files specified by `file_paths`.