From 67e6d707d476d91a643d46045dddc1804cecfa65 Mon Sep 17 00:00:00 2001 From: David Hagen Date: Wed, 29 Apr 2026 17:39:40 -0400 Subject: [PATCH 01/13] First pass --- pycobertura/cli.py | 103 +++++++- pycobertura/merge.py | 378 ++++++++++++++++++++++++++++++ tests/merge-branch-a.xml | 22 ++ tests/merge-branch-b.xml | 22 ++ tests/merge-branch-conflict-a.xml | 22 ++ tests/merge-branch-conflict-b.xml | 22 ++ tests/merge-linux.xml | 19 ++ tests/merge-overlap-a.xml | 22 ++ tests/merge-overlap-b.xml | 22 ++ tests/merge-python.xml | 26 ++ tests/merge-rust.xml | 28 +++ tests/merge-windows.xml | 19 ++ tests/test_cli.py | 140 +++++++++++ tests/test_merge.py | 260 ++++++++++++++++++++ 14 files changed, 1104 insertions(+), 1 deletion(-) create mode 100644 pycobertura/merge.py create mode 100644 tests/merge-branch-a.xml create mode 100644 tests/merge-branch-b.xml create mode 100644 tests/merge-branch-conflict-a.xml create mode 100644 tests/merge-branch-conflict-b.xml create mode 100644 tests/merge-linux.xml create mode 100644 tests/merge-overlap-a.xml create mode 100644 tests/merge-overlap-b.xml create mode 100644 tests/merge-python.xml create mode 100644 tests/merge-rust.xml create mode 100644 tests/merge-windows.xml create mode 100644 tests/test_merge.py diff --git a/pycobertura/cli.py b/pycobertura/cli.py index 163368c..ce704ea 100644 --- a/pycobertura/cli.py +++ b/pycobertura/cli.py @@ -1,6 +1,12 @@ import click +import lxml.etree as ET from pycobertura.cobertura import Cobertura, CoberturaDiff +from pycobertura.merge import ( + BranchConflictError, + merge_reports, + parse_source_path_pattern, +) from pycobertura.reporters import ( GitHubAnnotationReporter, HtmlReporter, @@ -18,7 +24,10 @@ GitHubAnnotationReporterDelta, ) from pycobertura.filesystem import filesystem_factory -from pycobertura.utils import get_dir_from_file_path +from pycobertura.utils import ( + get_dir_from_file_path, + get_filenames_that_do_not_match_regex, +) pycobertura = click.Group() @@ -351,3 +360,95 @@ def diff( exit_code = get_exit_code(reporter.differ, source) raise SystemExit(exit_code) + + +def _source_path_callback(ctx, param, value): + parsed = [] + for v in value: + try: + parsed.append(parse_source_path_pattern(v)) + except ValueError as e: + raise click.BadParameter(str(e), ctx=ctx, param=param) + return parsed + + +@pycobertura.command(help="""\ +Combine multiple Cobertura XML reports into a single merged report. + +Hits are summed across reports for matching lines; files are unioned across +reports. Branch coverage is merged by taking the entry with the highest +covered/total ratio. If two reports disagree on the number of branch +conditions for the same line (e.g. due to conditional compilation), the +merge aborts with an error. + +Use --source-path to canonicalize file paths that differ across reports +(e.g. between Linux and Windows runs). The pattern identifies a directory +that begins the canonical path; everything before it in the input filename +is stripped. A leading '**/' is allowed for clarity. + +EXAMPLES + + pycobertura merge linux.xml windows.xml --source-path src/ + + pycobertura merge a.xml b.xml c.xml -o merged.xml +""") +@click.argument("cobertura_files", nargs=-1, required=True) +@click.option( + "--ignore-regex", + default=None, + type=str, + help="Regex for which files to ignore from each input before merging.", +) +@click.option( + "--source-path", + "source_paths", + multiple=True, + metavar="", + callback=_source_path_callback, + help="Path pattern (e.g. 'src/' or '**/site-packages/') identifying the " + "start of the canonical source path. Repeatable. The first matching " + "pattern wins per filename.", +) +@click.option( + "-o", + "--output", + metavar="", + type=click.File("wb"), + help="Write merged XML to instead of stdout.", +) +def merge(cobertura_files, ignore_regex, source_paths, output): + """combine multiple Cobertura reports into one""" + roots = [] + for path in cobertura_files: + try: + root = ET.parse(path).getroot() + except (ET.XMLSyntaxError, OSError) as e: + raise click.ClickException(f"Failed to read {path}: {e}") + if ignore_regex: + _drop_ignored_classes(root, ignore_regex) + roots.append(root) + + try: + merged = merge_reports(roots, source_paths=source_paths) + except BranchConflictError as e: + raise click.ClickException(str(e)) + + report = ET.tostring( + merged, + xml_declaration=True, + encoding="UTF-8", + pretty_print=True, + ) + + isatty = True if output is None else output.isatty() + click.echo(report, file=output, nl=isatty) + + +def _drop_ignored_classes(root, ignore_regex): + """Remove elements whose filename matches ignore_regex from a parsed tree.""" + classes = root.xpath("./packages/package/classes/class") + filenames = [cls.get("filename", "") for cls in classes] + keep = set(get_filenames_that_do_not_match_regex(filenames, ignore_regex)) + for cls in classes: + if cls.get("filename", "") not in keep: + cls.getparent().remove(cls) diff --git a/pycobertura/merge.py b/pycobertura/merge.py new file mode 100644 index 0000000..ae6e403 --- /dev/null +++ b/pycobertura/merge.py @@ -0,0 +1,378 @@ +"""Merge multiple Cobertura XML reports into one. + +The public entry point is :func:`merge_reports`, which takes a sequence of +parsed lxml roots and returns a freshly-built merged root. Inputs are treated +as read-only and never mutated. +""" + +from __future__ import annotations + +import copy +import re +import time +from collections import OrderedDict +from typing import List, Sequence, Tuple + +import lxml.etree as ET + +try: + from importlib.metadata import version as _pkg_version +except ImportError: # pragma: no cover + from importlib_metadata import version as _pkg_version + + +_CONDITION_COVERAGE_RE = re.compile(r"^\s*(\d+)%\s*\((\d+)/(\d+)\)\s*$") + + +class BranchConflictError(Exception): + """Raised when two reports disagree on the number of branch conditions for + the same source line, which implies the line was generated from different + code (e.g. conditional compilation) and cannot be safely merged.""" + + +def parse_source_path_pattern(pattern: str) -> List[str]: + """Validate and tokenize a ``--source-path`` value into literal segments. + + A leading ``**/`` is allowed (and stripped); no other wildcards are + permitted in v1. Pattern must end with ``/``. + """ + if not pattern.endswith("/"): + raise ValueError( + f"--source-path pattern must end with '/': {pattern!r}" + ) + + body = pattern[:-1] + if body.startswith("**/"): + body = body[len("**/"):] + elif body == "**": + raise ValueError( + f"--source-path pattern is empty after stripping leading '**/': {pattern!r}" + ) + + if not body: + raise ValueError(f"--source-path pattern has no literal segments: {pattern!r}") + + segments = body.split("/") + for seg in segments: + if not seg: + raise ValueError( + f"--source-path pattern has empty segments: {pattern!r}" + ) + if "*" in seg: + raise ValueError( + f"--source-path pattern uses unsupported wildcard in {seg!r} " + f"(only a leading '**/' is supported in v1): {pattern!r}" + ) + + return segments + + +def canonicalize_filename( + filename: str, patterns: Sequence[Sequence[str]] +) -> str: + """Normalize separators and apply source-path remapping to ``filename``. + + ``patterns`` is the list of segment-lists produced by + :func:`parse_source_path_pattern`. The first pattern (in declaration order) + that matches at the leftmost directory boundary wins; the canonical name is + the suffix starting at the match. If no pattern matches, the filename is + returned with backslashes normalized to forward slashes. + """ + norm = filename.replace("\\", "/") + if not patterns: + return norm + + segments = norm.split("/") + for pattern_segs in patterns: + plen = len(pattern_segs) + if plen == 0: + continue + for start in range(len(segments) - plen + 1): + if segments[start:start + plen] == list(pattern_segs): + return "/".join(segments[start:]) + return norm + + +def merge_reports( + xml_roots: Sequence[ET._Element], + source_paths: Sequence[Sequence[str]] = (), +) -> ET._Element: + """Merge a sequence of Cobertura ```` roots into one fresh root. + + Inputs are treated as read-only. Raises :class:`BranchConflictError` when + two inputs report different branch-condition denominators for the same + line. + """ + if not xml_roots: + raise ValueError("merge_reports requires at least one input root") + + sources_seen: "OrderedDict[str, None]" = OrderedDict() + # packages_data: package_name -> OrderedDict[(class_name, canonical_filename) -> list[]] + packages_data: "OrderedDict[str, OrderedDict[Tuple[str, str], List[ET._Element]]]" = OrderedDict() + + for root in xml_roots: + for src in root.iterfind("./sources/source"): + text = (src.text or "").strip() + if text: + sources_seen.setdefault(text, None) + + for pkg in root.iterfind("./packages/package"): + pkg_name = pkg.get("name", "") or "" + pkg_classes = packages_data.setdefault(pkg_name, OrderedDict()) + for cls in pkg.iterfind("./classes/class"): + class_name = cls.get("name", "") or "" + filename = cls.get("filename", "") or "" + canonical = canonicalize_filename(filename, source_paths) + key = (class_name, canonical) + pkg_classes.setdefault(key, []).append(cls) + + new_root = ET.Element("coverage") + new_root.set("timestamp", str(int(time.time() * 1000))) + new_root.set("version", _producer_string()) + + sources_elem = ET.SubElement(new_root, "sources") + for src_text in sources_seen: + s = ET.SubElement(sources_elem, "source") + s.text = src_text + + packages_elem = ET.SubElement(new_root, "packages") + + total_lines = 0 + total_hits = 0 + total_branches = 0 + total_branch_hits = 0 + + for pkg_name, classes_map in packages_data.items(): + pkg_elem = ET.SubElement(packages_elem, "package") + pkg_elem.set("name", pkg_name) + classes_elem = ET.SubElement(pkg_elem, "classes") + + pkg_lines = 0 + pkg_hits = 0 + pkg_branches = 0 + pkg_branch_hits = 0 + pkg_complexity = None + + for (class_name, canonical_filename), versions in classes_map.items(): + cls_elem, c_lines, c_hits, c_branches, c_branch_hits, c_complexity = ( + _merge_class(class_name, canonical_filename, versions) + ) + classes_elem.append(cls_elem) + + pkg_lines += c_lines + pkg_hits += c_hits + pkg_branches += c_branches + pkg_branch_hits += c_branch_hits + if c_complexity is not None: + pkg_complexity = ( + c_complexity if pkg_complexity is None + else max(pkg_complexity, c_complexity) + ) + + pkg_elem.set("line-rate", _rate_str(pkg_hits, pkg_lines)) + pkg_elem.set("branch-rate", _rate_str(pkg_branch_hits, pkg_branches)) + if pkg_complexity is not None: + pkg_elem.set("complexity", _format_float(pkg_complexity)) + + total_lines += pkg_lines + total_hits += pkg_hits + total_branches += pkg_branches + total_branch_hits += pkg_branch_hits + + new_root.set("line-rate", _rate_str(total_hits, total_lines)) + new_root.set("branch-rate", _rate_str(total_branch_hits, total_branches)) + new_root.set("lines-valid", str(total_lines)) + new_root.set("lines-covered", str(total_hits)) + new_root.set("branches-valid", str(total_branches)) + new_root.set("branches-covered", str(total_branch_hits)) + + return new_root + + +def _merge_class( + class_name: str, + canonical_filename: str, + versions: Sequence[ET._Element], +) -> Tuple[ET._Element, int, int, int, int, "float | None"]: + cls_elem = ET.Element("class") + cls_elem.set("name", class_name) + cls_elem.set("filename", canonical_filename) + + # Group method line-elements by (name, signature) -> dict[line_number -> list[]] + methods_data: "OrderedDict[Tuple[str, str], OrderedDict[int, List[ET._Element]]]" = OrderedDict() + # Class-level lines (direct ./lines/line on ): line_number -> list[] + class_lines: "OrderedDict[int, List[ET._Element]]" = OrderedDict() + + for v in versions: + for m in v.iterfind("./methods/method"): + key = (m.get("name", "") or "", m.get("signature", "") or "") + method_lines = methods_data.setdefault(key, OrderedDict()) + for line in m.iterfind("./lines/line"): + n = int(line.get("number")) + method_lines.setdefault(n, []).append(line) + + for line in v.iterfind("./lines/line"): + n = int(line.get("number")) + class_lines.setdefault(n, []).append(line) + + methods_elem = ET.SubElement(cls_elem, "methods") + for (m_name, m_sig), lines_by_number in methods_data.items(): + m_elem = ET.SubElement(methods_elem, "method") + m_elem.set("name", m_name) + m_elem.set("signature", m_sig) + m_lines_elem = ET.SubElement(m_elem, "lines") + m_lines_count, m_hits, m_branches, m_branch_hits = _emit_merged_lines( + lines_by_number, m_lines_elem, canonical_filename + ) + m_elem.set("line-rate", _rate_str(m_hits, m_lines_count)) + m_elem.set("branch-rate", _rate_str(m_branch_hits, m_branches)) + + # Class-level : merge from inputs' direct ./lines/line. + # For inputs that store data only in methods (class-level is empty or absent), + # fall back to the union of merged method lines so the class-level block is populated. + cls_lines_elem = ET.SubElement(cls_elem, "lines") + if class_lines: + c_total, c_hits, c_branches, c_branch_hits = _emit_merged_lines( + class_lines, cls_lines_elem, canonical_filename + ) + else: + # Build from union of merged method lines. + union: "OrderedDict[int, List[ET._Element]]" = OrderedDict() + for lines_by_number in methods_data.values(): + for n, line_elems in lines_by_number.items(): + union.setdefault(n, []).extend(line_elems) + c_total, c_hits, c_branches, c_branch_hits = _emit_merged_lines( + union, cls_lines_elem, canonical_filename + ) + + cls_elem.set("line-rate", _rate_str(c_hits, c_total)) + cls_elem.set("branch-rate", _rate_str(c_branch_hits, c_branches)) + + complexity = None + for v in versions: + c = v.get("complexity") + if c is None: + continue + try: + f = float(c) + except ValueError: + continue + complexity = f if complexity is None else max(complexity, f) + if complexity is not None: + cls_elem.set("complexity", _format_float(complexity)) + + return cls_elem, c_total, c_hits, c_branches, c_branch_hits, complexity + + +def _emit_merged_lines( + lines_by_number: "OrderedDict[int, List[ET._Element]]", + parent: ET._Element, + filename: str, +) -> Tuple[int, int, int, int]: + """Append merged children to ``parent``. Returns (total, hits, branches, branch_hits).""" + total = 0 + hits_count = 0 + branches = 0 + branch_hits = 0 + + for n in sorted(lines_by_number): + line_elems = lines_by_number[n] + merged = _merge_line_elements(n, line_elems, filename) + parent.append(merged) + total += 1 + if _line_is_hit(merged): + hits_count += 1 + cc = merged.get("condition-coverage") + if cc: + parsed = _parse_condition_coverage(cc) + if parsed is not None: + a, b = parsed + branches += b + branch_hits += a + + return total, hits_count, branches, branch_hits + + +def _merge_line_elements( + line_no: int, line_elems: Sequence[ET._Element], filename: str +) -> ET._Element: + new_line = ET.Element("line") + new_line.set("number", str(line_no)) + + total_hits = sum(int(el.get("hits", "0") or "0") for el in line_elems) + new_line.set("hits", str(total_hits)) + + branch_elems = [el for el in line_elems if el.get("branch") == "true"] + is_branch = bool(branch_elems) + new_line.set("branch", "true" if is_branch else "false") + + if not is_branch: + return new_line + + parsed_entries = [] # list of (a, b, source_element) + denom = None + for el in branch_elems: + cc = el.get("condition-coverage") + if not cc: + continue + parsed = _parse_condition_coverage(cc) + if parsed is None: + continue + a, b = parsed + if denom is None: + denom = b + elif b != denom: + raise BranchConflictError( + f"Conflicting branch counts at {filename}:{line_no}: " + f"{denom} vs {b}" + ) + parsed_entries.append((a, b, el)) + + if not parsed_entries: + return new_line + + best_a, best_b, best_el = max(parsed_entries, key=lambda x: x[0]) + pct = int(round(100 * best_a / best_b)) if best_b > 0 else 0 + new_line.set("condition-coverage", f"{pct}% ({best_a}/{best_b})") + + conditions = best_el.find("conditions") + if conditions is not None: + new_line.append(copy.deepcopy(conditions)) + + return new_line + + +def _line_is_hit(line_elem: ET._Element) -> bool: + cc = line_elem.get("condition-coverage") + if cc: + return cc.lstrip().startswith("100%") + try: + return int(line_elem.get("hits", "0") or "0") > 0 + except ValueError: + return False + + +def _parse_condition_coverage(cc: str) -> "Tuple[int, int] | None": + m = _CONDITION_COVERAGE_RE.match(cc) + if not m: + return None + return int(m.group(2)), int(m.group(3)) + + +def _rate_str(numerator: int, denominator: int) -> str: + if denominator == 0: + return "0" + return _format_float(numerator / denominator) + + +def _format_float(value: float) -> str: + # Match the formatting style of typical Cobertura output (e.g. "0.875"). + s = f"{value:.6g}" + return s + + +def _producer_string() -> str: + try: + return f"pycobertura-{_pkg_version('pycobertura')}" + except Exception: # pragma: no cover + return "pycobertura" diff --git a/tests/merge-branch-a.xml b/tests/merge-branch-a.xml new file mode 100644 index 0000000..d9dd40b --- /dev/null +++ b/tests/merge-branch-a.xml @@ -0,0 +1,22 @@ + + + + . + + + + + + + + + + + + + + + + + + diff --git a/tests/merge-branch-b.xml b/tests/merge-branch-b.xml new file mode 100644 index 0000000..7cc74d0 --- /dev/null +++ b/tests/merge-branch-b.xml @@ -0,0 +1,22 @@ + + + + . + + + + + + + + + + + + + + + + + + diff --git a/tests/merge-branch-conflict-a.xml b/tests/merge-branch-conflict-a.xml new file mode 100644 index 0000000..d9dd40b --- /dev/null +++ b/tests/merge-branch-conflict-a.xml @@ -0,0 +1,22 @@ + + + + . + + + + + + + + + + + + + + + + + + diff --git a/tests/merge-branch-conflict-b.xml b/tests/merge-branch-conflict-b.xml new file mode 100644 index 0000000..44fd916 --- /dev/null +++ b/tests/merge-branch-conflict-b.xml @@ -0,0 +1,22 @@ + + + + . + + + + + + + + + + + + + + + + + + diff --git a/tests/merge-linux.xml b/tests/merge-linux.xml new file mode 100644 index 0000000..4cb0b24 --- /dev/null +++ b/tests/merge-linux.xml @@ -0,0 +1,19 @@ + + + + /home/runner/work/repo + + + + + + + + + + + + + + + diff --git a/tests/merge-overlap-a.xml b/tests/merge-overlap-a.xml new file mode 100644 index 0000000..f027789 --- /dev/null +++ b/tests/merge-overlap-a.xml @@ -0,0 +1,22 @@ + + + + . + + + + + + + + + + + + + + + + + + diff --git a/tests/merge-overlap-b.xml b/tests/merge-overlap-b.xml new file mode 100644 index 0000000..9e7aa61 --- /dev/null +++ b/tests/merge-overlap-b.xml @@ -0,0 +1,22 @@ + + + + . + + + + + + + + + + + + + + + + + + diff --git a/tests/merge-python.xml b/tests/merge-python.xml new file mode 100644 index 0000000..7937808 --- /dev/null +++ b/tests/merge-python.xml @@ -0,0 +1,26 @@ + + + + app + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/merge-rust.xml b/tests/merge-rust.xml new file mode 100644 index 0000000..af8c71e --- /dev/null +++ b/tests/merge-rust.xml @@ -0,0 +1,28 @@ + + + + src + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/merge-windows.xml b/tests/merge-windows.xml new file mode 100644 index 0000000..88633a9 --- /dev/null +++ b/tests/merge-windows.xml @@ -0,0 +1,19 @@ + + + + C:\Users\runner\work\repo + + + + + + + + + + + + + + + diff --git a/tests/test_cli.py b/tests/test_cli.py index e699e82..1c1d4d7 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1255,3 +1255,143 @@ def test_diff__format_yaml__with_ignore_regex(): # Verify total is present assert 'Filename: TOTAL' in result.output + + +# ---- merge command ---- + + +def test_merge__stdout_output(): + from pycobertura.cli import merge + + runner = CliRunner() + result = runner.invoke( + merge, + ['tests/merge-rust.xml', 'tests/merge-python.xml'], + catch_exceptions=False, + ) + assert result.exit_code == 0 + # Output is XML; check it parses and contains expected files. + import lxml.etree as ET + root = ET.fromstring(result.output.encode("utf-8") if isinstance(result.output, str) else result.output) + files = sorted(c.get("filename") for c in root.xpath(".//class")) + assert files == sorted(["src/lib.rs", "src/parser.rs", "app/main.py", "app/util.py"]) + + +def test_merge__output_to_file(tmp_path): + from pycobertura.cli import merge + + runner = CliRunner() + out = tmp_path / "merged.xml" + result = runner.invoke( + merge, + ['tests/merge-rust.xml', 'tests/merge-python.xml', '-o', str(out)], + catch_exceptions=False, + ) + assert result.exit_code == 0 + assert out.exists() + import lxml.etree as ET + root = ET.parse(str(out)).getroot() + assert root.tag == "coverage" + + +def test_merge__single_input_roundtrip(): + from pycobertura.cli import merge + + runner = CliRunner() + result = runner.invoke( + merge, + ['tests/merge-overlap-a.xml'], + catch_exceptions=False, + ) + assert result.exit_code == 0 + + +def test_merge__requires_at_least_one_input(): + from pycobertura.cli import merge + + runner = CliRunner() + result = runner.invoke(merge, [], catch_exceptions=False) + assert result.exit_code != 0 + + +def test_merge__source_path_unifies_cross_os_paths(): + from pycobertura.cli import merge + + runner = CliRunner() + result = runner.invoke( + merge, + [ + 'tests/merge-linux.xml', + 'tests/merge-windows.xml', + '--source-path', 'src/', + ], + catch_exceptions=False, + ) + assert result.exit_code == 0 + import lxml.etree as ET + root = ET.fromstring(result.output.encode("utf-8") if isinstance(result.output, str) else result.output) + files = sorted(c.get("filename") for c in root.xpath(".//class")) + assert files == ["src/main.py"] + + +def test_merge__source_path_rejects_mid_pattern_wildcard(): + from pycobertura.cli import merge + + runner = CliRunner() + result = runner.invoke( + merge, + ['tests/merge-rust.xml', '--source-path', 'src/*/foo/'], + catch_exceptions=False, + ) + assert result.exit_code != 0 + assert "unsupported wildcard" in result.output + + +def test_merge__source_path_rejects_missing_trailing_slash(): + from pycobertura.cli import merge + + runner = CliRunner() + result = runner.invoke( + merge, + ['tests/merge-rust.xml', '--source-path', 'src'], + catch_exceptions=False, + ) + assert result.exit_code != 0 + assert "must end with '/'" in result.output + + +def test_merge__branch_conflict_exits_with_error(): + from pycobertura.cli import merge + + runner = CliRunner() + result = runner.invoke( + merge, + [ + 'tests/merge-branch-conflict-a.xml', + 'tests/merge-branch-conflict-b.xml', + ], + catch_exceptions=False, + ) + assert result.exit_code != 0 + assert "app/main.py:10" in result.output + + +def test_merge__ignore_regex_drops_files_pre_merge(): + from pycobertura.cli import merge + + runner = CliRunner() + result = runner.invoke( + merge, + [ + 'tests/merge-rust.xml', + 'tests/merge-python.xml', + '--ignore-regex', r'.*\.rs$', + ], + catch_exceptions=False, + ) + assert result.exit_code == 0 + import lxml.etree as ET + root = ET.fromstring(result.output.encode("utf-8") if isinstance(result.output, str) else result.output) + files = sorted(c.get("filename") for c in root.xpath(".//class")) + assert all(not f.endswith(".rs") for f in files) + assert "app/main.py" in files diff --git a/tests/test_merge.py b/tests/test_merge.py new file mode 100644 index 0000000..9b9b8d9 --- /dev/null +++ b/tests/test_merge.py @@ -0,0 +1,260 @@ +import copy + +import lxml.etree as ET +import pytest + +from pycobertura.merge import ( + BranchConflictError, + canonicalize_filename, + merge_reports, + parse_source_path_pattern, +) + + +# ---- parse_source_path_pattern ---- + + +def test_parse_source_path_pattern_literal(): + assert parse_source_path_pattern("src/") == ["src"] + + +def test_parse_source_path_pattern_multi_segment(): + assert parse_source_path_pattern("a/b/c/") == ["a", "b", "c"] + + +def test_parse_source_path_pattern_strips_leading_double_star(): + assert parse_source_path_pattern("**/site-packages/") == ["site-packages"] + assert parse_source_path_pattern("**/a/b/") == ["a", "b"] + + +def test_parse_source_path_pattern_rejects_missing_trailing_slash(): + with pytest.raises(ValueError, match="must end with '/'"): + parse_source_path_pattern("src") + + +def test_parse_source_path_pattern_rejects_mid_pattern_wildcard(): + with pytest.raises(ValueError, match="unsupported wildcard"): + parse_source_path_pattern("src/*/foo/") + with pytest.raises(ValueError, match="unsupported wildcard"): + parse_source_path_pattern("src/**/foo/") + + +def test_parse_source_path_pattern_rejects_only_double_star(): + with pytest.raises(ValueError, match="empty after stripping"): + parse_source_path_pattern("**/") + + +def test_parse_source_path_pattern_rejects_empty_segment(): + with pytest.raises(ValueError, match="empty segments"): + parse_source_path_pattern("a//b/") + + +# ---- canonicalize_filename ---- + + +def test_canonicalize_filename_no_patterns_normalizes_separators(): + assert canonicalize_filename(r"a\b\c.py", []) == "a/b/c.py" + + +def test_canonicalize_filename_matches_segment(): + patterns = [["src"]] + assert canonicalize_filename("/home/runner/work/repo/src/foo.py", patterns) == "src/foo.py" + + +def test_canonicalize_filename_matches_windows_path(): + patterns = [["src"]] + assert canonicalize_filename(r"C:\Users\runner\work\repo\src\foo.py", patterns) == "src/foo.py" + + +def test_canonicalize_filename_matches_at_start(): + patterns = [["src"]] + assert canonicalize_filename("src/foo.py", patterns) == "src/foo.py" + + +def test_canonicalize_filename_matches_multi_segment_pattern(): + patterns = [["src", "main"]] + assert canonicalize_filename("/foo/bar/src/main/python/app.py", patterns) == "src/main/python/app.py" + + +def test_canonicalize_filename_no_match_normalizes_only(): + patterns = [["src"]] + assert canonicalize_filename(r"foo\bar\baz.py", patterns) == "foo/bar/baz.py" + + +def test_canonicalize_filename_first_pattern_wins(): + patterns = [["src"], ["site-packages"]] + # Both could potentially match if path had both; first declared wins. + assert canonicalize_filename("/x/site-packages/src/foo.py", patterns) == "src/foo.py" + + +def test_canonicalize_filename_leftmost_match_wins(): + patterns = [["src"]] + # Two `src` directories — leftmost wins. + assert canonicalize_filename("/a/src/b/src/c.py", patterns) == "src/b/src/c.py" + + +def test_canonicalize_filename_partial_segment_does_not_match(): + patterns = [["src"]] + # 'mysrc' should not match 'src'. + assert canonicalize_filename("/foo/mysrc/bar.py", patterns) == "/foo/mysrc/bar.py" + + +# ---- merge_reports ---- + + +def _parse(path): + return ET.parse(path).getroot() + + +def _files(root): + return sorted(cls.get("filename") for cls in root.xpath(".//class")) + + +def _line(root, filename, lineno): + """Return the merged element for (filename, lineno) at the class level.""" + classes = root.xpath(f".//class[@filename={filename!r}]") + for cls in classes: + for line in cls.xpath("./lines/line"): + if int(line.get("number")) == lineno: + return line + return None + + +def test_merge_disjoint_files(): + a = _parse("tests/merge-rust.xml") + b = _parse("tests/merge-python.xml") + merged = merge_reports([a, b]) + files = _files(merged) + assert files == sorted(["src/lib.rs", "src/parser.rs", "app/main.py", "app/util.py"]) + + +def test_merge_overlapping_files_sums_hits_and_unions_lines(): + a = _parse("tests/merge-overlap-a.xml") + b = _parse("tests/merge-overlap-b.xml") + merged = merge_reports([a, b]) + + # Line 3: hit=2 (a) + 1 (b) = 3 + assert _line(merged, "app/main.py", 3).get("hits") == "3" + # Line 1: only in a, hits=2 + assert _line(merged, "app/main.py", 1).get("hits") == "2" + # Line 7: only in b, hits=0 + assert _line(merged, "app/main.py", 7).get("hits") == "0" + # Union of lines: {1,2,3,4,5,6,7} + line_numbers = sorted(int(l.get("number")) for l in merged.xpath(".//class[@filename='app/main.py']/lines/line")) + assert line_numbers == [1, 2, 3, 4, 5, 6, 7] + + +def test_merge_branch_lines_takes_max_coverage(): + a = _parse("tests/merge-branch-a.xml") # 1/2 + b = _parse("tests/merge-branch-b.xml") # 2/2 + merged = merge_reports([a, b]) + line = _line(merged, "app/main.py", 10) + assert line.get("branch") == "true" + assert line.get("condition-coverage") == "100% (2/2)" + # hits are summed + assert line.get("hits") == "8" + + +def test_merge_branch_denominator_conflict_raises(): + a = _parse("tests/merge-branch-conflict-a.xml") # 1/2 + b = _parse("tests/merge-branch-conflict-b.xml") # 1/3 + with pytest.raises(BranchConflictError, match="app/main.py:10"): + merge_reports([a, b]) + + +def test_merge_with_source_paths_unifies_cross_os_paths(): + linux = _parse("tests/merge-linux.xml") + windows = _parse("tests/merge-windows.xml") + patterns = [parse_source_path_pattern("src/")] + merged = merge_reports([linux, windows], source_paths=patterns) + + # Both inputs' main.py merge under canonical path + files = _files(merged) + assert files == ["src/main.py"] + + # Line 2 appears in both → hits = 2 + 3 = 5 + assert _line(merged, "src/main.py", 2).get("hits") == "5" + # Line 1 only in linux + assert _line(merged, "src/main.py", 1).get("hits") == "2" + # Line 3 only in windows + assert _line(merged, "src/main.py", 3).get("hits") == "3" + + +def test_merge_multi_class_per_file_preserved(): + # Java-style: two classes share a filename. cobertura.xml has Main and Main$Helper. + a = _parse("tests/cobertura.xml") + merged = merge_reports([a]) + + main_classes = merged.xpath(".//class[@filename='Main.java']") + names = sorted(cls.get("name") for cls in main_classes) + assert names == ["Main", "Main$Helper"] + + +def test_merge_no_branch_rate_input(): + a = _parse("tests/cobertura-no-branch-rate.xml") + b = _parse("tests/merge-python.xml") + # Should not raise + merged = merge_reports([a, b]) + assert merged is not None + + +def test_merge_empty_methods_handled(): + # search.ISortedArraySearch in cobertura.xml has empty and empty . + a = _parse("tests/cobertura.xml") + merged = merge_reports([a]) + # The class is still present in the merged output. + isarr = merged.xpath(".//class[@name='search.ISortedArraySearch']") + assert len(isarr) == 1 + + +def test_merge_single_input_roundtrip(): + a = _parse("tests/merge-overlap-a.xml") + merged = merge_reports([a]) + # Same files, same lines, hits unchanged + line3 = _line(merged, "app/main.py", 3) + assert line3.get("hits") == "2" + line5 = _line(merged, "app/main.py", 5) + assert line5.get("hits") == "0" + + +def test_merge_does_not_mutate_inputs(): + a = _parse("tests/merge-overlap-a.xml") + b = _parse("tests/merge-overlap-b.xml") + before_a = ET.tostring(a) + before_b = ET.tostring(b) + merge_reports([a, b]) + assert ET.tostring(a) == before_a + assert ET.tostring(b) == before_b + + +def test_merge_sets_pycobertura_version(): + a = _parse("tests/merge-overlap-a.xml") + merged = merge_reports([a]) + assert merged.get("version", "").startswith("pycobertura-") + + +def test_merge_unions_sources(): + a = _parse("tests/merge-rust.xml") # src + b = _parse("tests/merge-python.xml") # app + merged = merge_reports([a, b]) + sources = [s.text for s in merged.xpath("./sources/source")] + assert "src" in sources + assert "app" in sources + + +def test_merge_aggregates_root_line_rate(): + # rust file has 6 lines (4 in lib.rs: 2 hit + 2 miss; 2 in parser.rs: 1 hit + 1 miss) + # python file has 4 lines (2 in main.py: 2 hit; 2 in util.py: 1 hit + 1 miss) + # Total: 10 lines, 6 hit + a = _parse("tests/merge-rust.xml") + b = _parse("tests/merge-python.xml") + merged = merge_reports([a, b]) + assert merged.get("lines-valid") == "10" + assert merged.get("lines-covered") == "6" + # line-rate = 6/10 = 0.6 + assert float(merged.get("line-rate")) == pytest.approx(0.6) + + +def test_merge_raises_on_empty_input(): + with pytest.raises(ValueError): + merge_reports([]) From e77a0cd0e8236e87364e860d4dafed5199b11e79 Mon Sep 17 00:00:00 2001 From: David Hagen Date: Wed, 6 May 2026 13:50:03 -0400 Subject: [PATCH 02/13] Denominator mismatches now longer error --- MERGE_SPEC.md | 304 +++++++++++++++++++++++++++++++++++++++++++ pycobertura/cli.py | 52 ++++---- pycobertura/merge.py | 33 ++--- tests/test_cli.py | 14 +- tests/test_merge.py | 44 ++++++- 5 files changed, 389 insertions(+), 58 deletions(-) create mode 100644 MERGE_SPEC.md diff --git a/MERGE_SPEC.md b/MERGE_SPEC.md new file mode 100644 index 0000000..a762220 --- /dev/null +++ b/MERGE_SPEC.md @@ -0,0 +1,304 @@ +# `pycobertura merge` — Feature Specification + +Status: **v1** (initial implementation) +Module: [`pycobertura/merge.py`](pycobertura/merge.py) +CLI command: [`pycobertura/cli.py`](pycobertura/cli.py) `merge` +Tests: [`tests/test_merge.py`](tests/test_merge.py), `merge` cases in [`tests/test_cli.py`](tests/test_cli.py) +Fixtures: `tests/merge-*.xml` + +## 1. Purpose + +Combine multiple Cobertura XML reports into a single merged report. Designed +for the case where coverage is gathered from: + +- Multiple languages in the same project (e.g. Rust + Python), producing + disjoint file sets. +- The same code run with different feature flags activated, producing + partially overlapping file sets. +- The same code run on multiple operating systems, producing the same + logical files under different absolute paths and path-separator + conventions. + +## 2. CLI surface + +``` +pycobertura merge FILE1 [FILE...] [-o OUTPUT] [--ignore-regex REGEX] [--source-path PATTERN]... +``` + +| Argument / option | Required | Repeatable | Description | +|--------------------------------|----------|------------|----------------------------------------------------------------------------------------------------------| +| `FILE...` (positional) | yes (≥1) | n/a | One or more Cobertura XML file paths. A single file is accepted (round-trips through the merge pipeline). | +| `-o`, `--output PATH` | no | no | Write merged XML to PATH. Defaults to stdout (matches `show`/`diff`). | +| `--ignore-regex REGEX` | no | no | Drop `` elements whose `filename` matches REGEX from each input *before* merging. | +| `--source-path PATTERN` | no | yes | Path pattern identifying the start of the canonical source path. See §4. | + +Output is always Cobertura XML. There is no `--format` flag — alternative +formats belong on `pycobertura show` of the merged file. + +Exit codes: +- `0` — merge succeeded. +- `1` (`ExitCodes.EXCEPTION`) — merge failed (input parse error, + malformed `--source-path`). + +## 3. Merge semantics + +### 3.1 Grouping + +Classes are grouped across all inputs by the tuple +`(package_name, class_name, canonical_filename)`. Filename alone is +insufficient because Java-style multi-class-per-file is real (e.g. +`Main.java` containing both `Main` and `Main$Helper` in +[tests/cobertura.xml](tests/cobertura.xml)). + +Methods within a class are grouped by `(name, signature)`. + +Lines within a method (or directly within a class for tools that don't emit +methods, like coverage.py) are grouped by line `number`. + +### 3.2 `` merge + +Union of `` text values across inputs, preserving first-seen order, +deduplicated. + +### 3.3 Line merge (per group) + +For each group of `` elements with the same number across inputs: + +| Attribute | Merge rule | +|----------------------|--------------------------------------------------------------------------------------------------| +| `number` | Same in all inputs (group key). | +| `hits` | Sum across inputs. | +| `branch` | `"true"` if any input marks it true, else `"false"`. | +| `condition-coverage` | If `branch="true"`: see §3.4. Else: absent. | +| `` child | Deep-copied from the input chosen by the §3.4 rule. | + +A line is considered "hit" (for line-rate computation) if its +`condition-coverage` starts with `"100%"`, or — when no `condition-coverage` +attribute is present — its `hits > 0`. This matches `pycobertura.utils.get_line_status`. + +### 3.4 Branch coverage merge + +Cobertura encodes per-line branch coverage as +`condition-coverage="P% (a/b)"` where `a` is the number of branch +outcomes covered and `b` is the total number of branch outcomes. + +For each branch line that appears in 2+ inputs: + +1. Parse `(a, b)` from each input's `condition-coverage`. +2. Restrict the candidates to those with the **largest denominator** `b`. The rationale is that a larger denominator implies more branch outcomes were detected, which we treat as a proxy for the most complete view of the source at that line (e.g. with all conditional-compilation branches active). +3. Among those candidates, choose the input with the highest numerator `a` and copy its `condition-coverage` (re-emitted as `P% (a/b)` with recomputed P) and `` block (deep-copied). + +For comparison: gcovr's strict mode aborts on denominator mismatch; lcov +silently merges (with documented data-loss bugs); coverage.py rejects +type-level mismatches; JaCoCo/cobertura-merge tools tend to silently merge +or drop information. Pycobertura prefers the largest-denominator entry +because it carries the most information; falling back to it never loses +coverage that the smaller-denominator entry uniquely captured (the +larger-denominator entry's numerator is already the best evidence we have +for that superset of branches). + +### 3.5 Aggregates + +`line-rate`, `branch-rate` are recomputed from totals at every level +(class, package, root). `complexity` is recomputed as the maximum across +inputs (no canonical aggregation rule exists). + +The root `` element also carries `lines-valid`, `lines-covered`, +`branches-valid`, `branches-covered` attributes derived from the merged +totals (consistent with the `coverage-04.dtd` convention used by coverage.py). + +### 3.6 Root metadata + +| Attribute | Value | +|--------------|----------------------------------------------------------------------------------------------------------| +| `version` | `f"pycobertura-{importlib.metadata.version('pycobertura')}"` (e.g. `pycobertura-4.1.0`). The `version` attribute identifies the producer; copying it from a first input would mislabel the merged output. | +| `timestamp` | `int(time.time() * 1000)` (Cobertura convention is ms since epoch). | + +## 4. Path remapping (`--source-path`) + +### 4.1 Motivation + +The same source file reported on Linux as +`/home/runner/work/repo/src/foo.py` and on Windows as +`C:\Users\runner\work\repo\src\foo.py` must merge to a single canonical +entry. v1 supports this through repeatable `--source-path` patterns. + +### 4.2 Pattern syntax + +A `--source-path` value: + +- **Must end with `/`** — the pattern identifies a directory. +- **May start with `**/`** — accepted for clarity; stripped before + matching. (All patterns are implicitly "match anywhere"; the leading + `**/` is not load-bearing in v1.) +- **May not contain `*` or `**` mid-pattern** — only literal directory + names are accepted between separators. Malformed patterns raise + `ValueError` at parse time, surfaced as `click.BadParameter` to the user. + +After parsing, a pattern reduces to a list of literal directory segments, +e.g. `**/site-packages/` → `["site-packages"]`, `src/main/` → `["src", "main"]`. + +### 4.3 Matching algorithm (`canonicalize_filename`) + +For every `` element across inputs: + +1. Replace all backslashes in `X` with forward slashes. +2. Split into directory segments at `/`. +3. For each declared `--source-path` pattern in order, scan from the + leftmost segment onward for the first position where the pattern's + segments appear contiguously at a directory boundary. +4. The first declared pattern that matches wins; the canonical filename + is the suffix starting at the matched position. +5. If no pattern matches, the filename is left as-is (with separators + normalized). + +### 4.4 Examples + +``` +pycobertura merge linux.xml windows.xml \ + --source-path 'src/' \ + --source-path '**/site-packages/' +``` + +| Input filename | Canonical filename | +|-----------------------------------------------------------|-----------------------------------| +| `/home/runner/work/repo/src/foo.py` | `src/foo.py` | +| `C:\Users\runner\work\repo\src\foo.py` | `src/foo.py` | +| `src/foo.py` | `src/foo.py` | +| `/usr/lib/python3.11/site-packages/mypkg/__init__.py` | `site-packages/mypkg/__init__.py` | +| `/foo/mysrc/bar.py` | `/foo/mysrc/bar.py` (no match — `mysrc` ≠ `src`) | +| `/some/random/path/unrelated.py` | `/some/random/path/unrelated.py` | + +## 5. Code organization + +The codebase is consistently input-immutable. The merge module follows +the same discipline: + +- **Inputs are treated as read-only.** Functions in `merge.py` never + mutate caller-provided lxml elements. Output is a freshly-constructed + `_Element` tree built via `lxml.etree.Element` / `SubElement`, with + `copy.deepcopy` for whole subtree carry-overs (e.g. `` + blocks chosen via the §3.4 rule). +- **No builder type in v1.** Direct construction via `SubElement` is + sufficient. A `CoberturaBuilder` class would be premature. +- **No `Cobertura.to_xml()` method.** The `Cobertura` class stays + read-only and aggressively memoized; merge operates on raw lxml trees. + +### 5.1 Public API of `pycobertura.merge` + +```python +def parse_source_path_pattern(pattern: str) -> list[str]: + """Validate and tokenize a --source-path value into literal segments. + + Raises ValueError on malformed patterns (mid-pattern wildcard, missing + trailing '/', empty after stripping leading '**/'). + """ + +def canonicalize_filename( + filename: str, patterns: Sequence[Sequence[str]] +) -> str: + """Normalize separators and apply source-path remapping. + + First pattern (in declaration order) that matches at the leftmost + directory boundary wins. If no pattern matches, returns the filename + with separators normalized only. + """ + +def merge_reports( + xml_roots: Sequence[lxml.etree._Element], + source_paths: Sequence[Sequence[str]] = (), +) -> lxml.etree._Element: + """Merge a sequence of Cobertura roots into a fresh root. + + Inputs are not mutated. Raises ValueError if xml_roots is empty. + """ +``` + +### 5.2 CLI integration (`pycobertura.cli`) + +The `merge` command is registered on the existing `pycobertura` Click +group. Each input is parsed via `lxml.etree.parse(...).getroot()` +directly (no full `Cobertura` instance — wasteful and tied to the read +API). `--ignore-regex` drops matching `` elements from each +parsed tree before invoking `merge_reports`. The merged tree is +serialized via `lxml.etree.tostring(..., xml_declaration=True, +encoding="UTF-8", pretty_print=True)` and emitted via +`click.echo(..., file=output)` (same idiom as `show`). + +## 6. Test surface + +`tests/test_merge.py` covers the algorithm and the public functions: + +- `parse_source_path_pattern`: literal segments, leading `**/`, error + cases (missing trailing slash, mid-pattern wildcard, only `**/`, + empty segment). +- `canonicalize_filename`: separator normalization, segment match, + Windows path, multi-segment pattern, no-match passthrough, + first-pattern-wins, leftmost-match-wins, partial-segment-no-match. +- `merge_reports`: disjoint files, overlapping files (hit summation, + line union), branch coverage with max-numerator selection, branch + denominator mismatch resolved by largest-denominator-then-largest- + numerator, cross-OS path unification with `--source-path`, multi- + class-per-file preservation, no-branch-rate input, empty methods, + single-input round-trip, input-immutability invariant, version + attribute, source union, root-level rate aggregation, empty input + rejection. + +`tests/test_cli.py` covers the CLI surface: + +- stdout default, `-o` to file, single positional input, missing + positional input (Click usage error), `--source-path` cross-OS unify, + malformed `--source-path` (mid-pattern wildcard, missing trailing + slash) → `BadParameter`, branch denominator mismatch merges + successfully (largest denominator wins), `--ignore-regex` drops files + pre-merge. + +## 7. Deferred to future iterations + +These are intentionally out of scope for v1; revisit only when a real +need surfaces. + +- **Full glob support in `--source-path`** (`*`, `**` mid-pattern). v1 + is prefix-match-only. +- **Per-input source-path rules.** v1 applies all patterns to all inputs + uniformly. +- **Conflict-resolution modes for non-branch fields** (`--strict`, + `--merge-mode`). +- **Config-file form of source-path rules** (e.g. reading from + `pyproject.toml` or `.coveragerc`-style `[paths]` groups). +- **Stdin input.** +- **Re-serialization on the `Cobertura` class itself** (`to_xml`, + `CoberturaBuilder`). Introduce only when a second consumer of XML + serialization appears in the codebase. + +## 8. Design rationale (why these choices) + +- **Variadic positional inputs, default to stdout, optional `-o`.** + Matches `show`/`diff`. Globbing (`coverage-*.xml`) is the dominant CI + pattern, and positional args support it cleanly. +- **Largest-denominator-wins on branch denominator mismatch.** Differing + denominators imply different generated branch structure at the same + source line (e.g. conditional compilation enabling extra branches in + one build). The largest-denominator entry sees the most branches, so + we treat it as the most complete view of that line and use its + numerator/conditions directly. This loses no information relative to + the smaller-denominator entries — they describe a strict subset of the + branch outcomes the larger entry already accounts for. Aborting (the + earlier strict rule, matching gcovr's default) made the merge command + unusable in exactly the case it was designed for: combining reports + from runs with different feature flags or platforms. +- **`--source-path` over `OLD=NEW` substitution or coverage.py-style + groups.** Reads naturally — "treat `src/` as a logical source root" — + and one flag per logical root scales cleanly. The user names the + destination directory once instead of enumerating every absolute + prefix variant. coverage.py's group form (canonical + alias list) + is more expressive but doesn't fit single-value CLI flags well. +- **Self-identifying `version`.** The Cobertura `version` attribute + identifies the producing tool. Copying it from a first input would + mislabel pycobertura's merged output and obscure debugging. +- **No `to_xml()` on `Cobertura`.** Adding write methods to a + read-only, memoized class invites cache-coherence bugs. Keep merge as + a pure tree transformation in its own module. +- **Pure functions returning fresh trees.** Matches the rest of the + codebase (`utils.py` and `Cobertura` are input-immutable). A builder + type would be premature for the v1 scope. diff --git a/pycobertura/cli.py b/pycobertura/cli.py index ce704ea..c8d5a32 100644 --- a/pycobertura/cli.py +++ b/pycobertura/cli.py @@ -2,28 +2,27 @@ import lxml.etree as ET from pycobertura.cobertura import Cobertura, CoberturaDiff +from pycobertura.filesystem import filesystem_factory from pycobertura.merge import ( - BranchConflictError, merge_reports, parse_source_path_pattern, ) from pycobertura.reporters import ( + CsvReporter, + CsvReporterDelta, GitHubAnnotationReporter, + GitHubAnnotationReporterDelta, HtmlReporter, - TextReporter, - CsvReporter, - MarkdownReporter, - JsonReporter, - YamlReporter, HtmlReporterDelta, - TextReporterDelta, - CsvReporterDelta, - MarkdownReporterDelta, + JsonReporter, JsonReporterDelta, + MarkdownReporter, + MarkdownReporterDelta, + TextReporter, + TextReporterDelta, + YamlReporter, YamlReporterDelta, - GitHubAnnotationReporterDelta, ) -from pycobertura.filesystem import filesystem_factory from pycobertura.utils import ( get_dir_from_file_path, get_filenames_that_do_not_match_regex, @@ -199,7 +198,8 @@ def show( } -@pycobertura.command(help="""\ +@pycobertura.command( + help="""\ The diff command compares and shows the changes between two Cobertura reports. NOTE: Reporting missing lines or showing the source code with the diff command @@ -210,7 +210,8 @@ def show( options `--source1` and `--source2` are necessary to point to the source code directories (or zip archives). If the source is not available at all, pass `--no-source` but missing lines and source code will not be reported. -""") +""" +) @click.argument("cobertura_file1") @click.argument("cobertura_file2") @click.option( @@ -372,26 +373,34 @@ def _source_path_callback(ctx, param, value): return parsed -@pycobertura.command(help="""\ +@pycobertura.command( + help="""\ Combine multiple Cobertura XML reports into a single merged report. Hits are summed across reports for matching lines; files are unioned across -reports. Branch coverage is merged by taking the entry with the highest -covered/total ratio. If two reports disagree on the number of branch -conditions for the same line (e.g. due to conditional compilation), the -merge aborts with an error. +reports. Branch coverage is merged by taking the entry with the largest +denominator (most branch outcomes detected, treated as the most complete +view of the line) and, among ties, the largest numerator (most branches +covered). Use --source-path to canonicalize file paths that differ across reports (e.g. between Linux and Windows runs). The pattern identifies a directory that begins the canonical path; everything before it in the input filename is stripped. A leading '**/' is allowed for clarity. +Limitations: +- Merged branch coverage is the maximum across reports, so if different branches + are coveraged in different reports, the merged report may not capture that. + This is a limitation of the Cobertura XML format, which stores only the + fraction of branched covered, not which ones. + EXAMPLES pycobertura merge linux.xml windows.xml --source-path src/ pycobertura merge a.xml b.xml c.xml -o merged.xml -""") +""" +) @click.argument("cobertura_files", nargs=-1, required=True) @click.option( "--ignore-regex", @@ -428,10 +437,7 @@ def merge(cobertura_files, ignore_regex, source_paths, output): _drop_ignored_classes(root, ignore_regex) roots.append(root) - try: - merged = merge_reports(roots, source_paths=source_paths) - except BranchConflictError as e: - raise click.ClickException(str(e)) + merged = merge_reports(roots, source_paths=source_paths) report = ET.tostring( merged, diff --git a/pycobertura/merge.py b/pycobertura/merge.py index ae6e403..2936317 100644 --- a/pycobertura/merge.py +++ b/pycobertura/merge.py @@ -24,12 +24,6 @@ _CONDITION_COVERAGE_RE = re.compile(r"^\s*(\d+)%\s*\((\d+)/(\d+)\)\s*$") -class BranchConflictError(Exception): - """Raised when two reports disagree on the number of branch conditions for - the same source line, which implies the line was generated from different - code (e.g. conditional compilation) and cannot be safely merged.""" - - def parse_source_path_pattern(pattern: str) -> List[str]: """Validate and tokenize a ``--source-path`` value into literal segments. @@ -99,9 +93,7 @@ def merge_reports( ) -> ET._Element: """Merge a sequence of Cobertura ```` roots into one fresh root. - Inputs are treated as read-only. Raises :class:`BranchConflictError` when - two inputs report different branch-condition denominators for the same - line. + Inputs are treated as read-only. """ if not xml_roots: raise ValueError("merge_reports requires at least one input root") @@ -222,7 +214,7 @@ def _merge_class( m_elem.set("signature", m_sig) m_lines_elem = ET.SubElement(m_elem, "lines") m_lines_count, m_hits, m_branches, m_branch_hits = _emit_merged_lines( - lines_by_number, m_lines_elem, canonical_filename + lines_by_number, m_lines_elem ) m_elem.set("line-rate", _rate_str(m_hits, m_lines_count)) m_elem.set("branch-rate", _rate_str(m_branch_hits, m_branches)) @@ -233,7 +225,7 @@ def _merge_class( cls_lines_elem = ET.SubElement(cls_elem, "lines") if class_lines: c_total, c_hits, c_branches, c_branch_hits = _emit_merged_lines( - class_lines, cls_lines_elem, canonical_filename + class_lines, cls_lines_elem ) else: # Build from union of merged method lines. @@ -242,7 +234,7 @@ def _merge_class( for n, line_elems in lines_by_number.items(): union.setdefault(n, []).extend(line_elems) c_total, c_hits, c_branches, c_branch_hits = _emit_merged_lines( - union, cls_lines_elem, canonical_filename + union, cls_lines_elem ) cls_elem.set("line-rate", _rate_str(c_hits, c_total)) @@ -267,7 +259,6 @@ def _merge_class( def _emit_merged_lines( lines_by_number: "OrderedDict[int, List[ET._Element]]", parent: ET._Element, - filename: str, ) -> Tuple[int, int, int, int]: """Append merged children to ``parent``. Returns (total, hits, branches, branch_hits).""" total = 0 @@ -277,7 +268,7 @@ def _emit_merged_lines( for n in sorted(lines_by_number): line_elems = lines_by_number[n] - merged = _merge_line_elements(n, line_elems, filename) + merged = _merge_line_elements(n, line_elems) parent.append(merged) total += 1 if _line_is_hit(merged): @@ -294,7 +285,7 @@ def _emit_merged_lines( def _merge_line_elements( - line_no: int, line_elems: Sequence[ET._Element], filename: str + line_no: int, line_elems: Sequence[ET._Element] ) -> ET._Element: new_line = ET.Element("line") new_line.set("number", str(line_no)) @@ -310,7 +301,6 @@ def _merge_line_elements( return new_line parsed_entries = [] # list of (a, b, source_element) - denom = None for el in branch_elems: cc = el.get("condition-coverage") if not cc: @@ -319,19 +309,14 @@ def _merge_line_elements( if parsed is None: continue a, b = parsed - if denom is None: - denom = b - elif b != denom: - raise BranchConflictError( - f"Conflicting branch counts at {filename}:{line_no}: " - f"{denom} vs {b}" - ) parsed_entries.append((a, b, el)) if not parsed_entries: return new_line - best_a, best_b, best_el = max(parsed_entries, key=lambda x: x[0]) + # Largest denominator wins (most complete view of branch outcomes); break + # ties by largest numerator (most branches actually covered). + best_a, best_b, best_el = max(parsed_entries, key=lambda x: (x[1], x[0])) pct = int(round(100 * best_a / best_b)) if best_b > 0 else 0 new_line.set("condition-coverage", f"{pct}% ({best_a}/{best_b})") diff --git a/tests/test_cli.py b/tests/test_cli.py index 1c1d4d7..cf60714 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1360,20 +1360,24 @@ def test_merge__source_path_rejects_missing_trailing_slash(): assert "must end with '/'" in result.output -def test_merge__branch_conflict_exits_with_error(): +def test_merge__branch_denominator_mismatch_succeeds(): from pycobertura.cli import merge runner = CliRunner() result = runner.invoke( merge, [ - 'tests/merge-branch-conflict-a.xml', - 'tests/merge-branch-conflict-b.xml', + 'tests/merge-branch-conflict-a.xml', # 1/2 + 'tests/merge-branch-conflict-b.xml', # 1/3 ], catch_exceptions=False, ) - assert result.exit_code != 0 - assert "app/main.py:10" in result.output + assert result.exit_code == 0 + import lxml.etree as ET + root = ET.fromstring(result.output.encode("utf-8") if isinstance(result.output, str) else result.output) + line = root.xpath(".//class[@filename='app/main.py']/lines/line[@number='10']")[0] + # Largest denominator wins: 1/3 from the second input. + assert line.get("condition-coverage") == "33% (1/3)" def test_merge__ignore_regex_drops_files_pre_merge(): diff --git a/tests/test_merge.py b/tests/test_merge.py index 9b9b8d9..81e80fe 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -4,7 +4,6 @@ import pytest from pycobertura.merge import ( - BranchConflictError, canonicalize_filename, merge_reports, parse_source_path_pattern, @@ -155,11 +154,44 @@ def test_merge_branch_lines_takes_max_coverage(): assert line.get("hits") == "8" -def test_merge_branch_denominator_conflict_raises(): - a = _parse("tests/merge-branch-conflict-a.xml") # 1/2 - b = _parse("tests/merge-branch-conflict-b.xml") # 1/3 - with pytest.raises(BranchConflictError, match="app/main.py:10"): - merge_reports([a, b]) +def test_merge_branch_denominator_mismatch_picks_largest_denominator(): + # a has 1/2 with 3 hits; b has 1/3 with 2 hits. Differing denominators no + # longer raise — largest denominator wins (treated as the most complete + # view of the source at that line), then largest numerator breaks ties. + a = _parse("tests/merge-branch-conflict-a.xml") + b = _parse("tests/merge-branch-conflict-b.xml") + merged = merge_reports([a, b]) + line = _line(merged, "app/main.py", 10) + assert line.get("branch") == "true" + assert line.get("condition-coverage") == "33% (1/3)" + # hits are still summed across inputs + assert line.get("hits") == "5" + + +def test_merge_branch_denominator_mismatch_breaks_ties_by_numerator(): + # Build two roots in-memory that share a denominator-mismatching line where + # the smaller-denominator entry has the bigger numerator. Largest + # denominator must still win. + def _root(num, denom, hits): + xml = f""" + + . + + + + + + + + +""" + return ET.fromstring(xml.encode("utf-8")) + + a = _root(2, 2, 1) # fully covered, but only 2 branches detected + b = _root(1, 4, 1) # 4 branches detected, only 1 covered + merged = merge_reports([a, b]) + line = _line(merged, "app/m.py", 1) + assert line.get("condition-coverage") == "25% (1/4)" def test_merge_with_source_paths_unifies_cross_os_paths(): From 847f4de8dfb3fbf68572cc6f7829d3f5c121392a Mon Sep 17 00:00:00 2001 From: David Hagen Date: Wed, 6 May 2026 16:17:40 -0400 Subject: [PATCH 03/13] Rename source-path to source --- MERGE_SPEC.md => merge-spec.md | 47 +++++++++++++++++----------------- pycobertura/cli.py | 20 +++++++-------- pycobertura/merge.py | 22 ++++++++-------- tests/test_cli.py | 12 ++++----- tests/test_merge.py | 42 +++++++++++++++--------------- 5 files changed, 71 insertions(+), 72 deletions(-) rename MERGE_SPEC.md => merge-spec.md (88%) diff --git a/MERGE_SPEC.md b/merge-spec.md similarity index 88% rename from MERGE_SPEC.md rename to merge-spec.md index a762220..c7b7e18 100644 --- a/MERGE_SPEC.md +++ b/merge-spec.md @@ -22,7 +22,7 @@ for the case where coverage is gathered from: ## 2. CLI surface ``` -pycobertura merge FILE1 [FILE...] [-o OUTPUT] [--ignore-regex REGEX] [--source-path PATTERN]... +pycobertura merge FILE1 [FILE...] [-o OUTPUT] [--ignore-regex REGEX] [--source PATTERN]... ``` | Argument / option | Required | Repeatable | Description | @@ -30,15 +30,14 @@ pycobertura merge FILE1 [FILE...] [-o OUTPUT] [--ignore-regex REGEX] [--source-p | `FILE...` (positional) | yes (≥1) | n/a | One or more Cobertura XML file paths. A single file is accepted (round-trips through the merge pipeline). | | `-o`, `--output PATH` | no | no | Write merged XML to PATH. Defaults to stdout (matches `show`/`diff`). | | `--ignore-regex REGEX` | no | no | Drop `` elements whose `filename` matches REGEX from each input *before* merging. | -| `--source-path PATTERN` | no | yes | Path pattern identifying the start of the canonical source path. See §4. | +| `--source PATTERN` | no | yes | Path pattern identifying the start of a source path. See §4. | -Output is always Cobertura XML. There is no `--format` flag — alternative -formats belong on `pycobertura show` of the merged file. +Output is always Cobertura XML. Exit codes: - `0` — merge succeeded. - `1` (`ExitCodes.EXCEPTION`) — merge failed (input parse error, - malformed `--source-path`). + malformed `--source`). ## 3. Merge semantics @@ -85,7 +84,7 @@ outcomes covered and `b` is the total number of branch outcomes. For each branch line that appears in 2+ inputs: 1. Parse `(a, b)` from each input's `condition-coverage`. -2. Restrict the candidates to those with the **largest denominator** `b`. The rationale is that a larger denominator implies more branch outcomes were detected, which we treat as a proxy for the most complete view of the source at that line (e.g. with all conditional-compilation branches active). +2. Restrict the candidates to those with the largest denominator `b`. The rationale is that a larger denominator implies more branch outcomes were detected, which we treat as a proxy for the most complete view of the source at that line. 3. Among those candidates, choose the input with the highest numerator `a` and copy its `condition-coverage` (re-emitted as `P% (a/b)` with recomputed P) and `` block (deep-copied). For comparison: gcovr's strict mode aborts on denominator mismatch; lcov @@ -114,18 +113,18 @@ totals (consistent with the `coverage-04.dtd` convention used by coverage.py). | `version` | `f"pycobertura-{importlib.metadata.version('pycobertura')}"` (e.g. `pycobertura-4.1.0`). The `version` attribute identifies the producer; copying it from a first input would mislabel the merged output. | | `timestamp` | `int(time.time() * 1000)` (Cobertura convention is ms since epoch). | -## 4. Path remapping (`--source-path`) +## 4. Path remapping (`--source`) ### 4.1 Motivation The same source file reported on Linux as `/home/runner/work/repo/src/foo.py` and on Windows as `C:\Users\runner\work\repo\src\foo.py` must merge to a single canonical -entry. v1 supports this through repeatable `--source-path` patterns. +entry. v1 supports this through repeatable `--source` patterns. ### 4.2 Pattern syntax -A `--source-path` value: +A `--source` value: - **Must end with `/`** — the pattern identifies a directory. - **May start with `**/`** — accepted for clarity; stripped before @@ -144,7 +143,7 @@ For every `` element across inputs: 1. Replace all backslashes in `X` with forward slashes. 2. Split into directory segments at `/`. -3. For each declared `--source-path` pattern in order, scan from the +3. For each declared `--source` pattern in order, scan from the leftmost segment onward for the first position where the pattern's segments appear contiguously at a directory boundary. 4. The first declared pattern that matches wins; the canonical filename @@ -156,8 +155,8 @@ For every `` element across inputs: ``` pycobertura merge linux.xml windows.xml \ - --source-path 'src/' \ - --source-path '**/site-packages/' + --source 'src/' \ + --source '**/site-packages/' ``` | Input filename | Canonical filename | @@ -187,8 +186,8 @@ the same discipline: ### 5.1 Public API of `pycobertura.merge` ```python -def parse_source_path_pattern(pattern: str) -> list[str]: - """Validate and tokenize a --source-path value into literal segments. +def parse_source_pattern(pattern: str) -> list[str]: + """Validate and tokenize a --source value into literal segments. Raises ValueError on malformed patterns (mid-pattern wildcard, missing trailing '/', empty after stripping leading '**/'). @@ -197,7 +196,7 @@ def parse_source_path_pattern(pattern: str) -> list[str]: def canonicalize_filename( filename: str, patterns: Sequence[Sequence[str]] ) -> str: - """Normalize separators and apply source-path remapping. + """Normalize separators and apply source remapping. First pattern (in declaration order) that matches at the leftmost directory boundary wins. If no pattern matches, returns the filename @@ -206,7 +205,7 @@ def canonicalize_filename( def merge_reports( xml_roots: Sequence[lxml.etree._Element], - source_paths: Sequence[Sequence[str]] = (), + sources: Sequence[Sequence[str]] = (), ) -> lxml.etree._Element: """Merge a sequence of Cobertura roots into a fresh root. @@ -229,7 +228,7 @@ encoding="UTF-8", pretty_print=True)` and emitted via `tests/test_merge.py` covers the algorithm and the public functions: -- `parse_source_path_pattern`: literal segments, leading `**/`, error +- `parse_source_pattern`: literal segments, leading `**/`, error cases (missing trailing slash, mid-pattern wildcard, only `**/`, empty segment). - `canonicalize_filename`: separator normalization, segment match, @@ -238,7 +237,7 @@ encoding="UTF-8", pretty_print=True)` and emitted via - `merge_reports`: disjoint files, overlapping files (hit summation, line union), branch coverage with max-numerator selection, branch denominator mismatch resolved by largest-denominator-then-largest- - numerator, cross-OS path unification with `--source-path`, multi- + numerator, cross-OS path unification with `--source`, multi- class-per-file preservation, no-branch-rate input, empty methods, single-input round-trip, input-immutability invariant, version attribute, source union, root-level rate aggregation, empty input @@ -247,8 +246,8 @@ encoding="UTF-8", pretty_print=True)` and emitted via `tests/test_cli.py` covers the CLI surface: - stdout default, `-o` to file, single positional input, missing - positional input (Click usage error), `--source-path` cross-OS unify, - malformed `--source-path` (mid-pattern wildcard, missing trailing + positional input (Click usage error), `--source` cross-OS unify, + malformed `--source` (mid-pattern wildcard, missing trailing slash) → `BadParameter`, branch denominator mismatch merges successfully (largest denominator wins), `--ignore-regex` drops files pre-merge. @@ -258,13 +257,13 @@ encoding="UTF-8", pretty_print=True)` and emitted via These are intentionally out of scope for v1; revisit only when a real need surfaces. -- **Full glob support in `--source-path`** (`*`, `**` mid-pattern). v1 +- **Full glob support in `--source`** (`*`, `**` mid-pattern). v1 is prefix-match-only. -- **Per-input source-path rules.** v1 applies all patterns to all inputs +- **Per-input source rules.** v1 applies all patterns to all inputs uniformly. - **Conflict-resolution modes for non-branch fields** (`--strict`, `--merge-mode`). -- **Config-file form of source-path rules** (e.g. reading from +- **Config-file form of source rules** (e.g. reading from `pyproject.toml` or `.coveragerc`-style `[paths]` groups). - **Stdin input.** - **Re-serialization on the `Cobertura` class itself** (`to_xml`, @@ -287,7 +286,7 @@ need surfaces. earlier strict rule, matching gcovr's default) made the merge command unusable in exactly the case it was designed for: combining reports from runs with different feature flags or platforms. -- **`--source-path` over `OLD=NEW` substitution or coverage.py-style +- **`--source` over `OLD=NEW` substitution or coverage.py-style groups.** Reads naturally — "treat `src/` as a logical source root" — and one flag per logical root scales cleanly. The user names the destination directory once instead of enumerating every absolute diff --git a/pycobertura/cli.py b/pycobertura/cli.py index c8d5a32..8ce2519 100644 --- a/pycobertura/cli.py +++ b/pycobertura/cli.py @@ -5,7 +5,7 @@ from pycobertura.filesystem import filesystem_factory from pycobertura.merge import ( merge_reports, - parse_source_path_pattern, + parse_source_pattern, ) from pycobertura.reporters import ( CsvReporter, @@ -363,11 +363,11 @@ def diff( raise SystemExit(exit_code) -def _source_path_callback(ctx, param, value): +def _source_callback(ctx, param, value): parsed = [] for v in value: try: - parsed.append(parse_source_path_pattern(v)) + parsed.append(parse_source_pattern(v)) except ValueError as e: raise click.BadParameter(str(e), ctx=ctx, param=param) return parsed @@ -383,7 +383,7 @@ def _source_path_callback(ctx, param, value): view of the line) and, among ties, the largest numerator (most branches covered). -Use --source-path to canonicalize file paths that differ across reports +Use --source to canonicalize file paths that differ across reports (e.g. between Linux and Windows runs). The pattern identifies a directory that begins the canonical path; everything before it in the input filename is stripped. A leading '**/' is allowed for clarity. @@ -396,7 +396,7 @@ def _source_path_callback(ctx, param, value): EXAMPLES - pycobertura merge linux.xml windows.xml --source-path src/ + pycobertura merge linux.xml windows.xml --source src/ pycobertura merge a.xml b.xml c.xml -o merged.xml """ @@ -409,11 +409,11 @@ def _source_path_callback(ctx, param, value): help="Regex for which files to ignore from each input before merging.", ) @click.option( - "--source-path", - "source_paths", + "--source", + "sources", multiple=True, metavar="", - callback=_source_path_callback, + callback=_source_callback, help="Path pattern (e.g. 'src/' or '**/site-packages/') identifying the " "start of the canonical source path. Repeatable. The first matching " "pattern wins per filename.", @@ -425,7 +425,7 @@ def _source_path_callback(ctx, param, value): type=click.File("wb"), help="Write merged XML to instead of stdout.", ) -def merge(cobertura_files, ignore_regex, source_paths, output): +def merge(cobertura_files, ignore_regex, sources, output): """combine multiple Cobertura reports into one""" roots = [] for path in cobertura_files: @@ -437,7 +437,7 @@ def merge(cobertura_files, ignore_regex, source_paths, output): _drop_ignored_classes(root, ignore_regex) roots.append(root) - merged = merge_reports(roots, source_paths=source_paths) + merged = merge_reports(roots, sources=sources) report = ET.tostring( merged, diff --git a/pycobertura/merge.py b/pycobertura/merge.py index 2936317..b669eee 100644 --- a/pycobertura/merge.py +++ b/pycobertura/merge.py @@ -24,15 +24,15 @@ _CONDITION_COVERAGE_RE = re.compile(r"^\s*(\d+)%\s*\((\d+)/(\d+)\)\s*$") -def parse_source_path_pattern(pattern: str) -> List[str]: - """Validate and tokenize a ``--source-path`` value into literal segments. +def parse_source_pattern(pattern: str) -> List[str]: + """Validate and tokenize a ``--source`` value into literal segments. A leading ``**/`` is allowed (and stripped); no other wildcards are permitted in v1. Pattern must end with ``/``. """ if not pattern.endswith("/"): raise ValueError( - f"--source-path pattern must end with '/': {pattern!r}" + f"--source pattern must end with '/': {pattern!r}" ) body = pattern[:-1] @@ -40,21 +40,21 @@ def parse_source_path_pattern(pattern: str) -> List[str]: body = body[len("**/"):] elif body == "**": raise ValueError( - f"--source-path pattern is empty after stripping leading '**/': {pattern!r}" + f"--source pattern is empty after stripping leading '**/': {pattern!r}" ) if not body: - raise ValueError(f"--source-path pattern has no literal segments: {pattern!r}") + raise ValueError(f"--source pattern has no literal segments: {pattern!r}") segments = body.split("/") for seg in segments: if not seg: raise ValueError( - f"--source-path pattern has empty segments: {pattern!r}" + f"--source pattern has empty segments: {pattern!r}" ) if "*" in seg: raise ValueError( - f"--source-path pattern uses unsupported wildcard in {seg!r} " + f"--source pattern uses unsupported wildcard in {seg!r} " f"(only a leading '**/' is supported in v1): {pattern!r}" ) @@ -64,10 +64,10 @@ def parse_source_path_pattern(pattern: str) -> List[str]: def canonicalize_filename( filename: str, patterns: Sequence[Sequence[str]] ) -> str: - """Normalize separators and apply source-path remapping to ``filename``. + """Normalize separators and apply source remapping to ``filename``. ``patterns`` is the list of segment-lists produced by - :func:`parse_source_path_pattern`. The first pattern (in declaration order) + :func:`parse_source_pattern`. The first pattern (in declaration order) that matches at the leftmost directory boundary wins; the canonical name is the suffix starting at the match. If no pattern matches, the filename is returned with backslashes normalized to forward slashes. @@ -89,7 +89,7 @@ def canonicalize_filename( def merge_reports( xml_roots: Sequence[ET._Element], - source_paths: Sequence[Sequence[str]] = (), + sources: Sequence[Sequence[str]] = (), ) -> ET._Element: """Merge a sequence of Cobertura ```` roots into one fresh root. @@ -114,7 +114,7 @@ def merge_reports( for cls in pkg.iterfind("./classes/class"): class_name = cls.get("name", "") or "" filename = cls.get("filename", "") or "" - canonical = canonicalize_filename(filename, source_paths) + canonical = canonicalize_filename(filename, sources) key = (class_name, canonical) pkg_classes.setdefault(key, []).append(cls) diff --git a/tests/test_cli.py b/tests/test_cli.py index cf60714..2c6806b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1314,7 +1314,7 @@ def test_merge__requires_at_least_one_input(): assert result.exit_code != 0 -def test_merge__source_path_unifies_cross_os_paths(): +def test_merge__source_unifies_cross_os_paths(): from pycobertura.cli import merge runner = CliRunner() @@ -1323,7 +1323,7 @@ def test_merge__source_path_unifies_cross_os_paths(): [ 'tests/merge-linux.xml', 'tests/merge-windows.xml', - '--source-path', 'src/', + '--source', 'src/', ], catch_exceptions=False, ) @@ -1334,26 +1334,26 @@ def test_merge__source_path_unifies_cross_os_paths(): assert files == ["src/main.py"] -def test_merge__source_path_rejects_mid_pattern_wildcard(): +def test_merge__source_rejects_mid_pattern_wildcard(): from pycobertura.cli import merge runner = CliRunner() result = runner.invoke( merge, - ['tests/merge-rust.xml', '--source-path', 'src/*/foo/'], + ['tests/merge-rust.xml', '--source', 'src/*/foo/'], catch_exceptions=False, ) assert result.exit_code != 0 assert "unsupported wildcard" in result.output -def test_merge__source_path_rejects_missing_trailing_slash(): +def test_merge__source_rejects_missing_trailing_slash(): from pycobertura.cli import merge runner = CliRunner() result = runner.invoke( merge, - ['tests/merge-rust.xml', '--source-path', 'src'], + ['tests/merge-rust.xml', '--source', 'src'], catch_exceptions=False, ) assert result.exit_code != 0 diff --git a/tests/test_merge.py b/tests/test_merge.py index 81e80fe..6df8f9a 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -6,46 +6,46 @@ from pycobertura.merge import ( canonicalize_filename, merge_reports, - parse_source_path_pattern, + parse_source_pattern, ) -# ---- parse_source_path_pattern ---- +# ---- parse_source_pattern ---- -def test_parse_source_path_pattern_literal(): - assert parse_source_path_pattern("src/") == ["src"] +def test_parse_source_pattern_literal(): + assert parse_source_pattern("src/") == ["src"] -def test_parse_source_path_pattern_multi_segment(): - assert parse_source_path_pattern("a/b/c/") == ["a", "b", "c"] +def test_parse_source_pattern_multi_segment(): + assert parse_source_pattern("a/b/c/") == ["a", "b", "c"] -def test_parse_source_path_pattern_strips_leading_double_star(): - assert parse_source_path_pattern("**/site-packages/") == ["site-packages"] - assert parse_source_path_pattern("**/a/b/") == ["a", "b"] +def test_parse_source_pattern_strips_leading_double_star(): + assert parse_source_pattern("**/site-packages/") == ["site-packages"] + assert parse_source_pattern("**/a/b/") == ["a", "b"] -def test_parse_source_path_pattern_rejects_missing_trailing_slash(): +def test_parse_source_pattern_rejects_missing_trailing_slash(): with pytest.raises(ValueError, match="must end with '/'"): - parse_source_path_pattern("src") + parse_source_pattern("src") -def test_parse_source_path_pattern_rejects_mid_pattern_wildcard(): +def test_parse_source_pattern_rejects_mid_pattern_wildcard(): with pytest.raises(ValueError, match="unsupported wildcard"): - parse_source_path_pattern("src/*/foo/") + parse_source_pattern("src/*/foo/") with pytest.raises(ValueError, match="unsupported wildcard"): - parse_source_path_pattern("src/**/foo/") + parse_source_pattern("src/**/foo/") -def test_parse_source_path_pattern_rejects_only_double_star(): +def test_parse_source_pattern_rejects_only_double_star(): with pytest.raises(ValueError, match="empty after stripping"): - parse_source_path_pattern("**/") + parse_source_pattern("**/") -def test_parse_source_path_pattern_rejects_empty_segment(): +def test_parse_source_pattern_rejects_empty_segment(): with pytest.raises(ValueError, match="empty segments"): - parse_source_path_pattern("a//b/") + parse_source_pattern("a//b/") # ---- canonicalize_filename ---- @@ -194,11 +194,11 @@ def _root(num, denom, hits): assert line.get("condition-coverage") == "25% (1/4)" -def test_merge_with_source_paths_unifies_cross_os_paths(): +def test_merge_with_sources_unifies_cross_os_paths(): linux = _parse("tests/merge-linux.xml") windows = _parse("tests/merge-windows.xml") - patterns = [parse_source_path_pattern("src/")] - merged = merge_reports([linux, windows], source_paths=patterns) + patterns = [parse_source_pattern("src/")] + merged = merge_reports([linux, windows], sources=patterns) # Both inputs' main.py merge under canonical path files = _files(merged) From 8463a7e42d1701990e636163a2fc671ee9937657 Mon Sep 17 00:00:00 2001 From: David Hagen Date: Thu, 7 May 2026 16:11:34 -0400 Subject: [PATCH 04/13] Simplify version string --- merge-spec.md | 22 ++++++++++++++------- pycobertura/merge.py | 14 ++++++++++---- tests/test_merge.py | 46 +++++++++++++++++++++++++++++++++++--------- 3 files changed, 62 insertions(+), 20 deletions(-) diff --git a/merge-spec.md b/merge-spec.md index c7b7e18..c396c13 100644 --- a/merge-spec.md +++ b/merge-spec.md @@ -108,10 +108,14 @@ totals (consistent with the `coverage-04.dtd` convention used by coverage.py). ### 3.6 Root metadata -| Attribute | Value | -|--------------|----------------------------------------------------------------------------------------------------------| -| `version` | `f"pycobertura-{importlib.metadata.version('pycobertura')}"` (e.g. `pycobertura-4.1.0`). The `version` attribute identifies the producer; copying it from a first input would mislabel the merged output. | -| `timestamp` | `int(time.time() * 1000)` (Cobertura convention is ms since epoch). | +Follows coverage.py's convention: bare version on the attribute, producer +identified via an XML comment under the root. + +| Attribute / node | Value | +|-----------------------|----------------------------------------------------------------------------------------------------| +| `version` | `importlib.metadata.version('pycobertura')` (e.g. `"4.1.0"`). Bare version, no tool-name prefix — matches every other Cobertura emitter (coverage.py, the Java Cobertura tool, istanbul, etc.). Downstream parsers that `float(version)` keep working. | +| `timestamp` | `int(time.time() * 1000)` (Cobertura convention is ms since epoch). | +| `` | First child of ``: `Generated by pycobertura merge: https://github.com/aconrad/pycobertura`. Identifies the producer without polluting the `version` attribute. | ## 4. Path remapping (`--source`) @@ -292,9 +296,13 @@ need surfaces. destination directory once instead of enumerating every absolute prefix variant. coverage.py's group form (canonical + alias list) is more expressive but doesn't fit single-value CLI flags well. -- **Self-identifying `version`.** The Cobertura `version` attribute - identifies the producing tool. Copying it from a first input would - mislabel pycobertura's merged output and obscure debugging. +- **Bare `version` + generator comment (coverage.py style).** Every + widely-deployed Cobertura emitter writes a bare version number in the + `version` attribute and identifies itself through an XML comment. + Copying the attribute from a first input would mislabel pycobertura's + output, but a tool-name-prefixed `version` would deviate from the + ecosystem and risk breaking downstream parsers that try to coerce the + attribute to a number. - **No `to_xml()` on `Cobertura`.** Adding write methods to a read-only, memoized class invites cache-coherence bugs. Keep merge as a pure tree transformation in its own module. diff --git a/pycobertura/merge.py b/pycobertura/merge.py index b669eee..4b78bd5 100644 --- a/pycobertura/merge.py +++ b/pycobertura/merge.py @@ -120,7 +120,13 @@ def merge_reports( new_root = ET.Element("coverage") new_root.set("timestamp", str(int(time.time() * 1000))) - new_root.set("version", _producer_string()) + new_root.set("version", _pycobertura_version()) + new_root.append( + ET.Comment( + " Generated by pycobertura merge: " + "https://github.com/aconrad/pycobertura " + ) + ) sources_elem = ET.SubElement(new_root, "sources") for src_text in sources_seen: @@ -356,8 +362,8 @@ def _format_float(value: float) -> str: return s -def _producer_string() -> str: +def _pycobertura_version() -> str: try: - return f"pycobertura-{_pkg_version('pycobertura')}" + return _pkg_version("pycobertura") except Exception: # pragma: no cover - return "pycobertura" + return "" diff --git a/tests/test_merge.py b/tests/test_merge.py index 6df8f9a..ce22ed3 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -9,7 +9,6 @@ parse_source_pattern, ) - # ---- parse_source_pattern ---- @@ -57,12 +56,18 @@ def test_canonicalize_filename_no_patterns_normalizes_separators(): def test_canonicalize_filename_matches_segment(): patterns = [["src"]] - assert canonicalize_filename("/home/runner/work/repo/src/foo.py", patterns) == "src/foo.py" + assert ( + canonicalize_filename("/home/runner/work/repo/src/foo.py", patterns) + == "src/foo.py" + ) def test_canonicalize_filename_matches_windows_path(): patterns = [["src"]] - assert canonicalize_filename(r"C:\Users\runner\work\repo\src\foo.py", patterns) == "src/foo.py" + assert ( + canonicalize_filename(r"C:\Users\runner\work\repo\src\foo.py", patterns) + == "src/foo.py" + ) def test_canonicalize_filename_matches_at_start(): @@ -72,7 +77,10 @@ def test_canonicalize_filename_matches_at_start(): def test_canonicalize_filename_matches_multi_segment_pattern(): patterns = [["src", "main"]] - assert canonicalize_filename("/foo/bar/src/main/python/app.py", patterns) == "src/main/python/app.py" + assert ( + canonicalize_filename("/foo/bar/src/main/python/app.py", patterns) + == "src/main/python/app.py" + ) def test_canonicalize_filename_no_match_normalizes_only(): @@ -83,7 +91,9 @@ def test_canonicalize_filename_no_match_normalizes_only(): def test_canonicalize_filename_first_pattern_wins(): patterns = [["src"], ["site-packages"]] # Both could potentially match if path had both; first declared wins. - assert canonicalize_filename("/x/site-packages/src/foo.py", patterns) == "src/foo.py" + assert ( + canonicalize_filename("/x/site-packages/src/foo.py", patterns) == "src/foo.py" + ) def test_canonicalize_filename_leftmost_match_wins(): @@ -124,7 +134,9 @@ def test_merge_disjoint_files(): b = _parse("tests/merge-python.xml") merged = merge_reports([a, b]) files = _files(merged) - assert files == sorted(["src/lib.rs", "src/parser.rs", "app/main.py", "app/util.py"]) + assert files == sorted( + ["src/lib.rs", "src/parser.rs", "app/main.py", "app/util.py"] + ) def test_merge_overlapping_files_sums_hits_and_unions_lines(): @@ -139,7 +151,10 @@ def test_merge_overlapping_files_sums_hits_and_unions_lines(): # Line 7: only in b, hits=0 assert _line(merged, "app/main.py", 7).get("hits") == "0" # Union of lines: {1,2,3,4,5,6,7} - line_numbers = sorted(int(l.get("number")) for l in merged.xpath(".//class[@filename='app/main.py']/lines/line")) + line_numbers = sorted( + int(l.get("number")) + for l in merged.xpath(".//class[@filename='app/main.py']/lines/line") + ) assert line_numbers == [1, 2, 3, 4, 5, 6, 7] @@ -259,10 +274,23 @@ def test_merge_does_not_mutate_inputs(): assert ET.tostring(b) == before_b -def test_merge_sets_pycobertura_version(): +def test_merge_sets_bare_pycobertura_version(): + # Follow coverage.py's convention: bare version string, no tool-name prefix. + a = _parse("tests/merge-overlap-a.xml") + merged = merge_reports([a]) + version = merged.get("version", "") + assert version + assert version[0].isdigit() + + +def test_merge_emits_generator_comment(): + # Follow coverage.py's convention: a "Generated by ..." XML comment + # under the root element. a = _parse("tests/merge-overlap-a.xml") merged = merge_reports([a]) - assert merged.get("version", "").startswith("pycobertura-") + comments = [node for node in merged if isinstance(node, ET._Comment)] + assert len(comments) == 1 + assert "Generated by pycobertura" in comments[0].text def test_merge_unions_sources(): From ce7c9999fbcb3cfedf264d083929321fc6998197 Mon Sep 17 00:00:00 2001 From: David Hagen Date: Sat, 16 May 2026 18:52:45 -0400 Subject: [PATCH 05/13] Reinvent --source to strip prefixes --- merge-spec.md | 150 +++++++++++++++++++++++++--------------- pycobertura/cli.py | 21 +++--- pycobertura/merge.py | 91 +++++++++++++++++-------- tests/test_cli.py | 15 ++-- tests/test_merge.py | 159 +++++++++++++++++++++++++++++++++---------- 5 files changed, 303 insertions(+), 133 deletions(-) diff --git a/merge-spec.md b/merge-spec.md index c396c13..b9e9c1c 100644 --- a/merge-spec.md +++ b/merge-spec.md @@ -30,7 +30,7 @@ pycobertura merge FILE1 [FILE...] [-o OUTPUT] [--ignore-regex REGEX] [--source P | `FILE...` (positional) | yes (≥1) | n/a | One or more Cobertura XML file paths. A single file is accepted (round-trips through the merge pipeline). | | `-o`, `--output PATH` | no | no | Write merged XML to PATH. Defaults to stdout (matches `show`/`diff`). | | `--ignore-regex REGEX` | no | no | Drop `` elements whose `filename` matches REGEX from each input *before* merging. | -| `--source PATTERN` | no | yes | Path pattern identifying the start of a source path. See §4. | +| `--source PATTERN` | no | yes | Filesystem prefix identifying a logical root; the matching prefix is stripped from each filename. See §4. | Output is always Cobertura XML. @@ -87,7 +87,7 @@ For each branch line that appears in 2+ inputs: 2. Restrict the candidates to those with the largest denominator `b`. The rationale is that a larger denominator implies more branch outcomes were detected, which we treat as a proxy for the most complete view of the source at that line. 3. Among those candidates, choose the input with the highest numerator `a` and copy its `condition-coverage` (re-emitted as `P% (a/b)` with recomputed P) and `` block (deep-copied). -For comparison: gcovr's strict mode aborts on denominator mismatch; lcov +For comparison: gcovr's strict mode aborts on denominator mismatch; lcovz silently merges (with documented data-loss bugs); coverage.py rejects type-level mismatches; JaCoCo/cobertura-merge tools tend to silently merge or drop information. Pycobertura prefers the largest-denominator entry @@ -113,7 +113,7 @@ identified via an XML comment under the root. | Attribute / node | Value | |-----------------------|----------------------------------------------------------------------------------------------------| -| `version` | `importlib.metadata.version('pycobertura')` (e.g. `"4.1.0"`). Bare version, no tool-name prefix — matches every other Cobertura emitter (coverage.py, the Java Cobertura tool, istanbul, etc.). Downstream parsers that `float(version)` keep working. | +| `version` | `importlib.metadata.version('pycobertura')` (e.g. `"4.1.0"`). Bare version, matches every other Cobertura emitter (coverage.py, the Java Cobertura tool, istanbul, etc.). | | `timestamp` | `int(time.time() * 1000)` (Cobertura convention is ms since epoch). | | `` | First child of ``: `Generated by pycobertura merge: https://github.com/aconrad/pycobertura`. Identifies the producer without polluting the `version` attribute. | @@ -124,53 +124,77 @@ identified via an XML comment under the root. The same source file reported on Linux as `/home/runner/work/repo/src/foo.py` and on Windows as `C:\Users\runner\work\repo\src\foo.py` must merge to a single canonical -entry. v1 supports this through repeatable `--source` patterns. +entry. A `--source` pattern names a filesystem prefix that identifies a +logical root; the matching prefix is stripped from the filename, leaving +the project-relative path as the canonical key. ### 4.2 Pattern syntax -A `--source` value: +A `--source` value is a sequence of `/`-separated segments. Leading and +trailing `/` are cosmetic (`src`, `/src`, `src/`, `/src/` all parse +identically). Backslashes in the pattern are normalized to forward +slashes before splitting. -- **Must end with `/`** — the pattern identifies a directory. -- **May start with `**/`** — accepted for clarity; stripped before - matching. (All patterns are implicitly "match anywhere"; the leading - `**/` is not load-bearing in v1.) -- **May not contain `*` or `**` mid-pattern** — only literal directory - names are accepted between separators. Malformed patterns raise - `ValueError` at parse time, surfaced as `click.BadParameter` to the user. +Each segment must be one of: -After parsing, a pattern reduces to a list of literal directory segments, -e.g. `**/site-packages/` → `["site-packages"]`, `src/main/` → `["src", "main"]`. +- **A literal directory name** (e.g. `src`, `site-packages`, `Users`). +- **`*`** — matches exactly one path segment. +- **`**`** — matches zero or more path segments (leftmost match). + +In-segment wildcards (e.g. `*name`, `lib*`, `na*me`) are not supported and +raise `ValueError` at parse time. Empty segments (`src//foo`) and a +pattern that reduces to a single `**` (which would match everything and +strip the entire path) are also rejected. + +Examples after parsing: + +| Pattern | Segments | +|-------------------------------|-----------------------------------| +| `src/` | `["src"]` | +| `/Users/jdoe/myproj/` | `["Users", "jdoe", "myproj"]` | +| `**/site-packages/` | `["**", "site-packages"]` | +| `**/build/*/src/` | `["**", "build", "*", "src"]` | ### 4.3 Matching algorithm (`canonicalize_filename`) For every `` element across inputs: 1. Replace all backslashes in `X` with forward slashes. -2. Split into directory segments at `/`. -3. For each declared `--source` pattern in order, scan from the - leftmost segment onward for the first position where the pattern's - segments appear contiguously at a directory boundary. -4. The first declared pattern that matches wins; the canonical filename - is the suffix starting at the matched position. -5. If no pattern matches, the filename is left as-is (with separators - normalized). +2. If no patterns are declared, return the normalized filename unchanged. +3. Strip a single leading `/` (for matching purposes only) and split on + `/` to obtain the filename's segments. +4. For each declared `--source` pattern in order, attempt to match + anchored at the first segment: + - A literal segment matches iff equal. + - `*` matches exactly one segment. + - `**` matches zero or more segments and chooses the **leftmost** + match — it consumes the fewest segments such that the remainder of + the pattern matches starting at the next segment. +5. The first pattern that matches wins. The canonical filename is the + remaining segments (those after the matched prefix) joined by `/`. +6. If no pattern matches, return the filename with `\` → `/` + normalization only (any original leading `/` preserved). ### 4.4 Examples ``` pycobertura merge linux.xml windows.xml \ - --source 'src/' \ + --source '/home/runner/work/repo/' \ + --source 'C:/Users/runner/work/repo/' \ --source '**/site-packages/' ``` -| Input filename | Canonical filename | -|-----------------------------------------------------------|-----------------------------------| -| `/home/runner/work/repo/src/foo.py` | `src/foo.py` | -| `C:\Users\runner\work\repo\src\foo.py` | `src/foo.py` | -| `src/foo.py` | `src/foo.py` | -| `/usr/lib/python3.11/site-packages/mypkg/__init__.py` | `site-packages/mypkg/__init__.py` | -| `/foo/mysrc/bar.py` | `/foo/mysrc/bar.py` (no match — `mysrc` ≠ `src`) | -| `/some/random/path/unrelated.py` | `/some/random/path/unrelated.py` | +| Input filename | Canonical filename | +|-----------------------------------------------------------|-------------------------------------| +| `/home/runner/work/repo/src/foo.py` | `src/foo.py` | +| `C:\Users\runner\work\repo\src\foo.py` | `src/foo.py` | +| `/usr/lib/python3.11/site-packages/mypkg/__init__.py` | `mypkg/__init__.py` | +| `/some/random/path/unrelated.py` | `/some/random/path/unrelated.py` | + +A `**` pattern matches leftmost: against +`/a/site-packages/b/site-packages/c.py` with `--source '**/site-packages/'` +the canonical is `b/site-packages/c.py` (the first `site-packages` is +consumed). ## 5. Code organization @@ -191,10 +215,15 @@ the same discipline: ```python def parse_source_pattern(pattern: str) -> list[str]: - """Validate and tokenize a --source value into literal segments. + """Validate and tokenize a --source value into segments. + + Each segment is a literal directory name, '*' (one segment), or '**' + (zero or more segments). Leading/trailing '/' are stripped; backslashes + are normalized to forward slashes. - Raises ValueError on malformed patterns (mid-pattern wildcard, missing - trailing '/', empty after stripping leading '**/'). + Raises ValueError on in-segment wildcards (e.g. '*name'), empty + segments, an empty pattern, or a pattern of just '**' (which would + match everything). """ def canonicalize_filename( @@ -202,9 +231,11 @@ def canonicalize_filename( ) -> str: """Normalize separators and apply source remapping. - First pattern (in declaration order) that matches at the leftmost - directory boundary wins. If no pattern matches, returns the filename - with separators normalized only. + Each pattern is matched anchored at the first segment of the filename + (after a single leading '/' is stripped for matching). The first + pattern (in declaration order) that matches wins; the canonical name + is the remaining segments joined by '/'. If no pattern matches, + returns the filename with separators normalized only. """ def merge_reports( @@ -232,12 +263,13 @@ encoding="UTF-8", pretty_print=True)` and emitted via `tests/test_merge.py` covers the algorithm and the public functions: -- `parse_source_pattern`: literal segments, leading `**/`, error - cases (missing trailing slash, mid-pattern wildcard, only `**/`, - empty segment). -- `canonicalize_filename`: separator normalization, segment match, - Windows path, multi-segment pattern, no-match passthrough, - first-pattern-wins, leftmost-match-wins, partial-segment-no-match. +- `parse_source_pattern`: literal segments, leading/trailing slash + normalization, `*` and `**` wildcards, error cases (in-segment + wildcard, empty segment, empty pattern, lone `**`). +- `canonicalize_filename`: separator normalization, prefix strip, + Windows path, multi-segment pattern, `*` single-segment wildcard, + `**` leftmost match, no-match passthrough, first-pattern-wins, + partial-segment-no-match. - `merge_reports`: disjoint files, overlapping files (hit summation, line union), branch coverage with max-numerator selection, branch denominator mismatch resolved by largest-denominator-then-largest- @@ -251,18 +283,15 @@ encoding="UTF-8", pretty_print=True)` and emitted via - stdout default, `-o` to file, single positional input, missing positional input (Click usage error), `--source` cross-OS unify, - malformed `--source` (mid-pattern wildcard, missing trailing - slash) → `BadParameter`, branch denominator mismatch merges - successfully (largest denominator wins), `--ignore-regex` drops files - pre-merge. + malformed `--source` (in-segment wildcard, lone `**`) → + `BadParameter`, branch denominator mismatch merges successfully + (largest denominator wins), `--ignore-regex` drops files pre-merge. ## 7. Deferred to future iterations These are intentionally out of scope for v1; revisit only when a real need surfaces. -- **Full glob support in `--source`** (`*`, `**` mid-pattern). v1 - is prefix-match-only. - **Per-input source rules.** v1 applies all patterns to all inputs uniformly. - **Conflict-resolution modes for non-branch fields** (`--strict`, @@ -290,12 +319,23 @@ need surfaces. earlier strict rule, matching gcovr's default) made the merge command unusable in exactly the case it was designed for: combining reports from runs with different feature flags or platforms. -- **`--source` over `OLD=NEW` substitution or coverage.py-style - groups.** Reads naturally — "treat `src/` as a logical source root" — - and one flag per logical root scales cleanly. The user names the - destination directory once instead of enumerating every absolute - prefix variant. coverage.py's group form (canonical + alias list) - is more expressive but doesn't fit single-value CLI flags well. +- **`--source` as a prefix to strip, not an OLD=NEW substitution.** + Each pattern names where a logical root lives on a given filesystem; + the canonical filename is whatever follows. Users declare one + `--source` per filesystem variant (Linux path, Windows path, + `site-packages`, etc.) and all variants collapse to the same + project-relative key. coverage.py's group form (canonical + alias + list) is more expressive but doesn't fit single-value CLI flags + well. +- **Anchored prefix match with `*`/`**` wildcards.** Patterns match + starting at the first path segment (after a leading `/` is stripped + for matching). `**` is the explicit "match anywhere" mechanism — + required, not implicit, so `src/` and `**/src/` mean visibly + different things. In-segment wildcards (`*name`, `lib*`) are + deliberately omitted: they invite case-sensitivity confusion across + OSes, escaping-vs-regex expectations, and ambiguous matches; the + segment-granular form covers the real use cases without those + hazards. - **Bare `version` + generator comment (coverage.py style).** Every widely-deployed Cobertura emitter writes a bare version number in the `version` attribute and identifies itself through an XML comment. diff --git a/pycobertura/cli.py b/pycobertura/cli.py index 8ce2519..6e7fa1c 100644 --- a/pycobertura/cli.py +++ b/pycobertura/cli.py @@ -383,10 +383,12 @@ def _source_callback(ctx, param, value): view of the line) and, among ties, the largest numerator (most branches covered). -Use --source to canonicalize file paths that differ across reports -(e.g. between Linux and Windows runs). The pattern identifies a directory -that begins the canonical path; everything before it in the input filename -is stripped. A leading '**/' is allowed for clarity. +Use --source to canonicalize file paths that differ across reports (e.g. +between Linux and Windows runs). A --source pattern names a filesystem +prefix that identifies a logical root; the matching prefix is stripped +from each filename. Patterns are anchored: 'src/' matches only at the +start of a path. Use '**/' to match anywhere; '*' matches a single path +segment. Limitations: - Merged branch coverage is the maximum across reports, so if different branches @@ -396,7 +398,9 @@ def _source_callback(ctx, param, value): EXAMPLES - pycobertura merge linux.xml windows.xml --source src/ + pycobertura merge linux.xml windows.xml \\ + --source '/home/runner/work/repo/' \\ + --source 'C:/Users/runner/work/repo/' pycobertura merge a.xml b.xml c.xml -o merged.xml """ @@ -414,9 +418,10 @@ def _source_callback(ctx, param, value): multiple=True, metavar="", callback=_source_callback, - help="Path pattern (e.g. 'src/' or '**/site-packages/') identifying the " - "start of the canonical source path. Repeatable. The first matching " - "pattern wins per filename.", + help="Filesystem prefix identifying a logical root; the matching " + "prefix is stripped from each filename. Patterns are anchored at " + "the start of the path; use '**/' for match-anywhere (e.g. " + "'**/site-packages/'). Repeatable; first match wins.", ) @click.option( "-o", diff --git a/pycobertura/merge.py b/pycobertura/merge.py index 4b78bd5..d98f550 100644 --- a/pycobertura/merge.py +++ b/pycobertura/merge.py @@ -25,26 +25,24 @@ def parse_source_pattern(pattern: str) -> List[str]: - """Validate and tokenize a ``--source`` value into literal segments. + """Validate and tokenize a ``--source`` value into segments. - A leading ``**/`` is allowed (and stripped); no other wildcards are - permitted in v1. Pattern must end with ``/``. - """ - if not pattern.endswith("/"): - raise ValueError( - f"--source pattern must end with '/': {pattern!r}" - ) + Each segment is a literal directory name, ``*`` (one segment), or + ``**`` (zero or more segments). Leading and trailing ``/`` are + cosmetic; backslashes are normalized to forward slashes. - body = pattern[:-1] - if body.startswith("**/"): - body = body[len("**/"):] - elif body == "**": - raise ValueError( - f"--source pattern is empty after stripping leading '**/': {pattern!r}" - ) + Raises ``ValueError`` for in-segment wildcards (``*name``, ``lib*``), + empty segments (``a//b``), an empty pattern, or a pattern that + reduces to a single ``**`` (which would match everything). + """ + body = pattern.replace("\\", "/") + if body.startswith("/"): + body = body[1:] + if body.endswith("/"): + body = body[:-1] if not body: - raise ValueError(f"--source pattern has no literal segments: {pattern!r}") + raise ValueError(f"--source pattern is empty: {pattern!r}") segments = body.split("/") for seg in segments: @@ -52,12 +50,19 @@ def parse_source_pattern(pattern: str) -> List[str]: raise ValueError( f"--source pattern has empty segments: {pattern!r}" ) + if seg in ("*", "**"): + continue if "*" in seg: raise ValueError( - f"--source pattern uses unsupported wildcard in {seg!r} " - f"(only a leading '**/' is supported in v1): {pattern!r}" + f"--source pattern uses an in-segment wildcard in {seg!r}; " + f"only whole-segment '*' and '**' are supported: {pattern!r}" ) + if segments == ["**"]: + raise ValueError( + f"--source pattern '**' matches everything: {pattern!r}" + ) + return segments @@ -67,26 +72,56 @@ def canonicalize_filename( """Normalize separators and apply source remapping to ``filename``. ``patterns`` is the list of segment-lists produced by - :func:`parse_source_pattern`. The first pattern (in declaration order) - that matches at the leftmost directory boundary wins; the canonical name is - the suffix starting at the match. If no pattern matches, the filename is - returned with backslashes normalized to forward slashes. + :func:`parse_source_pattern`. Each pattern is matched anchored at the + first segment (after a single leading ``/`` is stripped for matching). + ``*`` matches one segment; ``**`` matches zero or more, leftmost. + The first pattern that matches wins; the canonical name is the + remaining segments joined by ``/``. If no pattern matches, the + filename is returned with backslashes normalized to forward slashes + only. """ norm = filename.replace("\\", "/") if not patterns: return norm - segments = norm.split("/") + work = norm[1:] if norm.startswith("/") else norm + segments = work.split("/") for pattern_segs in patterns: - plen = len(pattern_segs) - if plen == 0: + if not pattern_segs: continue - for start in range(len(segments) - plen + 1): - if segments[start:start + plen] == list(pattern_segs): - return "/".join(segments[start:]) + rest = _match_prefix(segments, list(pattern_segs)) + if rest is not None: + return "/".join(rest) return norm +def _match_prefix( + segments: List[str], pattern: List[str] +) -> "List[str] | None": + """Match ``pattern`` anchored at the start of ``segments``. + + Returns the remaining segments after the matched prefix, or ``None`` + if no match. ``**`` chooses the leftmost match (fewest consumed + segments such that the rest of the pattern still matches). + """ + if not pattern: + return segments + head = pattern[0] + rest = pattern[1:] + if head == "**": + # Try consuming 0, 1, 2, ... segments — leftmost match. + for k in range(0, len(segments) + 1): + tail = _match_prefix(segments[k:], rest) + if tail is not None: + return tail + return None + if not segments: + return None + if head == "*" or head == segments[0]: + return _match_prefix(segments[1:], rest) + return None + + def merge_reports( xml_roots: Sequence[ET._Element], sources: Sequence[Sequence[str]] = (), diff --git a/tests/test_cli.py b/tests/test_cli.py index 2c6806b..5d451a2 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1323,7 +1323,8 @@ def test_merge__source_unifies_cross_os_paths(): [ 'tests/merge-linux.xml', 'tests/merge-windows.xml', - '--source', 'src/', + '--source', '/home/runner/work/repo/', + '--source', 'C:/Users/runner/work/repo/', ], catch_exceptions=False, ) @@ -1334,30 +1335,30 @@ def test_merge__source_unifies_cross_os_paths(): assert files == ["src/main.py"] -def test_merge__source_rejects_mid_pattern_wildcard(): +def test_merge__source_rejects_in_segment_wildcard(): from pycobertura.cli import merge runner = CliRunner() result = runner.invoke( merge, - ['tests/merge-rust.xml', '--source', 'src/*/foo/'], + ['tests/merge-rust.xml', '--source', 'src/*name/'], catch_exceptions=False, ) assert result.exit_code != 0 - assert "unsupported wildcard" in result.output + assert "in-segment wildcard" in result.output -def test_merge__source_rejects_missing_trailing_slash(): +def test_merge__source_rejects_lone_double_star(): from pycobertura.cli import merge runner = CliRunner() result = runner.invoke( merge, - ['tests/merge-rust.xml', '--source', 'src'], + ['tests/merge-rust.xml', '--source', '**/'], catch_exceptions=False, ) assert result.exit_code != 0 - assert "must end with '/'" in result.output + assert "matches everything" in result.output def test_merge__branch_denominator_mismatch_succeeds(): diff --git a/tests/test_merge.py b/tests/test_merge.py index ce22ed3..9499c4d 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -1,5 +1,3 @@ -import copy - import lxml.etree as ET import pytest @@ -20,26 +18,59 @@ def test_parse_source_pattern_multi_segment(): assert parse_source_pattern("a/b/c/") == ["a", "b", "c"] -def test_parse_source_pattern_strips_leading_double_star(): - assert parse_source_pattern("**/site-packages/") == ["site-packages"] - assert parse_source_pattern("**/a/b/") == ["a", "b"] +def test_parse_source_pattern_trailing_slash_optional(): + assert parse_source_pattern("src") == ["src"] + assert parse_source_pattern("a/b/c") == ["a", "b", "c"] + + +def test_parse_source_pattern_leading_slash_optional(): + assert parse_source_pattern("/src/") == ["src"] + assert parse_source_pattern("/src") == ["src"] + assert parse_source_pattern("/Users/jdoe/proj/") == ["Users", "jdoe", "proj"] + + +def test_parse_source_pattern_normalizes_backslashes(): + assert parse_source_pattern("C:\\Users\\runner\\repo\\") == [ + "C:", "Users", "runner", "repo" + ] + assert parse_source_pattern("C:\\Users\\runner\\repo") == [ + "C:", "Users", "runner", "repo" + ] -def test_parse_source_pattern_rejects_missing_trailing_slash(): - with pytest.raises(ValueError, match="must end with '/'"): - parse_source_pattern("src") +def test_parse_source_pattern_double_star_segment(): + assert parse_source_pattern("**/site-packages/") == ["**", "site-packages"] + assert parse_source_pattern("**/a/b/") == ["**", "a", "b"] -def test_parse_source_pattern_rejects_mid_pattern_wildcard(): - with pytest.raises(ValueError, match="unsupported wildcard"): - parse_source_pattern("src/*/foo/") - with pytest.raises(ValueError, match="unsupported wildcard"): - parse_source_pattern("src/**/foo/") +def test_parse_source_pattern_single_star_segment(): + assert parse_source_pattern("*/foo.py") == ["*", "foo.py"] + assert parse_source_pattern("**/build/*/src/") == [ + "**", "build", "*", "src" + ] -def test_parse_source_pattern_rejects_only_double_star(): - with pytest.raises(ValueError, match="empty after stripping"): +def test_parse_source_pattern_rejects_in_segment_wildcard(): + with pytest.raises(ValueError, match="in-segment wildcard"): + parse_source_pattern("src/*name/") + with pytest.raises(ValueError, match="in-segment wildcard"): + parse_source_pattern("src/lib*/") + with pytest.raises(ValueError, match="in-segment wildcard"): + parse_source_pattern("src/na*me/") + + +def test_parse_source_pattern_rejects_lone_double_star(): + with pytest.raises(ValueError, match="matches everything"): parse_source_pattern("**/") + with pytest.raises(ValueError, match="matches everything"): + parse_source_pattern("**") + + +def test_parse_source_pattern_rejects_empty(): + with pytest.raises(ValueError, match="empty"): + parse_source_pattern("") + with pytest.raises(ValueError, match="empty"): + parse_source_pattern("/") def test_parse_source_pattern_rejects_empty_segment(): @@ -54,32 +85,85 @@ def test_canonicalize_filename_no_patterns_normalizes_separators(): assert canonicalize_filename(r"a\b\c.py", []) == "a/b/c.py" -def test_canonicalize_filename_matches_segment(): - patterns = [["src"]] +def test_canonicalize_filename_strips_project_root_linux(): + patterns = [["Users", "jdoe", "proj"]] assert ( - canonicalize_filename("/home/runner/work/repo/src/foo.py", patterns) + canonicalize_filename("/Users/jdoe/proj/src/foo.py", patterns) == "src/foo.py" ) -def test_canonicalize_filename_matches_windows_path(): - patterns = [["src"]] +def test_canonicalize_filename_strips_project_root_windows(): + patterns = [["C:", "Users", "runner", "repo"]] assert ( - canonicalize_filename(r"C:\Users\runner\work\repo\src\foo.py", patterns) + canonicalize_filename(r"C:\Users\runner\repo\src\foo.py", patterns) == "src/foo.py" ) -def test_canonicalize_filename_matches_at_start(): +def test_canonicalize_filename_cross_os_unification(): + patterns = [ + ["Users", "jdoe", "proj"], + ["C:", "Users", "runner", "repo"], + ] + a = canonicalize_filename("/Users/jdoe/proj/src/foo.py", patterns) + b = canonicalize_filename( + r"C:\Users\runner\repo\src\foo.py", patterns + ) + assert a == b == "src/foo.py" + + +def test_canonicalize_filename_strips_single_segment_prefix(): patterns = [["src"]] - assert canonicalize_filename("src/foo.py", patterns) == "src/foo.py" + assert canonicalize_filename("src/foo.py", patterns) == "foo.py" def test_canonicalize_filename_matches_multi_segment_pattern(): patterns = [["src", "main"]] assert ( - canonicalize_filename("/foo/bar/src/main/python/app.py", patterns) - == "src/main/python/app.py" + canonicalize_filename("src/main/python/app.py", patterns) + == "python/app.py" + ) + + +def test_canonicalize_filename_double_star_leftmost(): + patterns = [["**", "site-packages"]] + assert ( + canonicalize_filename( + "/usr/lib/python3.11/site-packages/mypkg/__init__.py", patterns + ) + == "mypkg/__init__.py" + ) + # Two site-packages — leftmost wins. + assert ( + canonicalize_filename( + "/a/site-packages/b/site-packages/c.py", patterns + ) + == "b/site-packages/c.py" + ) + + +def test_canonicalize_filename_double_star_matches_zero_segments(): + patterns = [["**", "src"]] + assert canonicalize_filename("src/foo.py", patterns) == "foo.py" + + +def test_canonicalize_filename_single_star_one_segment(): + patterns = [["*", "foo.py"]] + # '*' matches 'a', then 'foo.py' matches; whole path consumed. + assert canonicalize_filename("a/foo.py", patterns) == "" + # 'a/b/foo.py' has 'b' as second segment, not 'foo.py' → no match. + assert ( + canonicalize_filename("a/b/foo.py", patterns) == "a/b/foo.py" + ) + + +def test_canonicalize_filename_anchored_not_match_anywhere(): + patterns = [["src"]] + # Without leading '**', a pattern only matches at the start. + assert ( + canonicalize_filename("/home/runner/repo/src/foo.py", patterns) + == "/home/runner/repo/src/foo.py" ) @@ -88,24 +172,26 @@ def test_canonicalize_filename_no_match_normalizes_only(): assert canonicalize_filename(r"foo\bar\baz.py", patterns) == "foo/bar/baz.py" -def test_canonicalize_filename_first_pattern_wins(): - patterns = [["src"], ["site-packages"]] - # Both could potentially match if path had both; first declared wins. +def test_canonicalize_filename_no_match_preserves_leading_slash(): + patterns = [["src"]] assert ( - canonicalize_filename("/x/site-packages/src/foo.py", patterns) == "src/foo.py" + canonicalize_filename("/some/random/path.py", patterns) + == "/some/random/path.py" ) -def test_canonicalize_filename_leftmost_match_wins(): - patterns = [["src"]] - # Two `src` directories — leftmost wins. - assert canonicalize_filename("/a/src/b/src/c.py", patterns) == "src/b/src/c.py" +def test_canonicalize_filename_first_pattern_wins(): + patterns = [["src"], ["*"]] + # Both could match; first declared wins. + assert canonicalize_filename("src/foo.py", patterns) == "foo.py" def test_canonicalize_filename_partial_segment_does_not_match(): patterns = [["src"]] # 'mysrc' should not match 'src'. - assert canonicalize_filename("/foo/mysrc/bar.py", patterns) == "/foo/mysrc/bar.py" + assert ( + canonicalize_filename("mysrc/bar.py", patterns) == "mysrc/bar.py" + ) # ---- merge_reports ---- @@ -212,7 +298,10 @@ def _root(num, denom, hits): def test_merge_with_sources_unifies_cross_os_paths(): linux = _parse("tests/merge-linux.xml") windows = _parse("tests/merge-windows.xml") - patterns = [parse_source_pattern("src/")] + patterns = [ + parse_source_pattern("/home/runner/work/repo/"), + parse_source_pattern("C:/Users/runner/work/repo/"), + ] merged = merge_reports([linux, windows], sources=patterns) # Both inputs' main.py merge under canonical path From 3b836b94639ee51f2bac59f0747f924e752b3f6b Mon Sep 17 00:00:00 2001 From: David Hagen Date: Sat, 16 May 2026 19:19:32 -0400 Subject: [PATCH 06/13] Clean up references to things that don't exist --- merge-spec.md | 60 ++++++++++++++++++--------------------------- tests/test_merge.py | 6 ++--- 2 files changed, 27 insertions(+), 39 deletions(-) diff --git a/merge-spec.md b/merge-spec.md index b9e9c1c..517b0b8 100644 --- a/merge-spec.md +++ b/merge-spec.md @@ -1,6 +1,5 @@ # `pycobertura merge` — Feature Specification -Status: **v1** (initial implementation) Module: [`pycobertura/merge.py`](pycobertura/merge.py) CLI command: [`pycobertura/cli.py`](pycobertura/cli.py) `merge` Tests: [`tests/test_merge.py`](tests/test_merge.py), `merge` cases in [`tests/test_cli.py`](tests/test_cli.py) @@ -84,10 +83,15 @@ outcomes covered and `b` is the total number of branch outcomes. For each branch line that appears in 2+ inputs: 1. Parse `(a, b)` from each input's `condition-coverage`. -2. Restrict the candidates to those with the largest denominator `b`. The rationale is that a larger denominator implies more branch outcomes were detected, which we treat as a proxy for the most complete view of the source at that line. -3. Among those candidates, choose the input with the highest numerator `a` and copy its `condition-coverage` (re-emitted as `P% (a/b)` with recomputed P) and `` block (deep-copied). - -For comparison: gcovr's strict mode aborts on denominator mismatch; lcovz +2. Restrict the candidates to those with the largest denominator `b`. A + larger denominator implies more branch outcomes were detected, which + we treat as a proxy for the most complete view of the source at that + line. +3. Among those candidates, choose the input with the highest numerator + `a` and copy its `condition-coverage` (re-emitted as `P% (a/b)` with + recomputed P) and `` block (deep-copied). + +For comparison: gcovr's strict mode aborts on denominator mismatch; lcov silently merges (with documented data-loss bugs); coverage.py rejects type-level mismatches; JaCoCo/cobertura-merge tools tend to silently merge or drop information. Pycobertura prefers the largest-denominator entry @@ -206,10 +210,10 @@ the same discipline: `_Element` tree built via `lxml.etree.Element` / `SubElement`, with `copy.deepcopy` for whole subtree carry-overs (e.g. `` blocks chosen via the §3.4 rule). -- **No builder type in v1.** Direct construction via `SubElement` is - sufficient. A `CoberturaBuilder` class would be premature. -- **No `Cobertura.to_xml()` method.** The `Cobertura` class stays - read-only and aggressively memoized; merge operates on raw lxml trees. +- **Merge operates on raw lxml trees.** The `Cobertura` class stays + read-only and aggressively memoized; the merge module reads inputs + directly through lxml without constructing `Cobertura` instances and + emits output by building a fresh tree. ### 5.1 Public API of `pycobertura.merge` @@ -287,23 +291,7 @@ encoding="UTF-8", pretty_print=True)` and emitted via `BadParameter`, branch denominator mismatch merges successfully (largest denominator wins), `--ignore-regex` drops files pre-merge. -## 7. Deferred to future iterations - -These are intentionally out of scope for v1; revisit only when a real -need surfaces. - -- **Per-input source rules.** v1 applies all patterns to all inputs - uniformly. -- **Conflict-resolution modes for non-branch fields** (`--strict`, - `--merge-mode`). -- **Config-file form of source rules** (e.g. reading from - `pyproject.toml` or `.coveragerc`-style `[paths]` groups). -- **Stdin input.** -- **Re-serialization on the `Cobertura` class itself** (`to_xml`, - `CoberturaBuilder`). Introduce only when a second consumer of XML - serialization appears in the codebase. - -## 8. Design rationale (why these choices) +## 7. Design rationale (why these choices) - **Variadic positional inputs, default to stdout, optional `-o`.** Matches `show`/`diff`. Globbing (`coverage-*.xml`) is the dominant CI @@ -314,11 +302,12 @@ need surfaces. one build). The largest-denominator entry sees the most branches, so we treat it as the most complete view of that line and use its numerator/conditions directly. This loses no information relative to - the smaller-denominator entries — they describe a strict subset of the - branch outcomes the larger entry already accounts for. Aborting (the - earlier strict rule, matching gcovr's default) made the merge command - unusable in exactly the case it was designed for: combining reports - from runs with different feature flags or platforms. + the smaller-denominator entries — they describe a strict subset of + the branch outcomes the larger entry already accounts for. The + alternative of aborting on mismatch (gcovr's strict-mode behavior) + would make the merge command unusable in exactly the case it is + designed for: combining reports from runs with different feature + flags or platforms. - **`--source` as a prefix to strip, not an OLD=NEW substitution.** Each pattern names where a logical root lives on a given filesystem; the canonical filename is whatever follows. Users declare one @@ -343,9 +332,8 @@ need surfaces. output, but a tool-name-prefixed `version` would deviate from the ecosystem and risk breaking downstream parsers that try to coerce the attribute to a number. -- **No `to_xml()` on `Cobertura`.** Adding write methods to a - read-only, memoized class invites cache-coherence bugs. Keep merge as - a pure tree transformation in its own module. - **Pure functions returning fresh trees.** Matches the rest of the - codebase (`utils.py` and `Cobertura` are input-immutable). A builder - type would be premature for the v1 scope. + codebase (`utils.py` and `Cobertura` are input-immutable). The merge + module produces a new tree rather than mutating any input, which + keeps `Cobertura`'s memoized read API safe and the merge logic easy + to reason about in isolation. diff --git a/tests/test_merge.py b/tests/test_merge.py index 9499c4d..3cfd2e3 100644 --- a/tests/test_merge.py +++ b/tests/test_merge.py @@ -256,9 +256,9 @@ def test_merge_branch_lines_takes_max_coverage(): def test_merge_branch_denominator_mismatch_picks_largest_denominator(): - # a has 1/2 with 3 hits; b has 1/3 with 2 hits. Differing denominators no - # longer raise — largest denominator wins (treated as the most complete - # view of the source at that line), then largest numerator breaks ties. + # a has 1/2 with 3 hits; b has 1/3 with 2 hits. Largest denominator wins + # (treated as the most complete view of the source at that line), then + # largest numerator breaks ties. a = _parse("tests/merge-branch-conflict-a.xml") b = _parse("tests/merge-branch-conflict-b.xml") merged = merge_reports([a, b]) From a377a218ea9f42b2e87b12acdaca5b10f280c852 Mon Sep 17 00:00:00 2001 From: David Hagen Date: Thu, 18 Jun 2026 19:58:46 -0400 Subject: [PATCH 07/13] Deduplicate _line_is_hit and get_line_status --- pycobertura/merge.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/pycobertura/merge.py b/pycobertura/merge.py index d98f550..8ce6c0d 100644 --- a/pycobertura/merge.py +++ b/pycobertura/merge.py @@ -15,6 +15,8 @@ import lxml.etree as ET +from pycobertura.utils import get_line_status + try: from importlib.metadata import version as _pkg_version except ImportError: # pragma: no cover @@ -312,7 +314,7 @@ def _emit_merged_lines( merged = _merge_line_elements(n, line_elems) parent.append(merged) total += 1 - if _line_is_hit(merged): + if get_line_status(merged) == "hit": hits_count += 1 cc = merged.get("condition-coverage") if cc: @@ -368,16 +370,6 @@ def _merge_line_elements( return new_line -def _line_is_hit(line_elem: ET._Element) -> bool: - cc = line_elem.get("condition-coverage") - if cc: - return cc.lstrip().startswith("100%") - try: - return int(line_elem.get("hits", "0") or "0") > 0 - except ValueError: - return False - - def _parse_condition_coverage(cc: str) -> "Tuple[int, int] | None": m = _CONDITION_COVERAGE_RE.match(cc) if not m: From b9fee73f909ab04bf1fc15efff31c94c2ac2c3ae Mon Sep 17 00:00:00 2001 From: David Hagen Date: Thu, 18 Jun 2026 20:47:39 -0400 Subject: [PATCH 08/13] Remove all aggregate complexity numbers --- merge-spec.md | 26 ++++++++++++++++++++++---- pycobertura/cli.py | 9 +++++---- pycobertura/merge.py | 22 ++++++++++------------ 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/merge-spec.md b/merge-spec.md index 517b0b8..1d0909c 100644 --- a/merge-spec.md +++ b/merge-spec.md @@ -35,8 +35,12 @@ Output is always Cobertura XML. Exit codes: - `0` — merge succeeded. -- `1` (`ExitCodes.EXCEPTION`) — merge failed (input parse error, - malformed `--source`). +- `1` (`ExitCodes.EXCEPTION`) — merge failed at runtime (input file + could not be read or parsed). Raised as a `ClickException`. +- `2` — malformed argument value, e.g. a `--source` pattern that fails + to parse. Raised as a Click `BadParameter`/usage error, consistent with + how `--format` (`click.Choice`) and `--fail-threshold` (`click.IntRange`) + reject invalid values elsewhere in the CLI. ## 3. Merge semantics @@ -103,8 +107,22 @@ for that superset of branches). ### 3.5 Aggregates `line-rate`, `branch-rate` are recomputed from totals at every level -(class, package, root). `complexity` is recomputed as the maximum across -inputs (no canonical aggregation rule exists). +(class, package, root). + +`complexity` is carried at the **class level only**, taken as the maximum +across inputs for that class. Complexity is a property of the source, not +the coverage run, so the same class should report the same value in every +input; the max is a tiebreaker against drift (e.g. conditional compilation +changing the code). It is *not* emitted on `` or ``: +the Cobertura format defines aggregate complexity as the *average* CCN over +the contained methods/classes (e.g. a package `complexity="3.25"`), and that +average cannot be faithfully recomputed from merged class-level values without +the per-method counts the merge discards. Inventing a max- or sum-based number +there would not match the format's semantics, so aggregate complexity is +omitted rather than fabricated. This is permitted: although `coverage-04.dtd` +nominally marks `complexity` as required, real emitters routinely omit it +(coverage.py emits none; the Java Cobertura tool omits it on the root), and +pycobertura does not validate against the DTD. The root `` element also carries `lines-valid`, `lines-covered`, `branches-valid`, `branches-covered` attributes derived from the merged diff --git a/pycobertura/cli.py b/pycobertura/cli.py index 6e7fa1c..2c61984 100644 --- a/pycobertura/cli.py +++ b/pycobertura/cli.py @@ -391,10 +391,11 @@ def _source_callback(ctx, param, value): segment. Limitations: -- Merged branch coverage is the maximum across reports, so if different branches - are coveraged in different reports, the merged report may not capture that. - This is a limitation of the Cobertura XML format, which stores only the - fraction of branched covered, not which ones. +- Cobertura XML records only the fraction of branches covered at each line + (e.g. 1/2), not which specific branches. When reports cover different + branches of the same line, the merged report keeps a single entry (largest + denominator, then largest numerator) rather than unioning the covered + branches, so it may understate the true combined branch coverage. EXAMPLES diff --git a/pycobertura/merge.py b/pycobertura/merge.py index 8ce6c0d..c553ac8 100644 --- a/pycobertura/merge.py +++ b/pycobertura/merge.py @@ -186,11 +186,10 @@ def merge_reports( pkg_hits = 0 pkg_branches = 0 pkg_branch_hits = 0 - pkg_complexity = None for (class_name, canonical_filename), versions in classes_map.items(): - cls_elem, c_lines, c_hits, c_branches, c_branch_hits, c_complexity = ( - _merge_class(class_name, canonical_filename, versions) + cls_elem, c_lines, c_hits, c_branches, c_branch_hits = _merge_class( + class_name, canonical_filename, versions ) classes_elem.append(cls_elem) @@ -198,16 +197,9 @@ def merge_reports( pkg_hits += c_hits pkg_branches += c_branches pkg_branch_hits += c_branch_hits - if c_complexity is not None: - pkg_complexity = ( - c_complexity if pkg_complexity is None - else max(pkg_complexity, c_complexity) - ) pkg_elem.set("line-rate", _rate_str(pkg_hits, pkg_lines)) pkg_elem.set("branch-rate", _rate_str(pkg_branch_hits, pkg_branches)) - if pkg_complexity is not None: - pkg_elem.set("complexity", _format_float(pkg_complexity)) total_lines += pkg_lines total_hits += pkg_hits @@ -228,7 +220,7 @@ def _merge_class( class_name: str, canonical_filename: str, versions: Sequence[ET._Element], -) -> Tuple[ET._Element, int, int, int, int, "float | None"]: +) -> Tuple[ET._Element, int, int, int, int]: cls_elem = ET.Element("class") cls_elem.set("name", class_name) cls_elem.set("filename", canonical_filename) @@ -283,6 +275,12 @@ def _merge_class( cls_elem.set("line-rate", _rate_str(c_hits, c_total)) cls_elem.set("branch-rate", _rate_str(c_branch_hits, c_branches)) + # Complexity is a property of the source, not the coverage run, so the same + # class should carry the same value across inputs; max guards against drift + # (e.g. conditional compilation). It is kept at the class level only: the + # Cobertura format uses average complexity at the package/root level, which + # cannot be faithfully recomputed from merged class values, so aggregate + # complexity is deliberately not emitted rather than invented. complexity = None for v in versions: c = v.get("complexity") @@ -296,7 +294,7 @@ def _merge_class( if complexity is not None: cls_elem.set("complexity", _format_float(complexity)) - return cls_elem, c_total, c_hits, c_branches, c_branch_hits, complexity + return cls_elem, c_total, c_hits, c_branches, c_branch_hits def _emit_merged_lines( From a6f8e42f23debd68914c8d78fd38a052f0b0fab1 Mon Sep 17 00:00:00 2001 From: David Hagen Date: Thu, 18 Jun 2026 21:04:29 -0400 Subject: [PATCH 09/13] Harmonize code style with main project --- merge-spec.md | 2 +- pycobertura/cli.py | 2 +- pycobertura/merge.py | 88 +++++++++++++++++--------------------------- 3 files changed, 36 insertions(+), 56 deletions(-) diff --git a/merge-spec.md b/merge-spec.md index 1d0909c..0334831 100644 --- a/merge-spec.md +++ b/merge-spec.md @@ -236,7 +236,7 @@ the same discipline: ### 5.1 Public API of `pycobertura.merge` ```python -def parse_source_pattern(pattern: str) -> list[str]: +def parse_source_pattern(pattern: str) -> List[str]: """Validate and tokenize a --source value into segments. Each segment is a literal directory name, '*' (one segment), or '**' diff --git a/pycobertura/cli.py b/pycobertura/cli.py index 2c61984..98bfbb0 100644 --- a/pycobertura/cli.py +++ b/pycobertura/cli.py @@ -458,7 +458,7 @@ def merge(cobertura_files, ignore_regex, sources, output): def _drop_ignored_classes(root, ignore_regex): """Remove elements whose filename matches ignore_regex from a parsed tree.""" - classes = root.xpath("./packages/package/classes/class") + classes = list(root.iterfind("./packages/package/classes/class")) filenames = [cls.get("filename", "") for cls in classes] keep = set(get_filenames_that_do_not_match_regex(filenames, ignore_regex)) for cls in classes: diff --git a/pycobertura/merge.py b/pycobertura/merge.py index c553ac8..9e695bd 100644 --- a/pycobertura/merge.py +++ b/pycobertura/merge.py @@ -1,17 +1,7 @@ -"""Merge multiple Cobertura XML reports into one. - -The public entry point is :func:`merge_reports`, which takes a sequence of -parsed lxml roots and returns a freshly-built merged root. Inputs are treated -as read-only and never mutated. -""" - -from __future__ import annotations - import copy import re import time -from collections import OrderedDict -from typing import List, Sequence, Tuple +from typing import Dict, List, Sequence, Tuple, Union import lxml.etree as ET @@ -27,15 +17,15 @@ def parse_source_pattern(pattern: str) -> List[str]: - """Validate and tokenize a ``--source`` value into segments. + """Validate and tokenize a `--source` value into segments. - Each segment is a literal directory name, ``*`` (one segment), or - ``**`` (zero or more segments). Leading and trailing ``/`` are + Each segment is a literal directory name, `*` (one segment), or + `**` (zero or more segments). Leading and trailing `/` are cosmetic; backslashes are normalized to forward slashes. - Raises ``ValueError`` for in-segment wildcards (``*name``, ``lib*``), - empty segments (``a//b``), an empty pattern, or a pattern that - reduces to a single ``**`` (which would match everything). + Raises `ValueError` for in-segment wildcards (`*name`, `lib*`), + empty segments (`a//b`), an empty pattern, or a pattern that + reduces to a single `**` (which would match everything). """ body = pattern.replace("\\", "/") if body.startswith("/"): @@ -49,9 +39,7 @@ def parse_source_pattern(pattern: str) -> List[str]: segments = body.split("/") for seg in segments: if not seg: - raise ValueError( - f"--source pattern has empty segments: {pattern!r}" - ) + raise ValueError(f"--source pattern has empty segments: {pattern!r}") if seg in ("*", "**"): continue if "*" in seg: @@ -61,24 +49,20 @@ def parse_source_pattern(pattern: str) -> List[str]: ) if segments == ["**"]: - raise ValueError( - f"--source pattern '**' matches everything: {pattern!r}" - ) + raise ValueError(f"--source pattern '**' matches everything: {pattern!r}") return segments -def canonicalize_filename( - filename: str, patterns: Sequence[Sequence[str]] -) -> str: - """Normalize separators and apply source remapping to ``filename``. +def canonicalize_filename(filename: str, patterns: Sequence[Sequence[str]]) -> str: + """Normalize separators and apply source remapping to `filename`. - ``patterns`` is the list of segment-lists produced by - :func:`parse_source_pattern`. Each pattern is matched anchored at the - first segment (after a single leading ``/`` is stripped for matching). - ``*`` matches one segment; ``**`` matches zero or more, leftmost. + `patterns` is the list of segment-lists produced by + `parse_source_pattern`. Each pattern is matched anchored at the + first segment (after a single leading `/` is stripped for matching). + `*` matches one segment; `**` matches zero or more, leftmost. The first pattern that matches wins; the canonical name is the - remaining segments joined by ``/``. If no pattern matches, the + remaining segments joined by `/`. If no pattern matches, the filename is returned with backslashes normalized to forward slashes only. """ @@ -97,13 +81,11 @@ def canonicalize_filename( return norm -def _match_prefix( - segments: List[str], pattern: List[str] -) -> "List[str] | None": - """Match ``pattern`` anchored at the start of ``segments``. +def _match_prefix(segments: List[str], pattern: List[str]) -> Union[List[str], None]: + """Match `pattern` anchored at the start of `segments`. - Returns the remaining segments after the matched prefix, or ``None`` - if no match. ``**`` chooses the leftmost match (fewest consumed + Returns the remaining segments after the matched prefix, or `None` + if no match. `**` chooses the leftmost match (fewest consumed segments such that the rest of the pattern still matches). """ if not pattern: @@ -128,16 +110,16 @@ def merge_reports( xml_roots: Sequence[ET._Element], sources: Sequence[Sequence[str]] = (), ) -> ET._Element: - """Merge a sequence of Cobertura ```` roots into one fresh root. + """Merge a sequence of Cobertura `` roots into one fresh root. Inputs are treated as read-only. """ if not xml_roots: raise ValueError("merge_reports requires at least one input root") - sources_seen: "OrderedDict[str, None]" = OrderedDict() - # packages_data: package_name -> OrderedDict[(class_name, canonical_filename) -> list[]] - packages_data: "OrderedDict[str, OrderedDict[Tuple[str, str], List[ET._Element]]]" = OrderedDict() + sources_seen: Dict[str, None] = {} + # packages_data: package_name -> dict[(class_name, canonical_filename) -> list[]] + packages_data: Dict[str, Dict[Tuple[str, str], List[ET._Element]]] = {} for root in xml_roots: for src in root.iterfind("./sources/source"): @@ -147,7 +129,7 @@ def merge_reports( for pkg in root.iterfind("./packages/package"): pkg_name = pkg.get("name", "") or "" - pkg_classes = packages_data.setdefault(pkg_name, OrderedDict()) + pkg_classes = packages_data.setdefault(pkg_name, {}) for cls in pkg.iterfind("./classes/class"): class_name = cls.get("name", "") or "" filename = cls.get("filename", "") or "" @@ -160,8 +142,7 @@ def merge_reports( new_root.set("version", _pycobertura_version()) new_root.append( ET.Comment( - " Generated by pycobertura merge: " - "https://github.com/aconrad/pycobertura " + " Generated by pycobertura merge: https://github.com/aconrad/pycobertura " ) ) @@ -226,14 +207,14 @@ def _merge_class( cls_elem.set("filename", canonical_filename) # Group method line-elements by (name, signature) -> dict[line_number -> list[]] - methods_data: "OrderedDict[Tuple[str, str], OrderedDict[int, List[ET._Element]]]" = OrderedDict() + methods_data: Dict[Tuple[str, str], Dict[int, List[ET._Element]]] = {} # Class-level lines (direct ./lines/line on ): line_number -> list[] - class_lines: "OrderedDict[int, List[ET._Element]]" = OrderedDict() + class_lines: Dict[int, List[ET._Element]] = {} for v in versions: for m in v.iterfind("./methods/method"): key = (m.get("name", "") or "", m.get("signature", "") or "") - method_lines = methods_data.setdefault(key, OrderedDict()) + method_lines = methods_data.setdefault(key, {}) for line in m.iterfind("./lines/line"): n = int(line.get("number")) method_lines.setdefault(n, []).append(line) @@ -264,7 +245,7 @@ def _merge_class( ) else: # Build from union of merged method lines. - union: "OrderedDict[int, List[ET._Element]]" = OrderedDict() + union: Dict[int, List[ET._Element]] = {} for lines_by_number in methods_data.values(): for n, line_elems in lines_by_number.items(): union.setdefault(n, []).extend(line_elems) @@ -298,10 +279,10 @@ def _merge_class( def _emit_merged_lines( - lines_by_number: "OrderedDict[int, List[ET._Element]]", + lines_by_number: Dict[int, List[ET._Element]], parent: ET._Element, ) -> Tuple[int, int, int, int]: - """Append merged children to ``parent``. Returns (total, hits, branches, branch_hits).""" + """Append merged children to `parent`. Returns (total, hits, branches, branch_hits).""" total = 0 hits_count = 0 branches = 0 @@ -368,7 +349,7 @@ def _merge_line_elements( return new_line -def _parse_condition_coverage(cc: str) -> "Tuple[int, int] | None": +def _parse_condition_coverage(cc: str) -> Union[Tuple[int, int], None]: m = _CONDITION_COVERAGE_RE.match(cc) if not m: return None @@ -383,8 +364,7 @@ def _rate_str(numerator: int, denominator: int) -> str: def _format_float(value: float) -> str: # Match the formatting style of typical Cobertura output (e.g. "0.875"). - s = f"{value:.6g}" - return s + return f"{value:.6g}" def _pycobertura_version() -> str: From f74cd980ae1bb9efeb985e94b0cca414051359d9 Mon Sep 17 00:00:00 2001 From: David Hagen Date: Fri, 19 Jun 2026 06:37:45 -0400 Subject: [PATCH 10/13] Simplify ignore-regex code to do only one pass --- merge-spec.md | 10 +++++++--- pycobertura/cli.py | 15 +-------------- pycobertura/merge.py | 23 +++++++++++++++++------ pycobertura/utils.py | 30 +++++++++++++++++++----------- 4 files changed, 44 insertions(+), 34 deletions(-) diff --git a/merge-spec.md b/merge-spec.md index 0334831..7e04557 100644 --- a/merge-spec.md +++ b/merge-spec.md @@ -263,10 +263,13 @@ def canonicalize_filename( def merge_reports( xml_roots: Sequence[lxml.etree._Element], sources: Sequence[Sequence[str]] = (), + ignore_regex: str = None, ) -> lxml.etree._Element: """Merge a sequence of Cobertura roots into a fresh root. - Inputs are not mutated. Raises ValueError if xml_roots is empty. + Inputs are not mutated. If ignore_regex is given, elements + whose (pre-canonicalization) filename matches it are dropped from + every input before merging. Raises ValueError if xml_roots is empty. """ ``` @@ -275,8 +278,9 @@ def merge_reports( The `merge` command is registered on the existing `pycobertura` Click group. Each input is parsed via `lxml.etree.parse(...).getroot()` directly (no full `Cobertura` instance — wasteful and tied to the read -API). `--ignore-regex` drops matching `` elements from each -parsed tree before invoking `merge_reports`. The merged tree is +API). `--ignore-regex` is passed straight through to `merge_reports`, +which skips matching `` elements as it walks each input (no +mutation of the parsed trees). The merged tree is serialized via `lxml.etree.tostring(..., xml_declaration=True, encoding="UTF-8", pretty_print=True)` and emitted via `click.echo(..., file=output)` (same idiom as `show`). diff --git a/pycobertura/cli.py b/pycobertura/cli.py index 98bfbb0..5890a84 100644 --- a/pycobertura/cli.py +++ b/pycobertura/cli.py @@ -25,7 +25,6 @@ ) from pycobertura.utils import ( get_dir_from_file_path, - get_filenames_that_do_not_match_regex, ) pycobertura = click.Group() @@ -439,11 +438,9 @@ def merge(cobertura_files, ignore_regex, sources, output): root = ET.parse(path).getroot() except (ET.XMLSyntaxError, OSError) as e: raise click.ClickException(f"Failed to read {path}: {e}") - if ignore_regex: - _drop_ignored_classes(root, ignore_regex) roots.append(root) - merged = merge_reports(roots, sources=sources) + merged = merge_reports(roots, sources=sources, ignore_regex=ignore_regex) report = ET.tostring( merged, @@ -454,13 +451,3 @@ def merge(cobertura_files, ignore_regex, sources, output): isatty = True if output is None else output.isatty() click.echo(report, file=output, nl=isatty) - - -def _drop_ignored_classes(root, ignore_regex): - """Remove elements whose filename matches ignore_regex from a parsed tree.""" - classes = list(root.iterfind("./packages/package/classes/class")) - filenames = [cls.get("filename", "") for cls in classes] - keep = set(get_filenames_that_do_not_match_regex(filenames, ignore_regex)) - for cls in classes: - if cls.get("filename", "") not in keep: - cls.getparent().remove(cls) diff --git a/pycobertura/merge.py b/pycobertura/merge.py index 9e695bd..9dc498d 100644 --- a/pycobertura/merge.py +++ b/pycobertura/merge.py @@ -5,7 +5,11 @@ import lxml.etree as ET -from pycobertura.utils import get_line_status +from pycobertura.utils import ( + calculate_line_rate, + get_line_status, + make_filename_ignore_matcher, +) try: from importlib.metadata import version as _pkg_version @@ -109,14 +113,20 @@ def _match_prefix(segments: List[str], pattern: List[str]) -> Union[List[str], N def merge_reports( xml_roots: Sequence[ET._Element], sources: Sequence[Sequence[str]] = (), + ignore_regex: str = None, ) -> ET._Element: """Merge a sequence of Cobertura `` roots into one fresh root. - Inputs are treated as read-only. + Inputs are treated as read-only. If `ignore_regex` is given, any + `` whose (pre-canonicalization) `filename` matches it is dropped + from every input before merging; the value is interpreted exactly as in + the rest of the CLI (a regex, or a path to a file of `fnmatch` patterns). """ if not xml_roots: raise ValueError("merge_reports requires at least one input root") + is_ignored = make_filename_ignore_matcher(ignore_regex) if ignore_regex else None + sources_seen: Dict[str, None] = {} # packages_data: package_name -> dict[(class_name, canonical_filename) -> list[]] packages_data: Dict[str, Dict[Tuple[str, str], List[ET._Element]]] = {} @@ -131,8 +141,10 @@ def merge_reports( pkg_name = pkg.get("name", "") or "" pkg_classes = packages_data.setdefault(pkg_name, {}) for cls in pkg.iterfind("./classes/class"): - class_name = cls.get("name", "") or "" filename = cls.get("filename", "") or "" + if is_ignored is not None and is_ignored(filename): + continue + class_name = cls.get("name", "") or "" canonical = canonicalize_filename(filename, sources) key = (class_name, canonical) pkg_classes.setdefault(key, []).append(cls) @@ -357,9 +369,8 @@ def _parse_condition_coverage(cc: str) -> Union[Tuple[int, int], None]: def _rate_str(numerator: int, denominator: int) -> str: - if denominator == 0: - return "0" - return _format_float(numerator / denominator) + # numerator is the covered count; calculate_line_rate takes total and misses. + return _format_float(calculate_line_rate(denominator, denominator - numerator)) def _format_float(value: float) -> str: diff --git a/pycobertura/utils.py b/pycobertura/utils.py index 492dc2b..d9a8f57 100644 --- a/pycobertura/utils.py +++ b/pycobertura/utils.py @@ -268,21 +268,29 @@ def get_non_empty_non_commented_lines_from_file_in_ascii(file_path, comment_char return [res for res in result if res != ""] -def get_filenames_that_do_not_match_regex( - filenames, regex_param, comment_character="#" -): +def make_filename_ignore_matcher(regex_param, comment_character="#"): + """Return a predicate that is True for filenames that should be ignored. + + `regex_param` is either a path to a file of `fnmatch` patterns (one per + line, `#` comments) or a regular expression matched against the start of + each filename. The matcher is built once and can then be applied per file. + """ if os.path.isfile(regex_param): ignore_patterns = get_non_empty_non_commented_lines_from_file_in_ascii( regex_param, comment_character ) - remove_filenames = [ - filename - for igp in ignore_patterns - for filename in fnmatch.filter(filenames, igp) - ] - else: - remove_filenames = list(filter(re.compile(regex_param).match, filenames)) - return [fname for fname in filenames if fname not in remove_filenames] + return lambda filename: any( + fnmatch.fnmatch(filename, igp) for igp in ignore_patterns + ) + pattern = re.compile(regex_param) + return lambda filename: pattern.match(filename) is not None + + +def get_filenames_that_do_not_match_regex( + filenames, regex_param, comment_character="#" +): + ignore = make_filename_ignore_matcher(regex_param, comment_character) + return [fname for fname in filenames if not ignore(fname)] def get_line_status(line): From ce046cf8d71b6cb3c671dfe810f26866f667662d Mon Sep 17 00:00:00 2001 From: David Hagen Date: Fri, 19 Jun 2026 06:59:07 -0400 Subject: [PATCH 11/13] Put comment on otherwise strange variable --- pycobertura/merge.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pycobertura/merge.py b/pycobertura/merge.py index 9dc498d..9e33e2d 100644 --- a/pycobertura/merge.py +++ b/pycobertura/merge.py @@ -127,7 +127,7 @@ def merge_reports( is_ignored = make_filename_ignore_matcher(ignore_regex) if ignore_regex else None - sources_seen: Dict[str, None] = {} + sources_seen: Dict[str, None] = {} # stdlib OrderedSet # packages_data: package_name -> dict[(class_name, canonical_filename) -> list[]] packages_data: Dict[str, Dict[Tuple[str, str], List[ET._Element]]] = {} From fc8c918f6417dfd1931e2ae3252355734c1869a9 Mon Sep 17 00:00:00 2001 From: David Hagen Date: Fri, 19 Jun 2026 19:41:46 -0400 Subject: [PATCH 12/13] Run black --- pycobertura/cli.py | 12 ++++-------- pycobertura/merge.py | 16 +++++++++++----- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/pycobertura/cli.py b/pycobertura/cli.py index 5890a84..9ab4382 100644 --- a/pycobertura/cli.py +++ b/pycobertura/cli.py @@ -197,8 +197,7 @@ def show( } -@pycobertura.command( - help="""\ +@pycobertura.command(help="""\ The diff command compares and shows the changes between two Cobertura reports. NOTE: Reporting missing lines or showing the source code with the diff command @@ -209,8 +208,7 @@ def show( options `--source1` and `--source2` are necessary to point to the source code directories (or zip archives). If the source is not available at all, pass `--no-source` but missing lines and source code will not be reported. -""" -) +""") @click.argument("cobertura_file1") @click.argument("cobertura_file2") @click.option( @@ -372,8 +370,7 @@ def _source_callback(ctx, param, value): return parsed -@pycobertura.command( - help="""\ +@pycobertura.command(help="""\ Combine multiple Cobertura XML reports into a single merged report. Hits are summed across reports for matching lines; files are unioned across @@ -403,8 +400,7 @@ def _source_callback(ctx, param, value): --source 'C:/Users/runner/work/repo/' pycobertura merge a.xml b.xml c.xml -o merged.xml -""" -) +""") @click.argument("cobertura_files", nargs=-1, required=True) @click.option( "--ignore-regex", diff --git a/pycobertura/merge.py b/pycobertura/merge.py index 9e33e2d..2947783 100644 --- a/pycobertura/merge.py +++ b/pycobertura/merge.py @@ -128,7 +128,8 @@ def merge_reports( is_ignored = make_filename_ignore_matcher(ignore_regex) if ignore_regex else None sources_seen: Dict[str, None] = {} # stdlib OrderedSet - # packages_data: package_name -> dict[(class_name, canonical_filename) -> list[]] + # packages_data: package_name -> + # dict[(class_name, canonical_filename) -> list[]] packages_data: Dict[str, Dict[Tuple[str, str], List[ET._Element]]] = {} for root in xml_roots: @@ -218,7 +219,8 @@ def _merge_class( cls_elem.set("name", class_name) cls_elem.set("filename", canonical_filename) - # Group method line-elements by (name, signature) -> dict[line_number -> list[]] + # Group method line-elements by (name, signature) -> + # dict[line_number -> list[]] methods_data: Dict[Tuple[str, str], Dict[int, List[ET._Element]]] = {} # Class-level lines (direct ./lines/line on ): line_number -> list[] class_lines: Dict[int, List[ET._Element]] = {} @@ -248,8 +250,9 @@ def _merge_class( m_elem.set("branch-rate", _rate_str(m_branch_hits, m_branches)) # Class-level : merge from inputs' direct ./lines/line. - # For inputs that store data only in methods (class-level is empty or absent), - # fall back to the union of merged method lines so the class-level block is populated. + # For inputs that store data only in methods (class-level is empty + # or absent), fall back to the union of merged method lines so the + # class-level block is populated. cls_lines_elem = ET.SubElement(cls_elem, "lines") if class_lines: c_total, c_hits, c_branches, c_branch_hits = _emit_merged_lines( @@ -294,7 +297,10 @@ def _emit_merged_lines( lines_by_number: Dict[int, List[ET._Element]], parent: ET._Element, ) -> Tuple[int, int, int, int]: - """Append merged children to `parent`. Returns (total, hits, branches, branch_hits).""" + """Append merged children to `parent`. + + Returns (total, hits, branches, branch_hits). + """ total = 0 hits_count = 0 branches = 0 From 6480e8505d8c4f1b9e7d72a47fa4bbfdcc7e7bc3 Mon Sep 17 00:00:00 2001 From: David Hagen Date: Fri, 19 Jun 2026 06:59:40 -0400 Subject: [PATCH 13/13] Remove spec file before PR --- merge-spec.md | 361 -------------------------------------------------- 1 file changed, 361 deletions(-) delete mode 100644 merge-spec.md diff --git a/merge-spec.md b/merge-spec.md deleted file mode 100644 index 7e04557..0000000 --- a/merge-spec.md +++ /dev/null @@ -1,361 +0,0 @@ -# `pycobertura merge` — Feature Specification - -Module: [`pycobertura/merge.py`](pycobertura/merge.py) -CLI command: [`pycobertura/cli.py`](pycobertura/cli.py) `merge` -Tests: [`tests/test_merge.py`](tests/test_merge.py), `merge` cases in [`tests/test_cli.py`](tests/test_cli.py) -Fixtures: `tests/merge-*.xml` - -## 1. Purpose - -Combine multiple Cobertura XML reports into a single merged report. Designed -for the case where coverage is gathered from: - -- Multiple languages in the same project (e.g. Rust + Python), producing - disjoint file sets. -- The same code run with different feature flags activated, producing - partially overlapping file sets. -- The same code run on multiple operating systems, producing the same - logical files under different absolute paths and path-separator - conventions. - -## 2. CLI surface - -``` -pycobertura merge FILE1 [FILE...] [-o OUTPUT] [--ignore-regex REGEX] [--source PATTERN]... -``` - -| Argument / option | Required | Repeatable | Description | -|--------------------------------|----------|------------|----------------------------------------------------------------------------------------------------------| -| `FILE...` (positional) | yes (≥1) | n/a | One or more Cobertura XML file paths. A single file is accepted (round-trips through the merge pipeline). | -| `-o`, `--output PATH` | no | no | Write merged XML to PATH. Defaults to stdout (matches `show`/`diff`). | -| `--ignore-regex REGEX` | no | no | Drop `` elements whose `filename` matches REGEX from each input *before* merging. | -| `--source PATTERN` | no | yes | Filesystem prefix identifying a logical root; the matching prefix is stripped from each filename. See §4. | - -Output is always Cobertura XML. - -Exit codes: -- `0` — merge succeeded. -- `1` (`ExitCodes.EXCEPTION`) — merge failed at runtime (input file - could not be read or parsed). Raised as a `ClickException`. -- `2` — malformed argument value, e.g. a `--source` pattern that fails - to parse. Raised as a Click `BadParameter`/usage error, consistent with - how `--format` (`click.Choice`) and `--fail-threshold` (`click.IntRange`) - reject invalid values elsewhere in the CLI. - -## 3. Merge semantics - -### 3.1 Grouping - -Classes are grouped across all inputs by the tuple -`(package_name, class_name, canonical_filename)`. Filename alone is -insufficient because Java-style multi-class-per-file is real (e.g. -`Main.java` containing both `Main` and `Main$Helper` in -[tests/cobertura.xml](tests/cobertura.xml)). - -Methods within a class are grouped by `(name, signature)`. - -Lines within a method (or directly within a class for tools that don't emit -methods, like coverage.py) are grouped by line `number`. - -### 3.2 `` merge - -Union of `` text values across inputs, preserving first-seen order, -deduplicated. - -### 3.3 Line merge (per group) - -For each group of `` elements with the same number across inputs: - -| Attribute | Merge rule | -|----------------------|--------------------------------------------------------------------------------------------------| -| `number` | Same in all inputs (group key). | -| `hits` | Sum across inputs. | -| `branch` | `"true"` if any input marks it true, else `"false"`. | -| `condition-coverage` | If `branch="true"`: see §3.4. Else: absent. | -| `` child | Deep-copied from the input chosen by the §3.4 rule. | - -A line is considered "hit" (for line-rate computation) if its -`condition-coverage` starts with `"100%"`, or — when no `condition-coverage` -attribute is present — its `hits > 0`. This matches `pycobertura.utils.get_line_status`. - -### 3.4 Branch coverage merge - -Cobertura encodes per-line branch coverage as -`condition-coverage="P% (a/b)"` where `a` is the number of branch -outcomes covered and `b` is the total number of branch outcomes. - -For each branch line that appears in 2+ inputs: - -1. Parse `(a, b)` from each input's `condition-coverage`. -2. Restrict the candidates to those with the largest denominator `b`. A - larger denominator implies more branch outcomes were detected, which - we treat as a proxy for the most complete view of the source at that - line. -3. Among those candidates, choose the input with the highest numerator - `a` and copy its `condition-coverage` (re-emitted as `P% (a/b)` with - recomputed P) and `` block (deep-copied). - -For comparison: gcovr's strict mode aborts on denominator mismatch; lcov -silently merges (with documented data-loss bugs); coverage.py rejects -type-level mismatches; JaCoCo/cobertura-merge tools tend to silently merge -or drop information. Pycobertura prefers the largest-denominator entry -because it carries the most information; falling back to it never loses -coverage that the smaller-denominator entry uniquely captured (the -larger-denominator entry's numerator is already the best evidence we have -for that superset of branches). - -### 3.5 Aggregates - -`line-rate`, `branch-rate` are recomputed from totals at every level -(class, package, root). - -`complexity` is carried at the **class level only**, taken as the maximum -across inputs for that class. Complexity is a property of the source, not -the coverage run, so the same class should report the same value in every -input; the max is a tiebreaker against drift (e.g. conditional compilation -changing the code). It is *not* emitted on `` or ``: -the Cobertura format defines aggregate complexity as the *average* CCN over -the contained methods/classes (e.g. a package `complexity="3.25"`), and that -average cannot be faithfully recomputed from merged class-level values without -the per-method counts the merge discards. Inventing a max- or sum-based number -there would not match the format's semantics, so aggregate complexity is -omitted rather than fabricated. This is permitted: although `coverage-04.dtd` -nominally marks `complexity` as required, real emitters routinely omit it -(coverage.py emits none; the Java Cobertura tool omits it on the root), and -pycobertura does not validate against the DTD. - -The root `` element also carries `lines-valid`, `lines-covered`, -`branches-valid`, `branches-covered` attributes derived from the merged -totals (consistent with the `coverage-04.dtd` convention used by coverage.py). - -### 3.6 Root metadata - -Follows coverage.py's convention: bare version on the attribute, producer -identified via an XML comment under the root. - -| Attribute / node | Value | -|-----------------------|----------------------------------------------------------------------------------------------------| -| `version` | `importlib.metadata.version('pycobertura')` (e.g. `"4.1.0"`). Bare version, matches every other Cobertura emitter (coverage.py, the Java Cobertura tool, istanbul, etc.). | -| `timestamp` | `int(time.time() * 1000)` (Cobertura convention is ms since epoch). | -| `` | First child of ``: `Generated by pycobertura merge: https://github.com/aconrad/pycobertura`. Identifies the producer without polluting the `version` attribute. | - -## 4. Path remapping (`--source`) - -### 4.1 Motivation - -The same source file reported on Linux as -`/home/runner/work/repo/src/foo.py` and on Windows as -`C:\Users\runner\work\repo\src\foo.py` must merge to a single canonical -entry. A `--source` pattern names a filesystem prefix that identifies a -logical root; the matching prefix is stripped from the filename, leaving -the project-relative path as the canonical key. - -### 4.2 Pattern syntax - -A `--source` value is a sequence of `/`-separated segments. Leading and -trailing `/` are cosmetic (`src`, `/src`, `src/`, `/src/` all parse -identically). Backslashes in the pattern are normalized to forward -slashes before splitting. - -Each segment must be one of: - -- **A literal directory name** (e.g. `src`, `site-packages`, `Users`). -- **`*`** — matches exactly one path segment. -- **`**`** — matches zero or more path segments (leftmost match). - -In-segment wildcards (e.g. `*name`, `lib*`, `na*me`) are not supported and -raise `ValueError` at parse time. Empty segments (`src//foo`) and a -pattern that reduces to a single `**` (which would match everything and -strip the entire path) are also rejected. - -Examples after parsing: - -| Pattern | Segments | -|-------------------------------|-----------------------------------| -| `src/` | `["src"]` | -| `/Users/jdoe/myproj/` | `["Users", "jdoe", "myproj"]` | -| `**/site-packages/` | `["**", "site-packages"]` | -| `**/build/*/src/` | `["**", "build", "*", "src"]` | - -### 4.3 Matching algorithm (`canonicalize_filename`) - -For every `` element across inputs: - -1. Replace all backslashes in `X` with forward slashes. -2. If no patterns are declared, return the normalized filename unchanged. -3. Strip a single leading `/` (for matching purposes only) and split on - `/` to obtain the filename's segments. -4. For each declared `--source` pattern in order, attempt to match - anchored at the first segment: - - A literal segment matches iff equal. - - `*` matches exactly one segment. - - `**` matches zero or more segments and chooses the **leftmost** - match — it consumes the fewest segments such that the remainder of - the pattern matches starting at the next segment. -5. The first pattern that matches wins. The canonical filename is the - remaining segments (those after the matched prefix) joined by `/`. -6. If no pattern matches, return the filename with `\` → `/` - normalization only (any original leading `/` preserved). - -### 4.4 Examples - -``` -pycobertura merge linux.xml windows.xml \ - --source '/home/runner/work/repo/' \ - --source 'C:/Users/runner/work/repo/' \ - --source '**/site-packages/' -``` - -| Input filename | Canonical filename | -|-----------------------------------------------------------|-------------------------------------| -| `/home/runner/work/repo/src/foo.py` | `src/foo.py` | -| `C:\Users\runner\work\repo\src\foo.py` | `src/foo.py` | -| `/usr/lib/python3.11/site-packages/mypkg/__init__.py` | `mypkg/__init__.py` | -| `/some/random/path/unrelated.py` | `/some/random/path/unrelated.py` | - -A `**` pattern matches leftmost: against -`/a/site-packages/b/site-packages/c.py` with `--source '**/site-packages/'` -the canonical is `b/site-packages/c.py` (the first `site-packages` is -consumed). - -## 5. Code organization - -The codebase is consistently input-immutable. The merge module follows -the same discipline: - -- **Inputs are treated as read-only.** Functions in `merge.py` never - mutate caller-provided lxml elements. Output is a freshly-constructed - `_Element` tree built via `lxml.etree.Element` / `SubElement`, with - `copy.deepcopy` for whole subtree carry-overs (e.g. `` - blocks chosen via the §3.4 rule). -- **Merge operates on raw lxml trees.** The `Cobertura` class stays - read-only and aggressively memoized; the merge module reads inputs - directly through lxml without constructing `Cobertura` instances and - emits output by building a fresh tree. - -### 5.1 Public API of `pycobertura.merge` - -```python -def parse_source_pattern(pattern: str) -> List[str]: - """Validate and tokenize a --source value into segments. - - Each segment is a literal directory name, '*' (one segment), or '**' - (zero or more segments). Leading/trailing '/' are stripped; backslashes - are normalized to forward slashes. - - Raises ValueError on in-segment wildcards (e.g. '*name'), empty - segments, an empty pattern, or a pattern of just '**' (which would - match everything). - """ - -def canonicalize_filename( - filename: str, patterns: Sequence[Sequence[str]] -) -> str: - """Normalize separators and apply source remapping. - - Each pattern is matched anchored at the first segment of the filename - (after a single leading '/' is stripped for matching). The first - pattern (in declaration order) that matches wins; the canonical name - is the remaining segments joined by '/'. If no pattern matches, - returns the filename with separators normalized only. - """ - -def merge_reports( - xml_roots: Sequence[lxml.etree._Element], - sources: Sequence[Sequence[str]] = (), - ignore_regex: str = None, -) -> lxml.etree._Element: - """Merge a sequence of Cobertura roots into a fresh root. - - Inputs are not mutated. If ignore_regex is given, elements - whose (pre-canonicalization) filename matches it are dropped from - every input before merging. Raises ValueError if xml_roots is empty. - """ -``` - -### 5.2 CLI integration (`pycobertura.cli`) - -The `merge` command is registered on the existing `pycobertura` Click -group. Each input is parsed via `lxml.etree.parse(...).getroot()` -directly (no full `Cobertura` instance — wasteful and tied to the read -API). `--ignore-regex` is passed straight through to `merge_reports`, -which skips matching `` elements as it walks each input (no -mutation of the parsed trees). The merged tree is -serialized via `lxml.etree.tostring(..., xml_declaration=True, -encoding="UTF-8", pretty_print=True)` and emitted via -`click.echo(..., file=output)` (same idiom as `show`). - -## 6. Test surface - -`tests/test_merge.py` covers the algorithm and the public functions: - -- `parse_source_pattern`: literal segments, leading/trailing slash - normalization, `*` and `**` wildcards, error cases (in-segment - wildcard, empty segment, empty pattern, lone `**`). -- `canonicalize_filename`: separator normalization, prefix strip, - Windows path, multi-segment pattern, `*` single-segment wildcard, - `**` leftmost match, no-match passthrough, first-pattern-wins, - partial-segment-no-match. -- `merge_reports`: disjoint files, overlapping files (hit summation, - line union), branch coverage with max-numerator selection, branch - denominator mismatch resolved by largest-denominator-then-largest- - numerator, cross-OS path unification with `--source`, multi- - class-per-file preservation, no-branch-rate input, empty methods, - single-input round-trip, input-immutability invariant, version - attribute, source union, root-level rate aggregation, empty input - rejection. - -`tests/test_cli.py` covers the CLI surface: - -- stdout default, `-o` to file, single positional input, missing - positional input (Click usage error), `--source` cross-OS unify, - malformed `--source` (in-segment wildcard, lone `**`) → - `BadParameter`, branch denominator mismatch merges successfully - (largest denominator wins), `--ignore-regex` drops files pre-merge. - -## 7. Design rationale (why these choices) - -- **Variadic positional inputs, default to stdout, optional `-o`.** - Matches `show`/`diff`. Globbing (`coverage-*.xml`) is the dominant CI - pattern, and positional args support it cleanly. -- **Largest-denominator-wins on branch denominator mismatch.** Differing - denominators imply different generated branch structure at the same - source line (e.g. conditional compilation enabling extra branches in - one build). The largest-denominator entry sees the most branches, so - we treat it as the most complete view of that line and use its - numerator/conditions directly. This loses no information relative to - the smaller-denominator entries — they describe a strict subset of - the branch outcomes the larger entry already accounts for. The - alternative of aborting on mismatch (gcovr's strict-mode behavior) - would make the merge command unusable in exactly the case it is - designed for: combining reports from runs with different feature - flags or platforms. -- **`--source` as a prefix to strip, not an OLD=NEW substitution.** - Each pattern names where a logical root lives on a given filesystem; - the canonical filename is whatever follows. Users declare one - `--source` per filesystem variant (Linux path, Windows path, - `site-packages`, etc.) and all variants collapse to the same - project-relative key. coverage.py's group form (canonical + alias - list) is more expressive but doesn't fit single-value CLI flags - well. -- **Anchored prefix match with `*`/`**` wildcards.** Patterns match - starting at the first path segment (after a leading `/` is stripped - for matching). `**` is the explicit "match anywhere" mechanism — - required, not implicit, so `src/` and `**/src/` mean visibly - different things. In-segment wildcards (`*name`, `lib*`) are - deliberately omitted: they invite case-sensitivity confusion across - OSes, escaping-vs-regex expectations, and ambiguous matches; the - segment-granular form covers the real use cases without those - hazards. -- **Bare `version` + generator comment (coverage.py style).** Every - widely-deployed Cobertura emitter writes a bare version number in the - `version` attribute and identifies itself through an XML comment. - Copying the attribute from a first input would mislabel pycobertura's - output, but a tool-name-prefixed `version` would deviate from the - ecosystem and risk breaking downstream parsers that try to coerce the - attribute to a number. -- **Pure functions returning fresh trees.** Matches the rest of the - codebase (`utils.py` and `Cobertura` are input-immutable). The merge - module produces a new tree rather than mutating any input, which - keeps `Cobertura`'s memoized read API safe and the merge logic easy - to reason about in isolation.