tests: validate "patch" and "ed" commands once, print meaningful messages (#226)

macOS' /usr/bin/patch and GNU patch have very subtle incompatibilities
that cause only some "more advanced" tests to fail in obscure and very
time-consuming ways - while other tests pass. In some cases (depending
on test threads racing), the lack of newlines in some test data even
causes the whole test suite to stall.

This fix runs `patch -version` (only once), makes sure the output starts
with "GNU patch" and shows a meaningful assert message when not. It also
looks for `gpatch` instead of `patch` on macOS and shows a meaningful
assert message if either is missing.

Fixes: #225

This also provides faster and better feedback when `ed` is missing (see
#39) and implements a portable and basic check.

Last but not least, this new code is generic enough to support the
validation of any other test dependency in the future.
This commit is contained in:
Marc Herbert
2026-05-24 11:08:52 -04:00
committed by GitHub
parent c1943c5abb
commit 1254f146f8
5 changed files with 137 additions and 32 deletions
+11 -8
View File
@@ -381,6 +381,9 @@ pub fn diff(expected: &[u8], actual: &[u8], params: &Params) -> Vec<u8> {
mod tests {
use super::*;
use pretty_assertions::assert_eq;
use crate::utils::testcmds::PATCH_CMD;
#[test]
fn test_permutations() {
// test all possible six-line files.
@@ -394,7 +397,6 @@ mod tests {
for &f in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"a\n" } else { b"b\n" })
@@ -450,7 +452,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.arg("--context")
.stdin(File::open(format!("{target}/ab.diff")).unwrap())
@@ -482,7 +485,6 @@ mod tests {
for &f in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"\n" } else { b"b\n" }).unwrap();
@@ -532,7 +534,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.arg("--context")
.stdin(File::open(format!("{target}/ab_.diff")).unwrap())
@@ -564,7 +567,6 @@ mod tests {
for &f in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"a\n" } else { b"" }).unwrap();
@@ -617,7 +619,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.arg("--context")
.stdin(File::open(format!("{target}/abx.diff")).unwrap())
@@ -649,7 +652,6 @@ mod tests {
for &f in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"a\n" } else { b"f\n" })
@@ -705,7 +707,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.arg("--context")
.stdin(File::open(format!("{target}/abr.diff")).unwrap())
+9 -6
View File
@@ -162,6 +162,9 @@ pub fn diff(expected: &[u8], actual: &[u8], params: &Params) -> Result<Vec<u8>,
mod tests {
use super::*;
use pretty_assertions::assert_eq;
use crate::utils::testcmds::ED_CMD;
pub fn diff_w(expected: &[u8], actual: &[u8], filename: &str) -> Result<Vec<u8>, DiffError> {
let mut output = diff(expected, actual, &Params::default())?;
writeln!(&mut output, "w {filename}").unwrap();
@@ -237,8 +240,8 @@ mod tests {
let _ = fb;
#[cfg(not(windows))] // there's no ed on windows
{
use std::process::Command;
let output = Command::new("ed")
let output = ED_CMD
.new()
.arg(format!("{target}/alef"))
.stdin(File::open(format!("{target}/ab.ed")).unwrap())
.output()
@@ -311,8 +314,8 @@ mod tests {
let _ = fb;
#[cfg(not(windows))] // there's no ed on windows
{
use std::process::Command;
let output = Command::new("ed")
let output = ED_CMD
.new()
.arg(format!("{target}/alef_"))
.stdin(File::open(format!("{target}/ab_.ed")).unwrap())
.output()
@@ -391,8 +394,8 @@ mod tests {
let _ = fb;
#[cfg(not(windows))] // there's no ed on windows
{
use std::process::Command;
let output = Command::new("ed")
let output = ED_CMD
.new()
.arg(format!("{target}/alefr"))
.stdin(File::open(format!("{target}/abr.ed")).unwrap())
.output()
+10 -8
View File
@@ -215,6 +215,8 @@ mod tests {
use super::*;
use pretty_assertions::assert_eq;
use crate::utils::testcmds::PATCH_CMD;
#[test]
fn test_basic() {
let mut a = Vec::new();
@@ -239,7 +241,6 @@ mod tests {
for &f in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"a\n" } else { b"b\n" })
@@ -285,7 +286,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.arg(format!("{target}/alef"))
.stdin(File::open(format!("{target}/ab.diff")).unwrap())
@@ -318,7 +320,6 @@ mod tests {
for &g in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"a\n" } else { b"b\n" })
@@ -377,7 +378,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.arg("--normal")
.arg(format!("{target}/alefn"))
@@ -411,7 +413,6 @@ mod tests {
for &f in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"\n" } else { b"b\n" }).unwrap();
@@ -451,7 +452,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.arg(format!("{target}/alef_"))
.stdin(File::open(format!("{target}/ab_.diff")).unwrap())
@@ -483,7 +485,6 @@ mod tests {
for &f in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"a\n" } else { b"f\n" })
@@ -529,7 +530,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.arg(format!("{target}/alefr"))
.stdin(File::open(format!("{target}/abr.diff")).unwrap())
+14 -10
View File
@@ -408,6 +408,8 @@ mod tests {
use super::*;
use pretty_assertions::assert_eq;
use crate::utils::testcmds::PATCH_CMD;
#[test]
fn test_permutations() {
let target = "target/unified-diff/";
@@ -421,7 +423,6 @@ mod tests {
for &f in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"a\n" } else { b"b\n" })
@@ -493,7 +494,10 @@ mod tests {
.unwrap_or_else(|_| String::from("[Invalid UTF-8]"))
);
let output = Command::new("patch")
use crate::utils::testcmds::PATCH_CMD;
let output = PATCH_CMD
.new()
.arg("-p0")
.stdin(File::open(format!("{target}/ab.diff")).unwrap())
.output()
@@ -525,7 +529,6 @@ mod tests {
for &g in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"a\n" } else { b"b\n" })
@@ -594,7 +597,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.stdin(File::open(format!("{target}/abn.diff")).unwrap())
.output()
@@ -627,7 +631,6 @@ mod tests {
for &g in &[0, 1, 2, 3] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"\n" } else { b"b\n" }).unwrap();
@@ -691,7 +694,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.stdin(File::open(format!("{target}/ab_.diff")).unwrap())
.output()
@@ -723,7 +727,6 @@ mod tests {
for &f in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"a\n" } else { b"" }).unwrap();
@@ -773,7 +776,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.stdin(File::open(format!("{target}/abx.diff")).unwrap())
.output()
@@ -804,7 +808,6 @@ mod tests {
for &f in &[0, 1, 2] {
use std::fs::{self, File};
use std::io::Write;
use std::process::Command;
let mut alef = Vec::new();
let mut bet = Vec::new();
alef.write_all(if a == 0 { b"a\n" } else { b"f\n" })
@@ -860,7 +863,8 @@ mod tests {
fb.write_all(&bet[..]).unwrap();
let _ = fa;
let _ = fb;
let output = Command::new("patch")
let output = PATCH_CMD
.new()
.arg("-p0")
.stdin(File::open(format!("{target}/abr.diff")).unwrap())
.output()
+93
View File
@@ -98,6 +98,99 @@ pub fn report_failure_to_read_input_file(
);
}
#[cfg(test)]
pub mod testcmds {
// Command construction wrapper that provides some validation and non-obscure, "fail fast"
// feedback and error messages.
use std::any::Any;
use std::io::Write;
use std::panic::catch_unwind;
use std::process::{Command, Stdio};
use std::sync::LazyLock;
pub struct CmdFactory {
cmd: &'static str,
validated_once: LazyLock<Result<(), String>>,
validate: fn(&CmdFactory) -> (),
}
impl CmdFactory {
pub fn new(&self) -> Command {
match &*self.validated_once {
Ok(()) => Command::new(self.cmd),
Err(errmsg) => panic!(
"'{}' validation failed in earlier thread/test: {}",
self.cmd, errmsg
),
}
}
// "self" is not compatible with static initialization
fn try_catch_validate(cmd: &CmdFactory) -> Result<(), String> {
// Note catch_unwind() does _not_ hide error messages, stack traces, etc.
catch_unwind(|| {
let _ = (cmd.validate)(cmd);
})
.map_err(find_panic_message)
}
}
fn find_panic_message(payload: Box<dyn Any + Send>) -> String {
// https://github.com/rust-lang/rust/blob/1.95.0/library/std/src/panicking.rs#L771
if let Some(&s) = payload.downcast_ref::<&'static str>() {
String::from(s)
} else if let Some(s) = payload.downcast_ref::<String>() {
s.clone()
} else {
format!(
"Unusual panic payload type {:?}, look for the first thread/test that failed",
payload.type_id(),
)
}
}
pub static PATCH_CMD: CmdFactory = CmdFactory {
cmd: if cfg!(target_os = "macos") {
"gpatch" // brew install patch
} else {
"patch"
},
validated_once: LazyLock::new(|| CmdFactory::try_catch_validate(&PATCH_CMD)),
validate: (|myself| {
let output = Command::new(myself.cmd)
.arg("--version")
.output()
.expect("`patch --version` failed");
// Non-GNU versions have subtle differences. When some newlines are missing in some test
// patches, the macOS version can even stall the whole test run.
assert!(output.stdout.starts_with(b"GNU patch"));
assert!(output.status.success());
}),
};
pub static ED_CMD: CmdFactory = CmdFactory {
cmd: "ed",
validated_once: LazyLock::new(|| CmdFactory::try_catch_validate(&ED_CMD)),
validate: (|myself| {
let mut child = Command::new(myself.cmd)
.arg("!echo hello_ed")
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.spawn()
.expect("Failed to start 'ed' command");
let mut stdin = child.stdin.take().unwrap();
writeln!(stdin, "1p\nq").expect("Failed to send command to 'ed'");
let output = child
.wait_with_output()
.expect("Failed to read 'ed' stdout");
assert_eq!(String::from_utf8_lossy(&output.stdout), "9\nhello_ed\n");
}),
};
}
#[cfg(test)]
mod tests {
use super::*;