Repository: sourcefrog/cargo-mutants (rust)

Issue:
`--in-diff` fails on diffs mentioning binary files
Suppose that `test.diff` contains git diff output such as:

```diff
diff --git a/test-renderers/expected/renderers/fog-None-wgpu.png b/test-renderers/expected/renderers/fog-None-wgpu.png
index 616d6ea8..afd1b043 100644
Binary files a/test-renderers/expected/renderers/fog-None-wgpu.png and b/test-renderers/expected/renderers/fog-None-wgpu.png differ
```

Then `cargo mutants --in-diff test.diff` will fail:

```
Error: Failed to parse diff: Line 1: Error while parsing: diff --git a/test-renderers/expected/renderers/fog-None-wgpu.png b/test-renderers/expected/renderers/fog-None-wgpu.png
index 616d6ea8..afd1b043 100644
Binary files a/test-renderers/expected/renderers/fog-None-wgpu.png and b/test-renderers/expected/renderers/fog-None-wgpu.png differ
```

If the diff contains files preceding this one, the outcome is similar, but as a panic:

```
thread 'main' panicked at /Users/kpreid/.cargo/registry/src/index.crates.io-6f17d22bba15001f/patch-0.7.0/src/parser.rs:84:5:
bug: failed to parse entire input. Remaining: 'diff --git a/test-renderers/expected/renderers/fog-None-wgpu.png b/test-renderers/expected/renderers/fog-None-wgpu.png
index 616d6ea8..afd1b043 100644
Binary files a/test-renderers/expected/renderers/fog-None-wgpu.png and b/test-renderers/expected/renderers/fog-None-wgpu.png differ
'
```

Instead, this diff should successfully parse (and, since binary files are not Rust source code that can be mutated, it should be skipped).

Pull request:
fix: Don't error on diffs mentioning binary files
The `patch` crate doesn't understand them, so just filter them out.

Fixes #391

Patch:
diff --git a/NEWS.md b/NEWS.md
index 34da220..e60d40c 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 755d32d..488d1f9 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<Mutant>, diff_text: &str) -> Result<Vec<Mutant>> {
+    // 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 ")))
+        .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<usize>> = HashMap::new();
     for patch in &patches {


Test patch:
diff --git a/tests/in_diff.rs b/tests/in_diff.rs
index 81a0eb5..0bb85cf 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();