Skip to content

Commit

Permalink
fix: Don't error on diffs mentioning binary files (#393)
Browse files Browse the repository at this point in the history
The `patch` crate doesn't understand them, so just filter them out.

Fixes #391
  • Loading branch information
sourcefrog authored Aug 18, 2024
2 parents 8ff4c5a + 24874b1 commit 41eab78
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 2 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
14 changes: 12 additions & 2 deletions src/in_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down
18 changes: 18 additions & 0 deletions tests/in_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

0 comments on commit 41eab78

Please sign in to comment.