From 78b7e313a4f3cc9bab504ca5d643c0ce3d9ce7a9 Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Tue, 13 Aug 2024 08:43:31 -0700 Subject: [PATCH 1/2] fix: Don't error on diffs mentioning binary files The `patch` crate doesn't understand them, so just filter them out. Fixes #391 --- NEWS.md | 2 ++ src/in_diff.rs | 14 ++++++++++++-- tests/in_diff.rs | 18 ++++++++++++++++++ 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 34da220f..e60d40c5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,8 @@ - New: cargo-mutants starts a GNU jobserver, shared across all children, so that running multiple `--jobs` does not spawn an excessive number of compiler processes. The jobserver is on by default and can be turned off with `--jobserver false`. +- Fixed: Don't error on diffs containing a "Binary files differ" message. + ## 24.7.1 - Changed: No build timeouts by default. Previously, cargo-mutants set a default build timeout based on the baseline build, but experience showed that this would sometimes make builds flaky, because build times can be quite variable. If mutants cause builds to hang, then you can still set a timeout using `--build-timeout` or `--build-timeout-multiplier`. diff --git a/src/in_diff.rs b/src/in_diff.rs index 755d32db..6972549c 100644 --- a/src/in_diff.rs +++ b/src/in_diff.rs @@ -4,6 +4,7 @@ //! for example from uncommitted or unmerged changes. use std::collections::HashMap; +use std::iter::once; use anyhow::{anyhow, bail}; use camino::Utf8Path; @@ -18,13 +19,22 @@ use crate::Result; /// Return only mutants to functions whose source was touched by this diff. pub fn diff_filter(mutants: Vec, diff_text: &str) -> Result> { + // Strip any "Binary files .. differ" lines because `patch` doesn't understand them at + // the moment; this could be removed if it's fixed in that crate. + let fixed_diff = diff_text + .lines() + .filter(|line| !(line.starts_with("Binary files ") && line.ends_with("differ"))) + .chain(once("")) + .join("\n"); + // Flatten the error to a string because otherwise it references the diff, and can't be returned. - if diff_text.trim().is_empty() { + if fixed_diff.trim().is_empty() { info!("diff file is empty; no mutants will match"); return Ok(Vec::new()); } + let patches = - Patch::from_multiple(diff_text).map_err(|err| anyhow!("Failed to parse diff: {err}"))?; + Patch::from_multiple(&fixed_diff).map_err(|err| anyhow!("Failed to parse diff: {err}"))?; check_diff_new_text_matches(&patches, &mutants)?; let mut lines_changed_by_path: HashMap<&Utf8Path, Vec> = HashMap::new(); for patch in &patches { diff --git a/tests/in_diff.rs b/tests/in_diff.rs index 81a0eb51..0bb85cff 100644 --- a/tests/in_diff.rs +++ b/tests/in_diff.rs @@ -69,6 +69,24 @@ fn list_mutants_changed_in_diff1() { ); } +#[test] +fn binary_diff_is_not_an_error_and_matches_nothing() { + // From https://github.com/sourcefrog/cargo-mutants/issues/391 + let mut diff_file = NamedTempFile::new().unwrap(); + diff_file.write_all(b"Binary files a/test-renderers/expected/renderers/fog-None-wgpu.png and b/test-renderers/expected/renderers/fog-None-wgpu.png differ\n").unwrap(); + let tmp = copy_of_testdata("diff1"); + run() + .args(["mutants", "--no-shuffle", "-d"]) + .arg(tmp.path()) + .arg("--in-diff") + .arg(diff_file.path()) + .arg("--list") + .assert() + .success() + .stdout("") + .stderr(predicate::str::contains("diff file is empty")); +} + #[test] fn empty_diff_is_not_an_error_and_matches_nothing() { let diff_file = NamedTempFile::new().unwrap(); From 24874b1c414a0f6d1c5d92ed6553fe90ffe73f6f Mon Sep 17 00:00:00 2001 From: Martin Pool Date: Sun, 18 Aug 2024 10:41:58 -0700 Subject: [PATCH 2/2] mutants: simplify condition to avoid missed mutant --- src/in_diff.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/in_diff.rs b/src/in_diff.rs index 6972549c..488d1f90 100644 --- a/src/in_diff.rs +++ b/src/in_diff.rs @@ -23,7 +23,7 @@ pub fn diff_filter(mutants: Vec, diff_text: &str) -> Result> // the moment; this could be removed if it's fixed in that crate. let fixed_diff = diff_text .lines() - .filter(|line| !(line.starts_with("Binary files ") && line.ends_with("differ"))) + .filter(|line| !(line.starts_with("Binary files "))) .chain(once("")) .join("\n");