diff --git a/NEWS.md b/NEWS.md index 646847c4..90497d38 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,7 @@ ## Unreleased +- New: `--in-git-diff REVISION` runs `git diff` and tests only mutants in code changed since that revision ([#245](https://github.com/sourcefrog/cargo-mutants/issues/245)). - New: `#[mutants::exclude_re("pattern")]` attribute to exclude specific mutations by regex, without disabling all mutations on the function. The attribute can be placed on functions, `impl` blocks, `trait` blocks, modules, files, and on expressions that can carry an attribute (such as `match`, struct literals, call expressions, method calls, and unary expressions). Multiple patterns can be applied. Also supported within `cfg_attr`. Requires the [mutants](https://crates.io/crates/mutants) crate version `0.0.5` or later. - Fixed: `#[mutants::skip]` (and `#[cfg_attr(..., mutants::skip)]`) is now honoured when placed on `const` and `static` items, including associated constants in `impl` and `trait` blocks. Previously the attribute was silently ignored on these items and operator mutants inside the initializer expression were still generated ([#508](https://github.com/sourcefrog/cargo-mutants/issues/508)). diff --git a/book/src/in-diff.md b/book/src/in-diff.md index 15b8d444..3a9a0ca1 100644 --- a/book/src/in-diff.md +++ b/book/src/in-diff.md @@ -4,12 +4,16 @@ If you're working on a large project or one with a long test suite, you may not The `--in-diff DIFF_FILE` option tests only mutants that overlap with regions changed in the diff. -The diff is expected to either have a prefix of `b/` on the new filename, which is the format produced by `git diff`, or no prefix. +The `--in-git-diff REVISION` option runs `git diff REVISION` in the workspace and tests mutants that overlap with that output. For example, `cargo mutants --in-git-diff main` tests code changed on the current branch since `main`. -Some ways you could use `--in-diff`: +`--in-diff` and `--in-git-diff` cannot be used together. -1. Before submitting code, check your uncommitted changes with `git diff`. -2. In CI, or locally, check the diff between the current branch and the base branch of the pull request. +A diff passed with `--in-diff` is expected to either have a prefix of `b/` on the new filename, which is the format produced by `git diff`, or no prefix. + +Some ways you can filter by a diff: + +1. Before submitting code, write `git diff` to a file and pass it with `--in-diff`. +2. In CI, or locally, pass the pull request's base branch to `--in-git-diff`. Changes to non-Rust files, or files from which no mutants are produced, are ignored. diff --git a/src/in_diff.rs b/src/in_diff.rs index e68b31f2..a191a0de 100644 --- a/src/in_diff.rs +++ b/src/in_diff.rs @@ -9,6 +9,7 @@ use std::collections::HashMap; use std::fmt::Display; use std::fs::File; use std::io::Read; +use std::process::Command; use anyhow::bail; use camino::Utf8Path; @@ -37,6 +38,8 @@ pub enum DiffFilterError { InvalidDiff(String), /// Can't open or read the diff file. File(String), + /// Can't run git or git rejects the revision. + Git(String), } impl DiffFilterError { @@ -50,9 +53,9 @@ impl DiffFilterError { | DiffFilterError::NoSourceFiles | DiffFilterError::NoMutants => ExitCode::Success, DiffFilterError::MismatchedDiff(_) => ExitCode::FilterDiffMismatch, - DiffFilterError::File(_) | DiffFilterError::InvalidDiff(_) => { - ExitCode::FilterDiffInvalid - } + DiffFilterError::File(_) + | DiffFilterError::Git(_) + | DiffFilterError::InvalidDiff(_) => ExitCode::FilterDiffInvalid, } } } @@ -66,10 +69,45 @@ impl Display for DiffFilterError { DiffFilterError::MismatchedDiff(msg) => write!(f, "{msg}"), DiffFilterError::InvalidDiff(msg) => write!(f, "Failed to parse diff: {msg}"), DiffFilterError::File(msg) => write!(f, "Failed to read diff file: {msg}"), + DiffFilterError::Git(msg) => write!(f, "Failed to run git diff: {msg}"), } } } +/// Run `git diff` and filter mutants to those changed since `revision`. +pub fn diff_filter_git( + mutants: Vec, + workspace_root: &Utf8Path, + revision: &str, +) -> Result, DiffFilterError> { + let output = Command::new("git") + .args([ + "diff", + "--no-color", + "--no-ext-diff", + "--no-textconv", + "--relative", + "--src-prefix=a/", + "--dst-prefix=b/", + revision, + "--", + ]) + .current_dir(workspace_root) + .output() + .map_err(|err| DiffFilterError::Git(err.to_string()))?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + let message = stderr.trim(); + return Err(DiffFilterError::Git(if message.is_empty() { + format!("git exited with {}", output.status) + } else { + message.to_owned() + })); + } + let diff_text = String::from_utf8_lossy(&output.stdout); + diff_filter(mutants, &diff_text) +} + pub fn diff_filter_file( mutants: Vec, diff_path: &Utf8Path, diff --git a/src/main.rs b/src/main.rs index 42cdfd61..04865663 100644 --- a/src/main.rs +++ b/src/main.rs @@ -65,7 +65,7 @@ use crate::{ build_dir::BuildDir, console::Console, exit_code::ExitCode, - in_diff::diff_filter_file, + in_diff::{diff_filter_file, diff_filter_git}, interrupt::check_interrupted, lab::test_mutants, list::{list_files, list_mutants}, @@ -220,6 +220,7 @@ pub struct Args { value_name = "FILE", long = "Zmutate-file", conflicts_with = "in_diff", + conflicts_with = "in_git_diff", conflicts_with = "package" )] mutate_file: Option, @@ -364,6 +365,15 @@ pub struct Args { #[arg(long, short = 'D', help_heading = "Filters")] in_diff: Option, + /// Include only mutants in code changed since this Git revision. + #[arg( + long, + value_name = "REVISION", + conflicts_with = "in_diff", + help_heading = "Filters" + )] + in_git_diff: Option, + /// Skip mutants that were caught in previous runs. #[arg(long, help_heading = "Filters")] iterate: bool, @@ -593,19 +603,24 @@ fn main() -> Result { return Ok(ExitCode::Success); } let mut mutants = discovered.mutants; - if let Some(diff_path) = &args.in_diff { - mutants = match diff_filter_file(mutants, diff_path) { - Ok(mutants) => mutants, - Err(err) => { - if err.exit_code() == ExitCode::Success { - info!("{err}"); - } else { - error!("{err}"); - } - return Ok(err.exit_code()); + let filtered = if let Some(diff_path) = &args.in_diff { + diff_filter_file(mutants, diff_path) + } else if let Some(revision) = &args.in_git_diff { + diff_filter_git(mutants, workspace.root(), revision) + } else { + Ok(mutants) + }; + mutants = match filtered { + Ok(mutants) => mutants, + Err(err) => { + if err.exit_code() == ExitCode::Success { + info!("{err}"); + } else { + error!("{err}"); } - }; - } + return Ok(err.exit_code()); + } + }; if let Some(shard) = &args.shard { mutants = options.sharding().shard(*shard, mutants); } @@ -671,6 +686,16 @@ mod test { println!("Error message: {}", args.unwrap_err()); } + #[test] + fn in_diff_conflicts_with_in_git_diff() { + let args = super::Args::try_parse_from([ + "mutants", + "--in-diff=changes.patch", + "--in-git-diff=main", + ]); + assert!(args.is_err(), "Expected conflicting diff sources to fail"); + } + #[test] fn option_help_sentence_case_without_period() { let args = super::Args::command(); diff --git a/tests/main.rs b/tests/main.rs index 57cba97c..ce9ac701 100644 --- a/tests/main.rs +++ b/tests/main.rs @@ -13,6 +13,7 @@ use std::env; use std::fs::{self, File, create_dir, create_dir_all, read_dir, read_to_string, rename, write}; use std::io::Write as IoWrite; use std::path::Path; +use std::process::Command; use indoc::indoc; use insta::assert_snapshot; @@ -2028,6 +2029,77 @@ fn list_mutants_changed_in_diff1() { ); } +#[test] +fn list_mutants_changed_in_git_diff_from_nested_workspace() { + let git_root = tempdir().unwrap(); + let workspace = git_root.path().join("project"); + create_dir(&workspace).unwrap(); + copy_testdata_to("diff0", &workspace); + for args in [ + &["init", "--quiet"][..], + &["config", "user.name", "Test User"], + &["config", "user.email", "test@example.com"], + &["add", "."], + &["commit", "--quiet", "--message", "baseline"], + ] { + let output = Command::new("git") + .args(args) + .current_dir(git_root.path()) + .output() + .unwrap(); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + fs::copy("testdata/diff1/src/lib.rs", workspace.join("src/lib.rs")).unwrap(); + + let output = run() + .args(["mutants", "--list", "--json", "--no-shuffle", "-d"]) + .arg(&workspace) + .args(["--in-git-diff", "HEAD"]) + .assert() + .success() + .get_output() + .stdout + .clone(); + let mutants: serde_json::Value = serde_json::from_slice(&output).unwrap(); + let names = mutants + .as_array() + .unwrap() + .iter() + .map(|mutant| mutant["name"].as_str().unwrap()) + .collect_vec(); + + assert_eq!( + names, + [ + "src/lib.rs:6:5: replace two -> String with String::new()", + "src/lib.rs:6:5: replace two -> String with \"xyzzy\".into()", + ] + ); +} + +#[test] +fn invalid_in_git_diff_revision_returns_exit_code_6() { + let tmp = copy_of_testdata("diff0"); + let output = Command::new("git") + .args(["init", "--quiet"]) + .current_dir(tmp.path()) + .output() + .unwrap(); + assert!(output.status.success()); + + run() + .args(["mutants", "--list", "-d"]) + .arg(tmp.path()) + .args(["--in-git-diff", "not-a-revision"]) + .assert() + .code(6) + .stderr(contains("Failed to run git diff").and(contains("not-a-revision"))); +} + #[test] fn binary_diff_is_not_an_error_and_matches_nothing() { // From https://github.com/sourcefrog/cargo-mutants/issues/391