Repository navigation
quality: report changed executable-line coverage in Python and Vitest pilots (#651) #686
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| """Informational changed-executable-line coverage for the two #651 B1 pilots. | ||
|
|
||
| This is deliberately *not* total/PR-wide coverage: only lines in the explicitly | ||
| instrumented files count. Non-executable diff lines are excluded and unavailable | ||
| data is reported as unavailable, never as a fabricated 0% or 100%. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import argparse | ||
| import json | ||
| import re | ||
| from pathlib import Path | ||
|
|
||
| HUNK = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@") | ||
|
|
||
|
|
||
| def added_lines(patch: str) -> dict[str, set[int]]: | ||
| """Return new-side added line numbers from Git's zero-context diff.""" | ||
| result: dict[str, set[int]] = {} | ||
| current: str | None = None | ||
| for line in patch.splitlines(): | ||
| if line.startswith("+++ "): | ||
| name = line[4:] | ||
| current = name[2:] if name.startswith("b/") else None | ||
| elif current is not None: | ||
| match = HUNK.match(line) | ||
| if match: | ||
| start = int(match.group(1)) | ||
| count = int(match.group(2)) if match.group(2) is not None else 1 | ||
| if count: | ||
| result.setdefault(current, set()).update(range(start, start + count)) | ||
| return result | ||
|
|
||
|
|
||
| def _find_entry(report: dict, source: str, kind: str) -> dict | None: | ||
| files = report["files"] if kind == "coveragepy" else { | ||
| key: value for key, value in report.items() if key != "total" | ||
| } | ||
| matches = [ | ||
| value for key, value in files.items() | ||
| if key.replace("\\", "/") == source | ||
| or key.replace("\\", "/").endswith("/" + source) | ||
| ] | ||
| if len(matches) > 1: | ||
| raise ValueError(f"Ambiguous coverage source: {source}") | ||
| return matches[0] if matches else None | ||
|
|
||
|
|
||
| def executable_lines(entry: dict, kind: str) -> tuple[set[int], set[int]] | None: | ||
| """Return (executable, covered) lines, or None if detail is unavailable.""" | ||
| if kind == "coveragepy": | ||
| if "executed_lines" not in entry or "missing_lines" not in entry: | ||
| return None | ||
| covered = set(map(int, entry["executed_lines"])) | ||
| missing = set(map(int, entry["missing_lines"])) | ||
| return covered | missing, covered | ||
| # Vitest's V8 provider uses Istanbul's JSON reporter. Unlike older | ||
| # Istanbul formats, coverage-final.json generally has statementMap/s | ||
| # but no precomputed line map. Combine statement hits on their start | ||
| # lines, using max so an uncovered statement cannot erase a covered one. | ||
| statements, hits = entry.get("statementMap"), entry.get("s") | ||
| if isinstance(statements, dict) and isinstance(hits, dict) and statements: | ||
| lines: dict[int, int] = {} | ||
| for statement_id, location in statements.items(): | ||
| if statement_id not in hits: | ||
| return None | ||
| line = int(location["start"]["line"]) | ||
| lines[line] = max(lines.get(line, 0), int(hits[statement_id])) | ||
| return set(lines), {line for line, count in lines.items() if count > 0} | ||
| # Older Istanbul JSON representations may include line hits directly. | ||
| lines = entry.get("l") | ||
| if not isinstance(lines, dict) or not lines: | ||
| return None | ||
|
Fooftilly marked this conversation as resolved.
|
||
| return {int(n) for n in lines}, {int(n) for n, count in lines.items() if count > 0} | ||
|
|
||
|
|
||
| def changed_line_report( | ||
| patch: str, report: dict, kind: str, sources: list[str] | ||
| ) -> str: | ||
| """Write a bounded Markdown summary without treating comments as misses.""" | ||
| touched = added_lines(patch) | ||
| rows = [ | ||
| "### Changed executable lines (instrumented pilot modules only)", | ||
| "", | ||
| "| Module | Added executable lines covered |", | ||
| "| --- | ---: |", | ||
| ] | ||
| covered_total = measured_total = 0 | ||
| unavailable = False | ||
| for source in sources: | ||
| added = touched.get(source, set()) | ||
| if not added: | ||
| rows.append(f"| `{source}` | No added lines in this pilot |") | ||
| continue | ||
| entry = _find_entry(report, source, kind) | ||
| detail = executable_lines(entry, kind) if entry is not None else None | ||
| if detail is None: | ||
| unavailable = True | ||
| rows.append(f"| `{source}` | Unavailable (no line-level coverage data) |") | ||
| continue | ||
| executable, covered = detail | ||
| eligible = added & executable | ||
| hit = len(eligible & covered) | ||
| covered_total += hit | ||
| measured_total += len(eligible) | ||
| if eligible: | ||
| rows.append(f"| `{source}` | {hit}/{len(eligible)} ({hit / len(eligible):.0%}) |") | ||
| else: | ||
| rows.append(f"| `{source}` | No added executable lines |") | ||
| overall = ( | ||
| "Unavailable for some changed pilot files" | ||
| if unavailable else | ||
| f"{covered_total}/{measured_total} ({covered_total / measured_total:.0%})" | ||
| if measured_total else "No added executable lines in pilot scope" | ||
| ) | ||
| rows.extend([ | ||
| f"| **Total (measurable pilot lines only)** | **{overall}** |", | ||
| "", | ||
| "Informational; no threshold. Counts only newly added executable lines in", | ||
| "the selected pilot modules, not total PR coverage, and excludes comments,", | ||
| "blank lines, deleted lines, uninstrumented modules and branch outcomes.", | ||
| "An unavailable source is never silently counted as covered.", | ||
| "", | ||
| ]) | ||
| return "\n".join(rows) | ||
|
|
||
|
|
||
| def main() -> None: | ||
| parser = argparse.ArgumentParser(description=__doc__) | ||
| parser.add_argument("--format", choices=("coveragepy", "istanbul"), required=True) | ||
| parser.add_argument("--report", type=Path, required=True) | ||
| parser.add_argument("--diff", type=Path, required=True) | ||
| parser.add_argument("--source", action="append", required=True) | ||
| parser.add_argument("--summary-file", type=Path, required=True) | ||
| args = parser.parse_args() | ||
| patch = args.diff.read_text(encoding="utf-8") | ||
| report = json.loads(args.report.read_text(encoding="utf-8")) | ||
| summary = changed_line_report(patch, report, args.format, args.source) | ||
| print(summary) | ||
| with args.summary_file.open("a", encoding="utf-8") as output: | ||
| output.write(summary) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,125 @@ | ||
| """Synthetic, deterministic regression tests for focused changed-line coverage.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import sys | ||
| import unittest | ||
| from pathlib import Path | ||
|
|
||
| ROOT = Path(__file__).resolve().parents[1] | ||
| if str(ROOT) not in sys.path: | ||
| sys.path.insert(0, str(ROOT)) | ||
|
|
||
| from scripts.changed_line_coverage import added_lines, changed_line_report, executable_lines | ||
|
|
||
| PYTHON_SOURCE = "backend/domain/text_identity.py" | ||
| TS_SOURCE = "frontend-app/src/workspace/tree.ts" | ||
|
|
||
|
|
||
| def _diff(path: str, first: int, count: int) -> str: | ||
| return ( | ||
| f"diff --git a/{path} b/{path}\n" | ||
| f"--- a/{path}\n+++ b/{path}\n" | ||
| f"@@ -1,0 +{first},{count} @@\n" | ||
| + "".join("+line\n" for _ in range(count)) | ||
| ) | ||
|
|
||
|
|
||
| class ChangedLineCoverageTests(unittest.TestCase): | ||
| def test_zero_context_hunks_multiple_files_and_deletions(self): | ||
| patch = ( | ||
| _diff(PYTHON_SOURCE, 8, 3) | ||
| + f"@@ -16 +19 @@\n+replacement\n" | ||
| + f"diff --git a/docs/readme.md b/docs/readme.md\n" | ||
| + "--- a/docs/readme.md\n+++ b/docs/readme.md\n" | ||
| + "@@ -10 +10,0 @@\n-deleted\n" | ||
| + _diff(TS_SOURCE, 30, 1) | ||
| ) | ||
| self.assertEqual( | ||
| added_lines(patch), | ||
| {PYTHON_SOURCE: {8, 9, 10, 19}, TS_SOURCE: {30}}, | ||
| ) | ||
|
|
||
| def test_python_ignores_nonexecutables_and_reports_partial_coverage(self): | ||
| patch = _diff(PYTHON_SOURCE, 8, 4) | ||
| report = { | ||
| "files": { | ||
| PYTHON_SOURCE: { | ||
| "executed_lines": [8, 14], | ||
| "missing_lines": [10, 15], | ||
| } | ||
| } | ||
| } | ||
| result = changed_line_report(patch, report, "coveragepy", [PYTHON_SOURCE]) | ||
| self.assertIn("1/2 (50%)", result) | ||
| self.assertIn("excludes comments", result) | ||
|
|
||
| def test_istanbul_report_uses_line_hits_and_path_suffix(self): | ||
| patch = _diff(TS_SOURCE, 30, 4) | ||
| report = { | ||
| "/runner/work/prks/" + TS_SOURCE: { | ||
| "l": {"30": 5, "31": 0, "33": 2, "40": 0}, | ||
| }, | ||
| "total": {"lines": {}}, | ||
| } | ||
| result = changed_line_report(patch, report, "istanbul", [TS_SOURCE]) | ||
| self.assertIn("2/3 (67%)", result) | ||
|
|
||
| def test_vitest_actual_coverage_final_statement_map_without_line_hits(self): | ||
| # Istanbul's JSON reporter usually supplies statementMap + s, not l. | ||
| patch = _diff(TS_SOURCE, 30, 4) | ||
| report = { | ||
| "/runner/prks/" + TS_SOURCE: { | ||
| "statementMap": { | ||
| "0": {"start": {"line": 30, "column": 0}, "end": {"line": 30, "column": 8}}, | ||
| "1": {"start": {"line": 31, "column": 0}, "end": {"line": 31, "column": 8}}, | ||
| "2": {"start": {"line": 31, "column": 9}, "end": {"line": 31, "column": 20}}, | ||
| "3": {"start": {"line": 33, "column": 0}, "end": {"line": 33, "column": 8}}, | ||
| }, | ||
| "s": {"0": 4, "1": 0, "2": 3, "3": 0}, | ||
| } | ||
| } | ||
| result = changed_line_report(patch, report, "istanbul", [TS_SOURCE]) | ||
| self.assertIn("2/3 (67%)", result) | ||
|
|
||
| def test_missing_istanbul_statement_hits_are_unavailable(self): | ||
| incomplete = { | ||
| "statementMap": { | ||
| "0": {"start": {"line": 30, "column": 0}, "end": {"line": 30, "column": 3}}, | ||
| }, | ||
| "s": {}, | ||
| } | ||
| self.assertIsNone(executable_lines(incomplete, "istanbul")) | ||
|
|
||
| def test_unchanged_pilot_sources_do_not_claim_zero_coverage(self): | ||
| report = {"files": {}} | ||
| result = changed_line_report(_diff("docs/note.md", 1, 1), report, "coveragepy", [PYTHON_SOURCE]) | ||
| self.assertIn("No added executable lines in pilot scope", result) | ||
| self.assertNotIn("(100%)", result) | ||
|
|
||
| def test_missing_changed_file_coverage_is_explicitly_unavailable(self): | ||
| result = changed_line_report( | ||
| _diff(PYTHON_SOURCE, 2, 1), {"files": {}}, "coveragepy", [PYTHON_SOURCE] | ||
| ) | ||
| self.assertIn("Unavailable for some changed pilot files", result) | ||
| self.assertNotIn("0/1", result) | ||
|
|
||
| def test_coverage_without_line_level_detail_is_unavailable(self): | ||
| self.assertIsNone(executable_lines({"covered_lines": 1}, "coveragepy")) | ||
| self.assertIsNone(executable_lines({"lines": {"pct": 100}}, "istanbul")) | ||
|
|
||
| def test_ambiguous_coverage_paths_raise(self): | ||
| report = { | ||
| "files": { | ||
| "/runner/one/" + PYTHON_SOURCE: {"executed_lines": [], "missing_lines": [2]}, | ||
| "/runner/two/" + PYTHON_SOURCE: {"executed_lines": [], "missing_lines": [2]}, | ||
| } | ||
| } | ||
| with self.assertRaisesRegex(ValueError, "Ambiguous"): | ||
| changed_line_report( | ||
| _diff(PYTHON_SOURCE, 2, 1), report, "coveragepy", [PYTHON_SOURCE] | ||
| ) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.