Skip to main content

testing_conventions/
patch_coverage.rs

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