Skip to main content

testing_conventions/
patch_coverage.rs

1//! Patch (changed-line) coverage (Python — #132; TypeScript — #135; parent #46).
2//!
3//! Enforces the README Coverage rule's changed-line guarantee: every line a diff
4//! touches must be covered by the unit suite. Where [`crate::coverage`] measures
5//! the *whole* suite against a floor (#26) and the #131 ratchet against a
6//! baseline, this measures only the lines `<base>...HEAD` added or modified —
7//! failing when any changed, executable line is left uncovered.
8//!
9//! Two inputs are combined:
10//!   - the **diff** — [`changed_lines`] runs `git diff --unified=0 <base>...HEAD`
11//!     and returns the new-side line numbers each file gained. This diff machinery
12//!     is language-agnostic, shared by both arms (and the forthcoming Rust twin).
13//!   - the **coverage** — per the language. Python ([`check`]) reads coverage.py's
14//!     per-file `missing_lines` / `missing_branches`
15//!     ([`crate::coverage::measure_patch_report`]); a changed line is uncovered
16//!     when it is a missing line or the source of a branch the suite never took
17//!     ([`uncovered_changed_lines`]). TypeScript ([`check_typescript`]) reads
18//!     vitest's per-file v8 coverage reduced to one uncovered-line set per file
19//!     ([`crate::coverage::measure_patch_typescript`]) and intersects it directly
20//!     ([`uncovered_changed_lines_ts`]). Either way, non-executable changed lines
21//!     (comments, blanks) and `coverage`-exempt files have nothing to cover and
22//!     are skipped.
23//!
24//! Relationship to the commit-scoped co-change rule ([`crate::co_change`], #33):
25//! co-change enforces that a changed source and its colocated *test* move
26//! together; patch coverage enforces that the changed *lines* are actually
27//! exercised. They are complementary, not overlapping — co-change can pass (the
28//! test file changed) while patch coverage fails (the change isn't covered), and
29//! vice versa.
30
31use std::collections::{BTreeMap, BTreeSet};
32use std::path::Path;
33use std::process::Command;
34
35use anyhow::{bail, Context, Result};
36
37use crate::coverage::{self, FileCoverage};
38
39/// A changed source line the unit suite doesn't cover — a `root`-relative path
40/// and the 1-based new-side line number.
41#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
42pub struct Uncovered {
43    /// `root`-relative path of the changed file.
44    pub file: String,
45    /// The 1-based new-side line number that isn't covered.
46    pub line: u64,
47}
48
49/// Every line added or modified in `root`'s `<base>...HEAD` diff that the unit
50/// suite doesn't cover, sorted for deterministic output. `omit` is the
51/// `coverage`-rule exemptions (as in [`crate::coverage::measure`]) — an exempt
52/// file is omitted from the run, so its changed lines are lifted.
53///
54/// Scopes to `.py` sources (the Python arm this slice) and returns early — with
55/// no coverage run — when the diff touches none, so a PR that changes only docs or
56/// other languages doesn't pay for a measurement. Requires coverage.py + pytest +
57/// git; an unresolvable `base` surfaces as an error rather than a silent pass.
58pub fn check(root: &Path, base: &str, omit: &[String]) -> Result<Vec<Uncovered>> {
59    let mut changed = changed_lines(root, base)?;
60    changed.retain(|path, _| path.ends_with(".py"));
61    if changed.is_empty() {
62        return Ok(Vec::new());
63    }
64    let report = coverage::measure_patch_report(root, omit)?;
65    let files = relative_keys(report.files, root);
66    Ok(uncovered_changed_lines(&changed, &files))
67}
68
69/// TypeScript source extensions patch coverage scopes to — the set
70/// `coverage`'s `TS_INCLUDE` measures. A `.d.ts` declaration ends in `.ts` but
71/// carries no runtime code; vitest excludes it from the report, so its changed
72/// lines find nothing to cover and are skipped without a special case here.
73const TS_EXTENSIONS: [&str; 4] = [".ts", ".tsx", ".mts", ".cts"];
74
75/// Every line added or modified in `root`'s `<base>...HEAD` diff that the
76/// TypeScript unit suite (vitest) doesn't cover, sorted for deterministic output.
77/// `exclude` is the `coverage`-rule exemptions (as in
78/// [`crate::coverage::measure_typescript`]) — an excluded file is left out of the
79/// run, so its changed lines are lifted.
80///
81/// The TypeScript twin of [`check`] (#135): reuses the same `<base>...HEAD` diff
82/// machinery ([`changed_lines`]), scoped to `.ts` / `.tsx` / `.mts` / `.cts`
83/// sources, and maps the changed lines against vitest's per-file v8 coverage
84/// ([`crate::coverage::measure_patch_typescript`]). Returns early — with no
85/// coverage run — when the diff touches no TypeScript source, so a PR that changes
86/// only docs or other languages doesn't pay for a measurement. Requires vitest +
87/// git; an unresolvable `base` surfaces as an error rather than a silent pass.
88pub fn check_typescript(root: &Path, base: &str, exclude: &[String]) -> Result<Vec<Uncovered>> {
89    let mut changed = changed_lines(root, base)?;
90    changed.retain(|path, _| TS_EXTENSIONS.iter().any(|ext| path.ends_with(ext)));
91    if changed.is_empty() {
92        return Ok(Vec::new());
93    }
94    let uncovered = relative_keys(coverage::measure_patch_typescript(root, exclude)?, root);
95    Ok(uncovered_changed_lines_ts(&changed, &uncovered))
96}
97
98/// The new-side lines each file gained in `repo`'s `<base>...HEAD` diff, keyed by
99/// `repo`-relative path. The diff machinery shared by the TS / Rust twins.
100///
101/// `<base>...HEAD` is the merge-base diff — the changes this branch introduced
102/// (what a PR shows). `--unified=0` drops context lines so every `+` line is a
103/// real addition; `--no-renames` keeps a rename a delete + an add (the added side
104/// is held to coverage); `--relative` reports paths relative to `repo`. Returns an
105/// error if `git diff` fails (e.g. `base` names no resolvable ref).
106pub fn changed_lines(repo: &Path, base: &str) -> Result<BTreeMap<String, BTreeSet<u64>>> {
107    let range = format!("{base}...HEAD");
108    let output = Command::new("git")
109        .current_dir(repo)
110        .args([
111            "diff",
112            "--no-color",
113            "--no-renames",
114            "--unified=0",
115            "--relative",
116            &range,
117        ])
118        .output()
119        .with_context(|| format!("running `git diff` in `{}`", repo.display()))?;
120    if !output.status.success() {
121        bail!(
122            "`git diff {range}` failed in `{}`: {}",
123            repo.display(),
124            String::from_utf8_lossy(&output.stderr).trim()
125        );
126    }
127    Ok(parse_unified_diff(&String::from_utf8_lossy(&output.stdout)))
128}
129
130/// Pure: parse `git diff --unified=0` output into the new-side lines each file
131/// gained. Tracks the current file from each `+++` header and the new-side line
132/// counter from each `@@ … +c,d @@` hunk header, then records every following `+`
133/// line (a deletion `-` consumes no new-side number). A deleted file
134/// (`+++ /dev/null`) yields no entry.
135fn parse_unified_diff(diff: &str) -> BTreeMap<String, BTreeSet<u64>> {
136    let mut changed: BTreeMap<String, BTreeSet<u64>> = BTreeMap::new();
137    let mut current: Option<String> = None;
138    let mut next_line: u64 = 0;
139    for line in diff.lines() {
140        if let Some(header) = line.strip_prefix("+++ ") {
141            current = new_side_path(header);
142        } else if line.starts_with("@@") {
143            if let Some(start) = hunk_new_start(line) {
144                next_line = start;
145            }
146        } else if line.starts_with('+') {
147            // An added new-side line — the `+++` header is handled above, so this
148            // is diff body. Record it against the current file and advance.
149            if let Some(file) = &current {
150                changed.entry(file.clone()).or_default().insert(next_line);
151            }
152            next_line += 1;
153        }
154        // `-` (deleted) and metadata lines consume no new-side line and are skipped.
155    }
156    changed
157}
158
159/// The `repo`-relative new-side path from a `+++` diff header, or `None` for a
160/// deletion (`+++ /dev/null`). Strips git's `b/` prefix and a trailing tab.
161fn new_side_path(header: &str) -> Option<String> {
162    let path = header
163        .split('\t')
164        .next()
165        .unwrap_or(header)
166        .trim_end_matches('\r');
167    if path == "/dev/null" {
168        return None;
169    }
170    let path = path.strip_prefix("b/").unwrap_or(path);
171    Some(path.replace('\\', "/"))
172}
173
174/// The new-side start line from a hunk header `@@ -a,b +c,d @@ …` — the `c`. With
175/// `--unified=0` the added lines that follow are numbered consecutively from it.
176fn hunk_new_start(header: &str) -> Option<u64> {
177    let plus = header.split_whitespace().find(|t| t.starts_with('+'))?;
178    let digits = plus.trim_start_matches('+');
179    digits.split(',').next().unwrap_or(digits).parse().ok()
180}
181
182/// Pure: every changed line the coverage report marks uncovered — a `missing_line`,
183/// or the source of a `missing_branch` (a branch out of the line the suite never
184/// took). A changed file absent from `files` was not measured (a test file, or a
185/// `coverage`-exempt file omitted from the run) and contributes nothing; a changed
186/// line that is neither missing nor a branch source (a comment or blank) has
187/// nothing to cover. `files` is keyed by `root`-relative path, as `changed` is.
188pub fn uncovered_changed_lines(
189    changed: &BTreeMap<String, BTreeSet<u64>>,
190    files: &BTreeMap<String, FileCoverage>,
191) -> Vec<Uncovered> {
192    let mut uncovered = Vec::new();
193    for (file, lines) in changed {
194        let Some(coverage) = files.get(file) else {
195            continue;
196        };
197        let missing: BTreeSet<u64> = coverage.missing_lines.iter().copied().collect();
198        // The source line of each branch never taken (the first of the
199        // `[src, dst]` pair; `dst` may be negative — an exit — but `src` is a real
200        // line, so a negative drops out via `try_from`).
201        let branch_sources: BTreeSet<u64> = coverage
202            .missing_branches
203            .iter()
204            .filter_map(|pair| pair.first().copied())
205            .filter_map(|src| u64::try_from(src).ok())
206            .collect();
207        for &line in lines {
208            if missing.contains(&line) || branch_sources.contains(&line) {
209                uncovered.push(Uncovered {
210                    file: file.clone(),
211                    line,
212                });
213            }
214        }
215    }
216    uncovered.sort();
217    uncovered
218}
219
220/// Pure: every changed line a TypeScript coverage report marks uncovered.
221/// `uncovered` is the per-file set of uncovered lines
222/// ([`crate::coverage::measure_patch_typescript`]) — statements the suite never
223/// ran and the source lines of branches a path of which it never took — keyed by
224/// `root`-relative path, as `changed` is. A changed file absent from `uncovered`
225/// was not measured (a test file, a declaration file, or a `coverage`-exempt file
226/// excluded from the run) and contributes nothing; a changed line not in its set
227/// (a comment or blank) has nothing to cover.
228///
229/// The TypeScript counterpart to [`uncovered_changed_lines`]: where coverage.py
230/// splits missing lines from missing branches, vitest's report is reduced to one
231/// uncovered-line set per file upstream, so this is the plain intersection.
232pub fn uncovered_changed_lines_ts(
233    changed: &BTreeMap<String, BTreeSet<u64>>,
234    uncovered: &BTreeMap<String, BTreeSet<u64>>,
235) -> Vec<Uncovered> {
236    let mut out = Vec::new();
237    for (file, lines) in changed {
238        let Some(uncovered_lines) = uncovered.get(file) else {
239            continue;
240        };
241        for &line in lines {
242            if uncovered_lines.contains(&line) {
243                out.push(Uncovered {
244                    file: file.clone(),
245                    line,
246                });
247            }
248        }
249    }
250    out.sort();
251    out
252}
253
254/// Re-key a report's per-file map to `root`-relative `/`-joined paths so they match
255/// the diff's paths. coverage.py reports paths relative to where it ran (here
256/// `root`) and vitest reports absolute paths; an absolute path is stripped to
257/// `root`, a relative one left as-is.
258fn relative_keys<V>(files: BTreeMap<String, V>, root: &Path) -> BTreeMap<String, V> {
259    files
260        .into_iter()
261        .map(|(key, value)| {
262            let path = Path::new(&key);
263            let rel = path
264                .strip_prefix(root)
265                .unwrap_or(path)
266                .to_string_lossy()
267                .replace('\\', "/");
268            (rel, value)
269        })
270        .collect()
271}
272
273#[cfg(test)]
274mod tests {
275    use super::*;
276
277    fn changed(entries: &[(&str, &[u64])]) -> BTreeMap<String, BTreeSet<u64>> {
278        entries
279            .iter()
280            .map(|(path, lines)| (path.to_string(), lines.iter().copied().collect()))
281            .collect()
282    }
283
284    fn file_coverage(missing_lines: &[u64], missing_branches: &[[i64; 2]]) -> FileCoverage {
285        FileCoverage {
286            executed_lines: Vec::new(),
287            missing_lines: missing_lines.to_vec(),
288            excluded_lines: Vec::new(),
289            missing_branches: missing_branches.iter().map(|b| b.to_vec()).collect(),
290        }
291    }
292
293    // ---- parse_unified_diff --------------------------------------------------
294
295    #[test]
296    fn parses_added_lines_from_a_hunk() {
297        // `+4,2` → two added lines numbered from 4; the function context after the
298        // second `@@` is ignored.
299        let diff = "diff --git a/widget.py b/widget.py\n\
300                    index abc..def 100644\n\
301                    --- a/widget.py\n\
302                    +++ b/widget.py\n\
303                    @@ -3,0 +4,2 @@ def f(x):\n\
304                    +    if x == 99:\n\
305                    +        return 7\n";
306        assert_eq!(parse_unified_diff(diff), changed(&[("widget.py", &[4, 5])]));
307    }
308
309    #[test]
310    fn parses_a_new_file_as_added_from_line_one() {
311        let diff = "diff --git a/lonely.py b/lonely.py\n\
312                    new file mode 100644\n\
313                    index 0000000..bbb\n\
314                    --- /dev/null\n\
315                    +++ b/lonely.py\n\
316                    @@ -0,0 +1,2 @@\n\
317                    +def lonely():\n\
318                    +    return 41\n";
319        assert_eq!(parse_unified_diff(diff), changed(&[("lonely.py", &[1, 2])]));
320    }
321
322    #[test]
323    fn a_deletion_only_hunk_records_no_added_lines() {
324        // `+3,0` adds nothing; the `-` lines consume no new-side number.
325        let diff = "diff --git a/widget.py b/widget.py\n\
326                    index abc..def 100644\n\
327                    --- a/widget.py\n\
328                    +++ b/widget.py\n\
329                    @@ -4,2 +3,0 @@ def f(x):\n\
330                    -    dead = 1\n\
331                    -    return dead\n";
332        assert!(parse_unified_diff(diff).is_empty());
333    }
334
335    #[test]
336    fn a_deleted_file_yields_no_entry() {
337        let diff = "diff --git a/gone.py b/gone.py\n\
338                    deleted file mode 100644\n\
339                    index abc..0000000\n\
340                    --- a/gone.py\n\
341                    +++ /dev/null\n\
342                    @@ -1,2 +0,0 @@\n\
343                    -def gone():\n\
344                    -    return 0\n";
345        assert!(parse_unified_diff(diff).is_empty());
346    }
347
348    #[test]
349    fn parses_multiple_files_and_a_single_line_hunk() {
350        // `+2` (no count) is one line at line 2; a nested path is kept verbatim.
351        let diff = "diff --git a/a.py b/a.py\n\
352                    --- a/a.py\n\
353                    +++ b/a.py\n\
354                    @@ -1,0 +2 @@ def a():\n\
355                    +    x = 1\n\
356                    diff --git a/pkg/b.py b/pkg/b.py\n\
357                    --- a/pkg/b.py\n\
358                    +++ b/pkg/b.py\n\
359                    @@ -10,0 +11,1 @@\n\
360                    +    y = 2\n";
361        assert_eq!(
362            parse_unified_diff(diff),
363            changed(&[("a.py", &[2]), ("pkg/b.py", &[11])])
364        );
365    }
366
367    // ---- uncovered_changed_lines --------------------------------------------
368
369    #[test]
370    fn a_missing_changed_line_is_uncovered() {
371        let out = uncovered_changed_lines(
372            &changed(&[("widget.py", &[5])]),
373            &BTreeMap::from([("widget.py".to_string(), file_coverage(&[5], &[]))]),
374        );
375        assert_eq!(
376            out,
377            vec![Uncovered {
378                file: "widget.py".to_string(),
379                line: 5
380            }]
381        );
382    }
383
384    #[test]
385    fn a_covered_changed_line_is_not_reported() {
386        // Line 3 changed but it's neither missing nor a branch source → covered.
387        let out = uncovered_changed_lines(
388            &changed(&[("widget.py", &[3])]),
389            &BTreeMap::from([("widget.py".to_string(), file_coverage(&[5], &[[4, 5]]))]),
390        );
391        assert!(out.is_empty());
392    }
393
394    #[test]
395    fn a_changed_branch_source_is_uncovered() {
396        // Line 4 is executed (not a missing line) but a branch out of it was never
397        // taken (`[4, 5]`), so a change to line 4 is still uncovered.
398        let out = uncovered_changed_lines(
399            &changed(&[("widget.py", &[4])]),
400            &BTreeMap::from([("widget.py".to_string(), file_coverage(&[5], &[[4, 5]]))]),
401        );
402        assert_eq!(
403            out,
404            vec![Uncovered {
405                file: "widget.py".to_string(),
406                line: 4
407            }]
408        );
409    }
410
411    #[test]
412    fn a_negative_branch_dest_is_ignored() {
413        // `[6, -1]` is a branch to a function exit; the source line 6 is what
414        // matters, and a change to line 6 is uncovered.
415        let out = uncovered_changed_lines(
416            &changed(&[("widget.py", &[6])]),
417            &BTreeMap::from([("widget.py".to_string(), file_coverage(&[], &[[6, -1]]))]),
418        );
419        assert_eq!(
420            out,
421            vec![Uncovered {
422                file: "widget.py".to_string(),
423                line: 6
424            }]
425        );
426    }
427
428    #[test]
429    fn a_changed_file_absent_from_coverage_is_skipped() {
430        // A test file (omitted from the run) never appears in the report, so its
431        // changed lines contribute nothing rather than panicking on a lookup.
432        let out = uncovered_changed_lines(
433            &changed(&[("widget_test.py", &[1, 2])]),
434            &BTreeMap::from([("widget.py".to_string(), file_coverage(&[5], &[]))]),
435        );
436        assert!(out.is_empty());
437    }
438
439    #[test]
440    fn reports_are_sorted_across_files_and_lines() {
441        let out = uncovered_changed_lines(
442            &changed(&[("z.py", &[2, 1]), ("a.py", &[9])]),
443            &BTreeMap::from([
444                ("z.py".to_string(), file_coverage(&[1, 2], &[])),
445                ("a.py".to_string(), file_coverage(&[9], &[])),
446            ]),
447        );
448        assert_eq!(
449            out,
450            vec![
451                Uncovered {
452                    file: "a.py".to_string(),
453                    line: 9
454                },
455                Uncovered {
456                    file: "z.py".to_string(),
457                    line: 1
458                },
459                Uncovered {
460                    file: "z.py".to_string(),
461                    line: 2
462                },
463            ]
464        );
465    }
466
467    // ---- uncovered_changed_lines_ts (TypeScript, #135) -----------------------
468
469    #[test]
470    fn ts_a_changed_uncovered_line_is_reported() {
471        // Line 4 changed and the vitest report marks it uncovered → reported.
472        let out = uncovered_changed_lines_ts(
473            &changed(&[("widget.ts", &[4])]),
474            &changed(&[("widget.ts", &[3, 4, 5])]),
475        );
476        assert_eq!(
477            out,
478            vec![Uncovered {
479                file: "widget.ts".to_string(),
480                line: 4
481            }]
482        );
483    }
484
485    #[test]
486    fn ts_a_covered_changed_line_is_not_reported() {
487        // Line 2 changed but it isn't in the uncovered set → covered, not reported.
488        let out = uncovered_changed_lines_ts(
489            &changed(&[("widget.ts", &[2])]),
490            &changed(&[("widget.ts", &[3, 4, 5])]),
491        );
492        assert!(out.is_empty());
493    }
494
495    #[test]
496    fn ts_a_changed_file_absent_from_coverage_is_skipped() {
497        // A test file never appears in the report (it's excluded from the run), so
498        // its changed lines contribute nothing rather than panicking on a lookup.
499        let out = uncovered_changed_lines_ts(
500            &changed(&[("widget.test.ts", &[1, 2])]),
501            &changed(&[("widget.ts", &[5])]),
502        );
503        assert!(out.is_empty());
504    }
505
506    #[test]
507    fn ts_reports_are_sorted_across_files_and_lines() {
508        let out = uncovered_changed_lines_ts(
509            &changed(&[("z.ts", &[2, 1]), ("a.ts", &[9])]),
510            &changed(&[("z.ts", &[1, 2]), ("a.ts", &[9])]),
511        );
512        assert_eq!(
513            out,
514            vec![
515                Uncovered {
516                    file: "a.ts".to_string(),
517                    line: 9
518                },
519                Uncovered {
520                    file: "z.ts".to_string(),
521                    line: 1
522                },
523                Uncovered {
524                    file: "z.ts".to_string(),
525                    line: 2
526                },
527            ]
528        );
529    }
530}