Merge pull request #181 from mingnus/2021-06-23-functional-tests

Functional tests revised

base/file_utils.cc

@@ -71,11 +71,18 @@ void
 file_utils::check_file_exists(string const &file, bool must_be_regular_file) {
 	struct stat info;
 	int r = ::stat(file.c_str(), &info);
-	if (r)
+	if (r) {
-		throw runtime_error("Couldn't stat file");
+		ostringstream msg;
+		msg << file << ": " << base::error_string(errno);
+		throw runtime_error(msg.str());
+	}
 
-	if (must_be_regular_file && !S_ISREG(info.st_mode))
+	if (!S_ISREG(info.st_mode)) {
+		if (must_be_regular_file)
 			throw runtime_error("Not a regular file");
+		if (!S_ISBLK(info.st_mode))
+			throw runtime_error("Not a block device or regular file");
+	}
 }
 
 file_utils::file_descriptor
@@ -116,8 +123,11 @@ file_utils::get_file_length(string const &file) {
 	uint64_t nr_bytes;
 
 	int r = ::stat(file.c_str(), &info);
-	if (r)
+	if (r) {
-		throw runtime_error("Couldn't stat path");
+		ostringstream msg;
+		msg << file << ": " << base::error_string(errno);
+		throw runtime_error(msg.str());
+	}
 
 	if (S_ISREG(info.st_mode))
 		// It's okay to cast st_size to a uint64_t value.
@@ -136,9 +146,11 @@ file_utils::get_file_length(string const &file) {
 			throw runtime_error("ioctl BLKGETSIZE64 failed");
 		}
 		::close(fd);
-	} else
+	} else {
-		// FIXME: needs a better message
+		ostringstream msg;
-		throw runtime_error("bad path");
+		msg << file << ": " << "Not a block device or regular file";
+		throw runtime_error(msg.str());
+	}
 
 	return nr_bytes;
 }

functional-tests/cache-functional-tests.scm

@@ -82,9 +82,12 @@
   (define-scenario (cache-restore missing-input-file)
     "the input file can't be found"
     (with-empty-metadata (md)
-      (run-fail-rcv (_ stderr) (cache-restore "-i no-such-file -o" md)
+      (let ((bad-path "no-such-file"))
+        (run-fail-rcv (_ stderr) (cache-restore "-i" bad-path "-o" md)
           (assert-superblock-all-zeroes md)
-        (assert-starts-with "Couldn't stat file" stderr))))
+          (assert-starts-with
+            (string-append bad-path ": No such file or directory")
+            stderr)))))
 
   (define-scenario (cache-restore garbage-input-file)
     "the input file is just zeroes"
@@ -264,9 +267,12 @@
   (define-scenario (cache-repair missing-input-file)
     "the input file can't be found"
     (with-empty-metadata (md)
+      (let ((bad-path "no-such-file"))
         (run-fail-rcv (_ stderr) (cache-repair "-i no-such-file -o" md)
           (assert-superblock-all-zeroes md)
-        (assert-starts-with "Couldn't stat path" stderr))))
+          (assert-starts-with
+            (string-append bad-path ": No such file or directory")
+            stderr)))))
 
   (define-scenario (cache-repair garbage-input-file)
     "the input file is just zeroes"

functional-tests/era-functional-tests.scm

@@ -152,9 +152,12 @@
   (define-scenario (era-restore missing-input-file)
     "the input file can't be found"
     (with-empty-metadata (md)
+      (let ((bad-path "no-such-file"))
         (run-fail-rcv (_ stderr) (era-restore "-i no-such-file -o" md)
           (assert-superblock-all-zeroes md)
-        (assert-starts-with "Couldn't stat file" stderr))))
+          (assert-starts-with
+            (string-append bad-path ": No such file or directory")
+            stderr)))))
 
   (define-scenario (era-restore garbage-input-file)
     "the input file is just zeroes"
@@ -197,7 +200,9 @@
       (with-empty-metadata (md)
         (run-fail-rcv (stdout stderr) (era-restore "--quiet" "-i" bad-xml "-o" md)
           (assert-eof stdout)
-          (assert-starts-with "Couldn't stat file" stderr)))))
+          (assert-starts-with
+            (string-append bad-xml ": No such file or directory")
+            stderr)))))
 
   (define-scenario (era-restore q-fail)
     "No output with --q(failing)"
@@ -205,7 +210,9 @@
       (with-empty-metadata (md)
         (run-fail-rcv (stdout stderr) (era-restore "-q" "-i" bad-xml "-o" md)
           (assert-eof stdout)
-          (assert-starts-with "Couldn't stat file" stderr)))))
+          (assert-starts-with
+            (string-append bad-xml ": No such file or directory")
+            stderr)))))
 
   ;;;-----------------------------------------------------------
   ;;; era_dump scenarios

src/bin/thin_explore.rs

@@ -183,8 +183,7 @@ impl<'a> StatefulWidget for SBWidget<'a> {
             format!("{}k", sb.data_block_size * 2),
         ];
 
-        let table = Table::new(
+        let table = Table::new(vec![
-            vec![
             Row::new(flags),
             Row::new(block),
             Row::new(uuid),
@@ -195,12 +194,8 @@ impl<'a> StatefulWidget for SBWidget<'a> {
             Row::new(mapping_root),
             Row::new(details_root),
             Row::new(data_block_size),
-            ]
+        ])
-        )
+        .header(Row::new(vec!["Field", "Value"]).style(Style::default().fg(Color::Yellow)))
-        .header(
-            Row::new(vec!["Field", "Value"])
-                .style(Style::default().fg(Color::Yellow))
-        )
         .block(
             Block::default()
                 .borders(Borders::ALL)
@@ -251,19 +246,14 @@ impl<'a> Widget for HeaderWidget<'a> {
         let max_entries = vec!["max_entries".to_string(), format!("{}", hdr.max_entries)];
         let value_size = vec!["value size".to_string(), format!("{}", hdr.value_size)];
 
-        let table = Table::new(
+        let table = Table::new(vec![
-            vec![
             Row::new(block),
             Row::new(kind),
             Row::new(nr_entries),
             Row::new(max_entries),
             Row::new(value_size),
-            ]
+        ])
-        )
+        .header(Row::new(vec!["Field", "Value"]).style(Style::default().fg(Color::Yellow)))
-        .header(
-            Row::new(vec!["Field", "Value"])
-                .style(Style::default().fg(Color::Yellow))
-        )
         .block(Block::default().borders(Borders::ALL).title(self.title))
         .widths(&[Constraint::Length(20), Constraint::Length(60)])
         .style(Style::default().fg(Color::White))

src/bin/thin_metadata_unpack.rs

@@ -39,7 +39,7 @@ fn main() {
     }
 
     if let Err(reason) = thinp::pack::toplevel::unpack(&input_file, &output_file) {
-        println!("Application error: {}", reason);
+        eprintln!("Application error: {}", reason);
         process::exit(1);
     }
 }

src/bin/thin_shrink.rs

@@ -18,6 +18,7 @@ fn main() {
             Arg::with_name("INPUT")
                 .help("Specify thinp metadata xml file")
                 .required(true)
+                .short("i")
                 .long("input")
                 .value_name("INPUT")
                 .takes_value(true),
@@ -26,6 +27,7 @@ fn main() {
             Arg::with_name("OUTPUT")
                 .help("Specify output xml file")
                 .required(true)
+                .short("o")
                 .long("output")
                 .value_name("OUTPUT")
                 .takes_value(true),

src/io_engine.rs

@@ -258,8 +258,7 @@ impl AsyncIoEngine {
         let fd_inner = inner.input.as_raw_fd();
 
         for (i, b) in blocks.iter().enumerate() {
-            let read_e = opcode::Read::new(
+            let read_e = opcode::Read::new(types::Fd(fd_inner), b.data, BLOCK_SIZE as u32)
-                    types::Fd(fd_inner), b.data, BLOCK_SIZE as u32)
                 .offset(b.loc as i64 * BLOCK_SIZE as i64);
 
             unsafe {
@@ -310,8 +309,7 @@ impl AsyncIoEngine {
         let fd_inner = inner.input.as_raw_fd();
 
         for (i, b) in blocks.iter().enumerate() {
-            let write_e = opcode::Write::new(
+            let write_e = opcode::Write::new(types::Fd(fd_inner), b.data, BLOCK_SIZE as u32)
-                    types::Fd(fd_inner), b.data, BLOCK_SIZE as u32)
                 .offset(b.loc as i64 * BLOCK_SIZE as i64);
 
             unsafe {

src/pack/node_encode.rs

@@ -59,11 +59,11 @@ fn summarise_node(data: &[u8]) -> IResult<&[u8], NodeSummary> {
 
 pub fn pack_btree_node<W: Write>(w: &mut W, data: &[u8]) -> PResult<()> {
     let (_, info) = nom_to_pr(summarise_node(data))?;
+    let (i, hdr) = nom_to_pr(take(32usize)(data))?;
+    let (i, keys) = nom_to_pr(run64(i, info.max_entries))?;
 
     if info.is_leaf {
         if info.value_size == std::mem::size_of::<u64>() {
-            let (i, hdr) = nom_to_pr(take(32usize)(data))?;
-            let (i, keys) = nom_to_pr(run64(i, info.max_entries))?;
             let (tail, values) = nom_to_pr(run64(i, info.max_entries))?;
 
             io_to_pr(pack_literal(w, hdr))?;
@@ -76,8 +76,7 @@ pub fn pack_btree_node<W: Write>(w: &mut W, data: &[u8]) -> PResult<()> {
             Ok(())
         } else {
             // We don't bother packing the values if they aren't u64
-            let (i, hdr) = nom_to_pr(take(32usize)(data))?;
+            let tail = i;
-            let (tail, keys) = nom_to_pr(run64(i, info.max_entries))?;
 
             io_to_pr(pack_literal(w, hdr))?;
             io_to_pr(pack_u64s(w, &keys))?;
@@ -87,8 +86,6 @@ pub fn pack_btree_node<W: Write>(w: &mut W, data: &[u8]) -> PResult<()> {
         }
     } else {
         // Internal node, values are also u64s
-        let (i, hdr) = nom_to_pr(take(32usize)(data))?;
-        let (i, keys) = nom_to_pr(run64(i, info.max_entries))?;
         let (tail, values) = nom_to_pr(run64(i, info.max_entries))?;
 
         io_to_pr(pack_literal(w, hdr))?;

src/pack/toplevel.rs

@@ -163,24 +163,28 @@ fn read_header<R>(mut r: R) -> io::Result<u64>
 where
     R: byteorder::ReadBytesExt,
 {
-    use std::process::exit;
-
     let magic = r.read_u64::<LittleEndian>()?;
     if magic != MAGIC {
-        eprintln!("Not a pack file.");
+        return Err(io::Error::new(
-        exit(1);
+            io::ErrorKind::InvalidData,
+            "Not a pack file",
+        ));
     }
 
     let version = r.read_u64::<LittleEndian>()?;
     if version != PACK_VERSION {
-        eprintln!("unsupported pack file version ({}).", PACK_VERSION);
+        return Err(io::Error::new(
-        exit(1);
+            io::ErrorKind::InvalidData,
+            format!("unsupported pack file version ({}).", PACK_VERSION),
+        ));
     }
 
     let block_size = r.read_u64::<LittleEndian>()?;
     if block_size != BLOCK_SIZE {
-        eprintln!("block size is not {}", BLOCK_SIZE);
+        return Err(io::Error::new(
-        exit(1);
+            io::ErrorKind::InvalidData,
+            format!("block size is not {}", BLOCK_SIZE),
+        ));
     }
 
     r.read_u64::<LittleEndian>()
@@ -270,6 +274,8 @@ pub fn unpack(input_file: &Path, output_file: &Path) -> Result<(), Box<dyn Error
         .write(false)
         .open(input_file)?;
 
+    let nr_blocks = read_header(&input)?;
+
     let mut output = OpenOptions::new()
         .read(false)
         .write(true)
@@ -277,8 +283,6 @@ pub fn unpack(input_file: &Path, output_file: &Path) -> Result<(), Box<dyn Error
         .truncate(true)
         .open(output_file)?;
 
-    let nr_blocks = read_header(&input)?;
-
     // zero the last block to size the file
     write_zero_block(&mut output, nr_blocks - 1)?;
 

src/pdata/btree_leaf_walker.rs

@@ -210,14 +210,13 @@ impl<'a> LeafWalker<'a> {
 
         let depth = self.get_depth::<V>(path, root, true)?;
 
+        self.sm_inc(root);
         if depth == 0 {
             // root is a leaf
-            self.sm_inc(root);
             self.leaves.insert(root as usize);
             visitor.visit(&kr, root)?;
             Ok(())
         } else {
-            self.sm_inc(root);
             let root = self.engine.read(root).map_err(|_| io_err(path))?;
 
             self.walk_node(depth - 1, path, visitor, &kr, &root, true)

tests/cache_check.rs

@@ -1,89 +1,93 @@
 use anyhow::Result;
-use duct::cmd;
-use thinp::version::tools_version;
 
 mod common;
 
+use common::common_args::*;
+use common::input_arg::*;
 use common::test_dir::*;
 use common::*;
 
 //------------------------------------------
 
-#[test]
+const USAGE: &str = "Usage: cache_check [options] {device|file}\n\
-fn accepts_v() -> Result<()> {
+                     Options:\n  \
-    let stdout = cache_check!("-V").read()?;
+                       {-q|--quiet}\n  \
-    assert!(stdout.contains(tools_version()));
+                       {-h|--help}\n  \
-    Ok(())
+                       {-V|--version}\n  \
+                       {--clear-needs-check-flag}\n  \
+                       {--super-block-only}\n  \
+                       {--skip-mappings}\n  \
+                       {--skip-hints}\n  \
+                       {--skip-discards}";
+
+//------------------------------------------
+
+struct CacheCheck;
+
+impl<'a> Program<'a> for CacheCheck {
+    fn name() -> &'a str {
+        "cache_check"
     }
 
-#[test]
+    fn path() -> &'a str {
-fn accepts_version() -> Result<()> {
+        CACHE_CHECK
-    let stdout = cache_check!("--version").read()?;
-    assert!(stdout.contains(tools_version()));
-    Ok(())
     }
 
-const USAGE: &str = "Usage: cache_check [options] {device|file}\nOptions:\n  {-q|--quiet}\n  {-h|--help}\n  {-V|--version}\n  {--clear-needs-check-flag}\n  {--super-block-only}\n  {--skip-mappings}\n  {--skip-hints}\n  {--skip-discards}";
+    fn usage() -> &'a str {
-
+        USAGE
-#[test]
-fn accepts_h() -> Result<()> {
-    let stdout = cache_check!("-h").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
-#[test]
+    fn arg_type() -> ArgType {
-fn accepts_help() -> Result<()> {
+        ArgType::InputArg
-    let stdout = cache_check!("--help").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
-#[test]
+    fn bad_option_hint(option: &str) -> String {
-fn missing_metadata() -> Result<()> {
+        msg::bad_option_hint(option)
-    let stderr = run_fail(cache_check!())?;
+    }
-    assert!(stderr.contains(msg::MISSING_INPUT_ARG));
-    Ok(())
 }
 
-#[test]
+impl<'a> InputProgram<'a> for CacheCheck {
-fn no_such_metadata() -> Result<()> {
+    fn mk_valid_input(td: &mut TestDir) -> Result<std::path::PathBuf> {
-    let stderr = run_fail(cache_check!("/arbitrary/filename"))?;
+        mk_valid_md(td)
-    assert!(stderr.contains("No such file or directory"));
-    Ok(())
     }
 
-#[test]
+    fn file_not_found() -> &'a str {
-fn metadata_cannot_be_a_directory() -> Result<()> {
+        msg::FILE_NOT_FOUND
-    let stderr = run_fail(cache_check!("/tmp"))?;
-    assert!(stderr.contains("Not a block device or regular file"));
-    Ok(())
     }
 
-#[test]
+    fn missing_input_arg() -> &'a str {
-fn unreadable_metadata() -> Result<()> {
+        msg::MISSING_INPUT_ARG
-    let mut td = TestDir::new()?;
-    let md = mk_valid_md(&mut td)?;
-    cmd!("chmod", "-r", &md).run()?;
-    let stderr = run_fail(cache_check!(&md))?;
-    assert!(stderr.contains("Permission denied"));
-    Ok(())
     }
 
-#[test]
+    fn corrupted_input() -> &'a str {
-fn corrupt_metadata() -> Result<()> {
+        msg::BAD_SUPERBLOCK
-    let mut td = TestDir::new()?;
-    let md = mk_zeroed_md(&mut td)?;
-    run_fail(cache_check!(&md))?;
-    Ok(())
     }
+}
+
+impl<'a> BinaryInputProgram<'_> for CacheCheck {}
+
+//------------------------------------------
+
+test_accepts_help!(CacheCheck);
+test_accepts_version!(CacheCheck);
+test_rejects_bad_option!(CacheCheck);
+
+test_missing_input_arg!(CacheCheck);
+test_input_file_not_found!(CacheCheck);
+test_input_cannot_be_a_directory!(CacheCheck);
+test_unreadable_input_file!(CacheCheck);
+
+test_help_message_for_tiny_input_file!(CacheCheck);
+test_spot_xml_data!(CacheCheck);
+test_corrupted_input_data!(CacheCheck);
+
+//------------------------------------------
 
 #[test]
 fn failing_q() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_zeroed_md(&mut td)?;
-    let output = cache_check!("-q", &md).unchecked().run()?;
+    let output = run_fail_raw(CACHE_CHECK, &["-q", md.to_str().unwrap()])?;
-    assert!(!output.status.success());
     assert_eq!(output.stdout.len(), 0);
     assert_eq!(output.stderr.len(), 0);
     Ok(())
@@ -93,8 +97,7 @@ fn failing_q() -> Result<()> {
 fn failing_quiet() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_zeroed_md(&mut td)?;
-    let output = cache_check!("--quiet", &md).unchecked().run()?;
+    let output = run_fail_raw(CACHE_CHECK, &["--quiet", md.to_str().unwrap()])?;
-    assert!(!output.status.success());
     assert_eq!(output.stdout.len(), 0);
     assert_eq!(output.stderr.len(), 0);
     Ok(())

tests/cache_dump.rs

@@ -0,0 +1,85 @@
+use anyhow::Result;
+
+mod common;
+
+use common::common_args::*;
+use common::input_arg::*;
+use common::test_dir::*;
+use common::*;
+
+//------------------------------------------
+
+const USAGE: &str = "Usage: cache_dump [options] {device|file}\n\
+                     Options:\n  \
+                       {-h|--help}\n  \
+                       {-o <xml file>}\n  \
+                       {-V|--version}\n  \
+                       {--repair}";
+
+//------------------------------------------
+
+struct CacheDump;
+
+impl<'a> Program<'a> for CacheDump {
+    fn name() -> &'a str {
+        "cache_dump"
+    }
+
+    fn path() -> &'a str {
+        CACHE_DUMP
+    }
+
+    fn usage() -> &'a str {
+        USAGE
+    }
+
+    fn arg_type() -> ArgType {
+        ArgType::InputArg
+    }
+
+    fn bad_option_hint(option: &str) -> String {
+        msg::bad_option_hint(option)
+    }
+}
+
+impl<'a> InputProgram<'a> for CacheDump {
+    fn mk_valid_input(td: &mut TestDir) -> Result<std::path::PathBuf> {
+        mk_valid_md(td)
+    }
+
+    fn file_not_found() -> &'a str {
+        msg::FILE_NOT_FOUND
+    }
+
+    fn missing_input_arg() -> &'a str {
+        msg::MISSING_INPUT_ARG
+    }
+
+    fn corrupted_input() -> &'a str {
+        msg::BAD_SUPERBLOCK
+    }
+}
+
+//------------------------------------------
+
+test_accepts_help!(CacheDump);
+test_accepts_version!(CacheDump);
+test_rejects_bad_option!(CacheDump);
+
+test_missing_input_arg!(CacheDump);
+test_input_file_not_found!(CacheDump);
+test_input_cannot_be_a_directory!(CacheDump);
+test_unreadable_input_file!(CacheDump);
+
+//------------------------------------------
+
+/*
+  (define-scenario (cache-dump restore-is-noop)
+    "cache_dump followed by cache_restore is a noop."
+    (with-valid-metadata (md)
+      (run-ok-rcv (d1-stdout _) (cache-dump md)
+        (with-temp-file-containing ((xml "cache.xml" d1-stdout))
+          (run-ok (cache-restore "-i" xml "-o" md))
+          (run-ok-rcv (d2-stdout _) (cache-dump md)
+            (assert-equal d1-stdout d2-stdout))))))
+*/

tests/common/common_args.rs

@@ -0,0 +1,98 @@
+use crate::common::*;
+use thinp::version::tools_version;
+
+//------------------------------------------
+// help
+
+pub fn test_help_short<'a, P>() -> Result<()>
+where
+    P: Program<'a>,
+{
+    let stdout = run_ok(P::path(), &["-h"])?;
+    assert_eq!(stdout, P::usage());
+    Ok(())
+}
+
+pub fn test_help_long<'a, P>() -> Result<()>
+where
+    P: Program<'a>,
+{
+    let stdout = run_ok(P::path(), &["--help"])?;
+    assert_eq!(stdout, P::usage());
+    Ok(())
+}
+
+#[macro_export]
+macro_rules! test_accepts_help {
+    ($program: ident) => {
+        #[test]
+        fn accepts_h() -> Result<()> {
+            test_help_short::<$program>()
+        }
+
+        #[test]
+        fn accepts_help() -> Result<()> {
+            test_help_long::<$program>()
+        }
+    };
+}
+
+//------------------------------------------
+// version
+
+pub fn test_version_short<'a, P>() -> Result<()>
+where
+    P: Program<'a>,
+{
+    let stdout = run_ok(P::path(), &["-V"])?;
+    assert!(stdout.contains(tools_version()));
+    Ok(())
+}
+
+pub fn test_version_long<'a, P>() -> Result<()>
+where
+    P: Program<'a>,
+{
+    let stdout = run_ok(P::path(), &["--version"])?;
+    assert!(stdout.contains(tools_version()));
+    Ok(())
+}
+
+#[macro_export]
+macro_rules! test_accepts_version {
+    ($program: ident) => {
+        #[test]
+        fn accepts_v() -> Result<()> {
+            test_version_short::<$program>()
+        }
+
+        #[test]
+        fn accepts_version() -> Result<()> {
+            test_version_long::<$program>()
+        }
+    };
+}
+
+//------------------------------------------
+
+pub fn test_rejects_bad_option<'a, P>() -> Result<()>
+where
+    P: Program<'a>,
+{
+    let option = "--hedgehogs-only";
+    let stderr = run_fail(P::path(), &[option])?;
+    assert!(stderr.contains(&P::bad_option_hint(option)));
+    Ok(())
+}
+
+#[macro_export]
+macro_rules! test_rejects_bad_option {
+    ($program: ident) => {
+        #[test]
+        fn rejects_bad_option() -> Result<()> {
+            test_rejects_bad_option::<$program>()
+        }
+    };
+}
+
+//------------------------------------------

tests/common/input_arg.rs

@@ -0,0 +1,263 @@
+use crate::common::thin_xml_generator::{write_xml, FragmentedS};
+use crate::common::*;
+
+//------------------------------------------
+// wrappers
+
+type ArgsBuilder = fn(&mut TestDir, &str, &dyn Fn(&[&str]) -> Result<()>) -> Result<()>;
+
+fn with_output_md_untouched(
+    td: &mut TestDir,
+    input: &str,
+    thunk: &dyn Fn(&[&str]) -> Result<()>,
+) -> Result<()> {
+    let output = mk_zeroed_md(td)?;
+    ensure_untouched(&output, || {
+        let args = ["-i", input, "-o", output.to_str().unwrap()];
+        thunk(&args)
+    })
+}
+
+fn with_output_superblock_zeroed(
+    td: &mut TestDir,
+    input: &str,
+    thunk: &dyn Fn(&[&str]) -> Result<()>,
+) -> Result<()> {
+    let output = mk_zeroed_md(td)?;
+    ensure_superblock_zeroed(&output, || {
+        let args = ["-i", input, "-o", output.to_str().unwrap()];
+        thunk(&args)
+    })
+}
+
+fn input_arg_only(
+    _td: &mut TestDir,
+    input: &str,
+    thunk: &dyn Fn(&[&str]) -> Result<()>,
+) -> Result<()> {
+    let args = [input];
+    thunk(&args)
+}
+
+fn build_args_fn(t: ArgType) -> Result<ArgsBuilder> {
+    match t {
+        ArgType::InputArg => Ok(input_arg_only),
+        ArgType::IoOptions => Ok(with_output_md_untouched),
+    }
+}
+
+//------------------------------------------
+// test invalid arguments
+
+pub fn test_missing_input_arg<'a, P>() -> Result<()>
+where
+    P: InputProgram<'a>,
+{
+    let stderr = run_fail(P::path(), &[])?;
+    assert!(stderr.contains(P::missing_input_arg()));
+    Ok(())
+}
+
+#[macro_export]
+macro_rules! test_missing_input_arg {
+    ($program: ident) => {
+        #[test]
+        fn missing_input_arg() -> Result<()> {
+            test_missing_input_arg::<$program>()
+        }
+    };
+}
+
+pub fn test_missing_input_option<'a, P>() -> Result<()>
+where
+    P: InputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+    let output = mk_zeroed_md(&mut td)?;
+    ensure_untouched(&output, || {
+        let args = ["-o", output.to_str().unwrap()];
+        let stderr = run_fail(P::path(), &args)?;
+        assert!(stderr.contains(P::missing_input_arg()));
+        Ok(())
+    })
+}
+
+#[macro_export]
+macro_rules! test_missing_input_option {
+    ($program: ident) => {
+        #[test]
+        fn missing_input_option() -> Result<()> {
+            test_missing_input_option::<$program>()
+        }
+    };
+}
+
+pub fn test_input_file_not_found<'a, P>() -> Result<()>
+where
+    P: InputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+
+    let wrapper = build_args_fn(P::arg_type())?;
+    wrapper(&mut td, "no-such-file", &|args: &[&str]| {
+        let stderr = run_fail(P::path(), args)?;
+        assert!(stderr.contains(P::file_not_found()));
+        Ok(())
+    })
+}
+
+#[macro_export]
+macro_rules! test_input_file_not_found {
+    ($program: ident) => {
+        #[test]
+        fn input_file_not_found() -> Result<()> {
+            test_input_file_not_found::<$program>()
+        }
+    };
+}
+
+pub fn test_input_cannot_be_a_directory<'a, P>() -> Result<()>
+where
+    P: InputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+
+    let wrapper = build_args_fn(P::arg_type())?;
+    wrapper(&mut td, "/tmp", &|args: &[&str]| {
+        let stderr = run_fail(P::path(), args)?;
+        assert!(stderr.contains("Not a block device or regular file"));
+        Ok(())
+    })
+}
+
+#[macro_export]
+macro_rules! test_input_cannot_be_a_directory {
+    ($program: ident) => {
+        #[test]
+        fn input_cannot_be_a_directory() -> Result<()> {
+            test_input_cannot_be_a_directory::<$program>()
+        }
+    };
+}
+
+pub fn test_unreadable_input_file<'a, P>() -> Result<()>
+where
+    P: InputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+
+    // input an unreadable file
+    let input = mk_valid_md(&mut td)?;
+    duct::cmd!("chmod", "-r", &input).run()?;
+
+    let wrapper = build_args_fn(P::arg_type())?;
+    wrapper(&mut td, input.to_str().unwrap(), &|args: &[&str]| {
+        let stderr = run_fail(P::path(), args)?;
+        assert!(stderr.contains("Permission denied"));
+        Ok(())
+    })
+}
+
+#[macro_export]
+macro_rules! test_unreadable_input_file {
+    ($program: ident) => {
+        #[test]
+        fn unreadable_input_file() -> Result<()> {
+            test_unreadable_input_file::<$program>()
+        }
+    };
+}
+
+//------------------------------------------
+// test invalid content
+
+pub fn test_help_message_for_tiny_input_file<'a, P>() -> Result<()>
+where
+    P: BinaryInputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+
+    let input = td.mk_path("meta.bin");
+    file_utils::create_sized_file(&input, 1024)?;
+
+    let wrapper = build_args_fn(P::arg_type())?;
+    wrapper(&mut td, input.to_str().unwrap(), &|args: &[&str]| {
+        let stderr = run_fail(P::path(), args)?;
+        assert!(stderr.contains("Metadata device/file too small.  Is this binary metadata?"));
+        Ok(())
+    })
+}
+
+#[macro_export]
+macro_rules! test_help_message_for_tiny_input_file {
+    ($program: ident) => {
+        #[test]
+        fn prints_help_message_for_tiny_input_file() -> Result<()> {
+            test_help_message_for_tiny_input_file::<$program>()
+        }
+    };
+}
+
+pub fn test_spot_xml_data<'a, P>() -> Result<()>
+where
+    P: BinaryInputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+
+    // input a large xml file
+    let input = td.mk_path("meta.xml");
+    let mut gen = FragmentedS::new(4, 10240);
+    write_xml(&input, &mut gen)?;
+
+    let wrapper = build_args_fn(P::arg_type())?;
+    wrapper(&mut td, input.to_str().unwrap(), &|args: &[&str]| {
+        let stderr = run_fail(P::path(), args)?;
+        eprintln!("{}", stderr);
+        let msg = format!(
+            "This looks like XML.  {} only checks the binary metadata format.",
+            P::name()
+        );
+        assert!(stderr.contains(&msg));
+        Ok(())
+    })
+}
+
+#[macro_export]
+macro_rules! test_spot_xml_data {
+    ($program: ident) => {
+        #[test]
+        fn spot_xml_data() -> Result<()> {
+            test_spot_xml_data::<$program>()
+        }
+    };
+}
+
+pub fn test_corrupted_input_data<'a, P>() -> Result<()>
+where
+    P: InputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+    let input = mk_zeroed_md(&mut td)?;
+
+    let wrapper = match P::arg_type() {
+        ArgType::InputArg => input_arg_only,
+        ArgType::IoOptions => with_output_superblock_zeroed,
+    };
+    wrapper(&mut td, input.to_str().unwrap(), &|args: &[&str]| {
+        let stderr = run_fail(P::path(), args)?;
+        assert!(stderr.contains(P::corrupted_input()));
+        Ok(())
+    })
+}
+
+#[macro_export]
+macro_rules! test_corrupted_input_data {
+    ($program: ident) => {
+        #[test]
+        fn corrupted_input_data() -> Result<()> {
+            test_corrupted_input_data::<$program>()
+        }
+    };
+}
+
+//------------------------------------------

tests/common/mod.rs

@@ -1,15 +1,16 @@
 #![allow(dead_code)]
 
 use anyhow::Result;
-use duct::{cmd, Expression};
 use std::fs::OpenOptions;
 use std::io::{Read, Write};
 use std::path::PathBuf;
-use std::str::from_utf8;
 use thinp::file_utils;
 use thinp::io_engine::*;
 
 pub mod cache_xml_generator;
+pub mod common_args;
+pub mod input_arg;
+pub mod output_option;
 pub mod test_dir;
 pub mod thin_xml_generator;
 
@@ -18,19 +19,32 @@ use test_dir::TestDir;
 
 //------------------------------------------
 
-#[cfg(not(feature = "rust_tests"))]
+pub mod cpp_msg {
-pub mod msg {
+    pub const FILE_NOT_FOUND: &str = "No such file or directory";
-    pub const FILE_NOT_FOUND: &str = "Couldn't stat file";
     pub const MISSING_INPUT_ARG: &str = "No input file provided";
     pub const MISSING_OUTPUT_ARG: &str = "No output file provided";
+    pub const BAD_SUPERBLOCK: &str = "bad checksum in superblock";
+
+    pub fn bad_option_hint(option: &str) -> String {
+        format!("unrecognized option '{}'", option)
+    }
 }
 
-#[cfg(feature = "rust_tests")]
+pub mod rust_msg {
-pub mod msg {
     pub const FILE_NOT_FOUND: &str = "Couldn't find input file";
-    pub const MISSING_INPUT_ARG: &str = "The following required arguments were not provided";
+    pub const MISSING_INPUT_ARG: &str = "The following required arguments were not provided"; // TODO: be specific
-    pub const MISSING_OUTPUT_ARG: &str = "The following required arguments were not provided";
+    pub const MISSING_OUTPUT_ARG: &str = "The following required arguments were not provided"; // TODO: be specific
+    pub const BAD_SUPERBLOCK: &str = "bad checksum in superblock";
+
+    pub fn bad_option_hint(option: &str) -> String {
+        format!("Found argument '{}' which wasn't expected", option)
     }
+}
+
+#[cfg(not(feature = "rust_tests"))]
+pub use cpp_msg as msg;
+#[cfg(feature = "rust_tests")]
+pub use rust_msg as msg;
 
 //------------------------------------------
 
@@ -64,150 +78,100 @@ macro_rules! path_to {
     };
 }
 
-// FIXME: write a macro to generate these commands
+//------------------------------------------
-// Known issue of nested macro definition: https://github.com/rust-lang/rust/issues/35853
-// RFC: https://github.com/rust-lang/rfcs/blob/master/text/3086-macro-metavar-expr.md
-#[macro_export]
-macro_rules! thin_check {
-    ( $( $arg: expr ),* ) => {
-        {
-            use std::ffi::OsString;
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
-            duct::cmd(path_to!("thin_check"), args).stdout_capture().stderr_capture()
-        }
-    };
-}
 
-#[macro_export]
+pub const CACHE_CHECK: &str = path_to!("cache_check");
-macro_rules! thin_restore {
+pub const CACHE_DUMP: &str = path_to!("cache_dump");
-    ( $( $arg: expr ),* ) => {
-        {
-            use std::ffi::OsString;
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
-            duct::cmd(path_to!("thin_restore"), args).stdout_capture().stderr_capture()
-        }
-    };
-}
 
-#[macro_export]
+pub const THIN_CHECK: &str = path_to!("thin_check");
-macro_rules! thin_dump {
+pub const THIN_DELTA: &str = path_to_cpp!("thin_delta"); // TODO: rust version
-    ( $( $arg: expr ),* ) => {
+pub const THIN_DUMP: &str = path_to!("thin_dump");
-        {
+pub const THIN_METADATA_PACK: &str = path_to_rust!("thin_metadata_pack"); // rust-only
-            use std::ffi::OsString;
+pub const THIN_METADATA_UNPACK: &str = path_to_rust!("thin_metadata_unpack"); // rust-only
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
+pub const THIN_REPAIR: &str = path_to_cpp!("thin_repair"); // TODO: rust version
-            duct::cmd(path_to!("thin_dump"), args).stdout_capture().stderr_capture()
+pub const THIN_RESTORE: &str = path_to!("thin_restore");
-        }
+pub const THIN_RMAP: &str = path_to_cpp!("thin_rmap"); // TODO: rust version
-    };
+pub const THIN_GENERATE_METADATA: &str = path_to_cpp!("thin_generate_metadata"); // cpp-only
-}
+pub const THIN_GENERATE_MAPPINGS: &str = path_to_cpp!("thin_generate_mappings"); // cpp-only
-
+pub const THIN_GENERATE_DAMAGE: &str = path_to_cpp!("thin_generate_damage"); // cpp-only
-#[macro_export]
-macro_rules! thin_rmap {
-    ( $( $arg: expr ),* ) => {
-        {
-            use std::ffi::OsString;
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
-            duct::cmd(path_to_cpp!("thin_rmap"), args).stdout_capture().stderr_capture()
-        }
-    };
-}
-
-#[macro_export]
-macro_rules! thin_repair {
-    ( $( $arg: expr ),* ) => {
-        {
-            use std::ffi::OsString;
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
-            duct::cmd(path_to_cpp!("thin_repair"), args).stdout_capture().stderr_capture()
-        }
-    };
-}
-
-#[macro_export]
-macro_rules! thin_delta {
-    ( $( $arg: expr ),* ) => {
-        {
-            use std::ffi::OsString;
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
-            duct::cmd(path_to_cpp!("thin_delta"), args).stdout_capture().stderr_capture()
-        }
-    };
-}
-
-#[macro_export]
-macro_rules! thin_metadata_pack {
-    ( $( $arg: expr ),* ) => {
-        {
-            use std::ffi::OsString;
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
-            duct::cmd(path_to_rust!("thin_metadata_pack"), args).stdout_capture().stderr_capture()
-        }
-    };
-}
-
-#[macro_export]
-macro_rules! thin_metadata_unpack {
-    ( $( $arg: expr ),* ) => {
-        {
-            use std::ffi::OsString;
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
-            duct::cmd(path_to_rust!("thin_metadata_unpack"), args).stdout_capture().stderr_capture()
-        }
-    };
-}
-
-#[macro_export]
-macro_rules! cache_check {
-    ( $( $arg: expr ),* ) => {
-        {
-            use std::ffi::OsString;
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
-            duct::cmd(path_to!("cache_check"), args).stdout_capture().stderr_capture()
-        }
-    };
-}
-
-#[macro_export]
-macro_rules! thin_generate_metadata {
-    ( $( $arg: expr ),* ) => {
-        {
-            use std::ffi::OsString;
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
-            duct::cmd(path_to_cpp!("thin_generate_metadata"), args).stdout_capture().stderr_capture()
-        }
-    };
-}
-
-#[macro_export]
-macro_rules! thin_generate_mappings {
-    ( $( $arg: expr ),* ) => {
-        {
-            use std::ffi::OsString;
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
-            duct::cmd(path_to_cpp!("thin_generate_mappings"), args).stdout_capture().stderr_capture()
-        }
-    };
-}
-
-#[macro_export]
-macro_rules! thin_generate_damage {
-    ( $( $arg: expr ),* ) => {
-        {
-            use std::ffi::OsString;
-            let args: &[OsString] = &[$( Into::<OsString>::into($arg) ),*];
-            duct::cmd("bin/thin_generate_damage", args).stdout_capture().stderr_capture()
-        }
-    };
-}
 
 //------------------------------------------
 
-// Returns stderr, a non zero status must be returned
+pub enum ArgType {
-pub fn run_fail(command: Expression) -> Result<String> {
+    InputArg,
-    let output = command.stderr_capture().unchecked().run()?;
+    IoOptions,
-    assert!(!output.status.success());
-    Ok(from_utf8(&output.stderr[0..]).unwrap().to_string())
 }
 
+pub trait Program<'a> {
+    fn name() -> &'a str;
+    fn path() -> &'a str;
+    fn usage() -> &'a str;
+    fn arg_type() -> ArgType;
+
+    // error messages
+    fn bad_option_hint(option: &str) -> String;
+}
+
+pub trait InputProgram<'a>: Program<'a> {
+    fn mk_valid_input(td: &mut TestDir) -> Result<PathBuf>;
+
+    // error messages
+    fn missing_input_arg() -> &'a str;
+    fn file_not_found() -> &'a str;
+    fn corrupted_input() -> &'a str;
+}
+
+pub trait BinaryInputProgram<'a>: InputProgram<'a> {}
+
+pub trait OutputProgram<'a>: InputProgram<'a> {
+    // error messages
+    fn missing_output_arg() -> &'a str;
+    fn file_not_found() -> &'a str;
+}
+
+pub trait BinaryOutputProgram<'a>: OutputProgram<'a> {}
+
+//------------------------------------------
+
+// Returns stdout. The command must return zero.
+pub fn run_ok(program: &str, args: &[&str]) -> Result<String> {
+    let command = duct::cmd(program, args).stdout_capture().stderr_capture();
+    let output = command.run()?;
+    assert!(output.status.success());
+    let stdout = std::str::from_utf8(&output.stdout[..])
+        .unwrap()
+        .trim_end_matches(|c| c == '\n' || c == '\r')
+        .to_string();
+    Ok(stdout)
+}
+
+// Returns the entire output. The command must return zero.
+pub fn run_ok_raw(program: &str, args: &[&str]) -> Result<std::process::Output> {
+    let command = duct::cmd(program, args).stdout_capture().stderr_capture();
+    let output = command.run()?;
+    assert!(output.status.success());
+    Ok(output)
+}
+
+// Returns stderr, a non zero status must be returned
+pub fn run_fail(program: &str, args: &[&str]) -> Result<String> {
+    let command = duct::cmd(program, args).stdout_capture().stderr_capture();
+    let output = command.unchecked().run()?;
+    assert!(!output.status.success());
+    let stderr = std::str::from_utf8(&output.stderr[..]).unwrap().to_string();
+    Ok(stderr)
+}
+
+// Returns the entire output, a non zero status must be returned
+pub fn run_fail_raw(program: &str, args: &[&str]) -> Result<std::process::Output> {
+    let command = duct::cmd(program, args).stdout_capture().stderr_capture();
+    let output = command.unchecked().run()?;
+    assert!(!output.status.success());
+    Ok(output)
+}
+
+//------------------------------------------
+
 pub fn mk_valid_xml(td: &mut TestDir) -> Result<PathBuf> {
     let xml = td.mk_path("meta.xml");
     let mut gen = SingleThinS::new(0, 1024, 2048, 2048);
@@ -223,7 +187,9 @@ pub fn mk_valid_md(td: &mut TestDir) -> Result<PathBuf> {
     write_xml(&xml, &mut gen)?;
 
     let _file = file_utils::create_sized_file(&md, 4096 * 4096);
-    thin_restore!("-i", xml, "-o", &md).run()?;
+    let args = ["-i", xml.to_str().unwrap(), "-o", md.to_str().unwrap()];
+    run_ok(THIN_RESTORE, &args)?;
+
     Ok(md)
 }
 
@@ -234,13 +200,6 @@ pub fn mk_zeroed_md(td: &mut TestDir) -> Result<PathBuf> {
     Ok(md)
 }
 
-pub fn accepts_flag(flag: &str) -> Result<()> {
-    let mut td = TestDir::new()?;
-    let md = mk_valid_md(&mut td)?;
-    thin_check!(flag, &md).run()?;
-    Ok(())
-}
-
 pub fn superblock_all_zeroes(path: &PathBuf) -> Result<bool> {
     let mut input = OpenOptions::new().read(true).write(false).open(path)?;
     let mut buf = vec![0; 4096];
@@ -266,50 +225,58 @@ pub fn damage_superblock(path: &PathBuf) -> Result<()> {
 // FIXME: replace mk_valid_md with this?
 pub fn prep_metadata(td: &mut TestDir) -> Result<PathBuf> {
     let md = mk_zeroed_md(td)?;
-    thin_generate_metadata!("-o", &md, "--format", "--nr-data-blocks", "102400").run()?;
+    let args = [
+        "-o",
+        md.to_str().unwrap(),
+        "--format",
+        "--nr-data-blocks",
+        "102400",
+    ];
+    run_ok(THIN_GENERATE_METADATA, &args)?;
 
     // Create a 2GB device
-    thin_generate_metadata!("-o", &md, "--create-thin", "1").run()?;
+    let args = ["-o", md.to_str().unwrap(), "--create-thin", "1"];
-    thin_generate_mappings!(
+    run_ok(THIN_GENERATE_METADATA, &args)?;
+    let args = [
         "-o",
-        &md,
+        md.to_str().unwrap(),
         "--dev-id",
         "1",
         "--size",
-        format!("{}", 1024 * 1024 * 2),
+        "2097152",
         "--rw=randwrite",
-        "--seq-nr=16"
+        "--seq-nr=16",
-    )
+    ];
-    .run()?;
+    run_ok(THIN_GENERATE_MAPPINGS, &args)?;
 
     // Take a few snapshots.
     let mut snap_id = 2;
     for _i in 0..10 {
         // take a snapshot
-        thin_generate_metadata!(
+        let args = [
             "-o",
-            &md,
+            md.to_str().unwrap(),
             "--create-snap",
-            format!("{}", snap_id),
+            &snap_id.to_string(),
             "--origin",
-            "1"
+            "1",
-        )
+        ];
-        .run()?;
+        run_ok(THIN_GENERATE_METADATA, &args)?;
 
         // partially overwrite the origin (64MB)
-        thin_generate_mappings!(
+        let args = [
             "-o",
-            &md,
+            md.to_str().unwrap(),
             "--dev-id",
-            format!("{}", 1),
+            "1",
             "--size",
-            format!("{}", 1024 * 1024 * 2),
+            "2097152",
             "--io-size",
-            format!("{}", 64 * 1024 * 2),
+            "131072",
             "--rw=randwrite",
-            "--seq-nr=16"
+            "--seq-nr=16",
-        )
+        ];
-        .run()?;
+        run_ok(THIN_GENERATE_MAPPINGS, &args)?;
         snap_id += 1;
     }
 
@@ -317,7 +284,8 @@ pub fn prep_metadata(td: &mut TestDir) -> Result<PathBuf> {
 }
 
 pub fn set_needs_check(md: &PathBuf) -> Result<()> {
-    thin_generate_metadata!("-o", &md, "--set-needs-check").run()?;
+    let args = ["-o", md.to_str().unwrap(), "--set-needs-check"];
+    run_ok(THIN_GENERATE_METADATA, &args)?;
     Ok(())
 }
 
@@ -327,21 +295,19 @@ pub fn generate_metadata_leaks(
     expected: u32,
     actual: u32,
 ) -> Result<()> {
-    let output = thin_generate_damage!(
+    let args = [
         "-o",
-        &md,
+        md.to_str().unwrap(),
         "--create-metadata-leaks",
         "--nr-blocks",
-        format!("{}", nr_blocks),
+        &nr_blocks.to_string(),
         "--expected",
-        format!("{}", expected),
+        &expected.to_string(),
         "--actual",
-        format!("{}", actual)
+        &actual.to_string(),
-    )
+    ];
-    .unchecked()
+    run_ok(THIN_GENERATE_DAMAGE, &args)?;
-    .run()?;
 
-    assert!(output.status.success());
     Ok(())
 }
 
@@ -354,7 +320,7 @@ pub fn get_needs_check(md: &PathBuf) -> Result<bool> {
 }
 
 pub fn md5(md: &PathBuf) -> Result<String> {
-    let output = cmd!("md5sum", "-b", &md).stdout_capture().run()?;
+    let output = duct::cmd!("md5sum", "-b", &md).stdout_capture().run()?;
     let csum = std::str::from_utf8(&output.stdout[0..])?.to_string();
     let csum = csum.split_ascii_whitespace().next().unwrap().to_string();
     Ok(csum)
@@ -371,3 +337,14 @@ where
     assert_eq!(csum, md5(p)?);
     Ok(())
 }
+
+pub fn ensure_superblock_zeroed<F>(p: &PathBuf, thunk: F) -> Result<()>
+where
+    F: Fn() -> Result<()>,
+{
+    thunk()?;
+    assert!(superblock_all_zeroes(p)?);
+    Ok(())
+}
+
+//------------------------------------------

tests/common/output_option.rs

@@ -0,0 +1,143 @@
+use crate::common::*;
+
+//-----------------------------------------
+// test invalid arguments
+
+pub fn test_missing_output_option<'a, P>() -> Result<()>
+where
+    P: OutputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+    let input = P::mk_valid_input(&mut td)?;
+    let stderr = run_fail(P::path(), &["-i", input.to_str().unwrap()])?;
+    assert!(stderr.contains(P::missing_output_arg()));
+    Ok(())
+}
+
+#[macro_export]
+macro_rules! test_missing_output_option {
+    ($program: ident) => {
+        #[test]
+        fn missing_output_option() -> Result<()> {
+            test_missing_output_option::<$program>()
+        }
+    };
+}
+
+pub fn test_output_file_not_found<'a, P>() -> Result<()>
+where
+    P: OutputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+    let input = P::mk_valid_input(&mut td)?;
+    let stderr = run_fail(
+        P::path(),
+        &["-i", input.to_str().unwrap(), "-o", "no-such-file"],
+    )?;
+    assert!(stderr.contains(<P as OutputProgram>::file_not_found()));
+    Ok(())
+}
+
+#[macro_export]
+macro_rules! test_output_file_not_found {
+    ($program: ident) => {
+        #[test]
+        fn output_file_not_found() -> Result<()> {
+            test_output_file_not_found::<$program>()
+        }
+    };
+}
+
+pub fn test_output_cannot_be_a_directory<'a, P>() -> Result<()>
+where
+    P: OutputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+    let input = P::mk_valid_input(&mut td)?;
+    let stderr = run_fail(P::path(), &["-i", input.to_str().unwrap(), "-o", "/tmp"])?;
+    assert!(stderr.contains("Not a block device or regular file"));
+    Ok(())
+}
+
+#[macro_export]
+macro_rules! test_output_cannot_be_a_directory {
+    ($program: ident) => {
+        #[test]
+        fn output_cannot_be_a_directory() -> Result<()> {
+            test_output_cannot_be_a_directory::<$program>()
+        }
+    };
+}
+
+pub fn test_unwritable_output_file<'a, P>() -> Result<()>
+where
+    P: OutputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+    let input = P::mk_valid_input(&mut td)?;
+
+    let output = td.mk_path("meta.bin");
+    let _file = file_utils::create_sized_file(&output, 4096);
+    duct::cmd!("chmod", "-w", &output).run()?;
+
+    let stderr = run_fail(
+        P::path(),
+        &[
+            "-i",
+            input.to_str().unwrap(),
+            "-o",
+            output.to_str().unwrap(),
+        ],
+    )?;
+    assert!(stderr.contains("Permission denied"));
+    Ok(())
+}
+
+#[macro_export]
+macro_rules! test_unwritable_output_file {
+    ($program: ident) => {
+        #[test]
+        fn unwritable_output_file() -> Result<()> {
+            test_unwritable_output_file::<$program>()
+        }
+    };
+}
+
+//----------------------------------------
+// test invalid content
+
+// currently thin/cache_restore only
+pub fn test_tiny_output_file<'a, P>() -> Result<()>
+where
+    P: BinaryOutputProgram<'a>,
+{
+    let mut td = TestDir::new()?;
+    let input = P::mk_valid_input(&mut td)?;
+
+    let output = td.mk_path("meta.bin");
+    let _file = file_utils::create_sized_file(&output, 4096);
+
+    let stderr = run_fail(
+        P::path(),
+        &[
+            "-i",
+            input.to_str().unwrap(),
+            "-o",
+            output.to_str().unwrap(),
+        ],
+    )?;
+    assert!(stderr.contains("Output file too small"));
+    Ok(())
+}
+
+#[macro_export]
+macro_rules! test_tiny_output_file {
+    ($program: ident) => {
+        #[test]
+        fn tiny_output_file() -> Result<()> {
+            test_tiny_output_file::<$program>()
+        }
+    };
+}
+
+//-----------------------------------------

tests/thin_check.rs

@@ -1,49 +1,96 @@
 use anyhow::Result;
-use thinp::file_utils;
-use thinp::version::tools_version;
 
 mod common;
 
+use common::common_args::*;
+use common::input_arg::*;
 use common::test_dir::*;
-use common::thin_xml_generator::{write_xml, FragmentedS};
 use common::*;
 
 //------------------------------------------
 
-#[test]
+const USAGE: &str = "Usage: thin_check [options] {device|file}\n\
-fn accepts_v() -> Result<()> {
+                     Options:\n  \
-    let stdout = thin_check!("-V").read()?;
+                       {-q|--quiet}\n  \
-    assert!(stdout.contains(tools_version()));
+                       {-h|--help}\n  \
-    Ok(())
+                       {-V|--version}\n  \
+                       {-m|--metadata-snap}\n  \
+                       {--auto-repair}\n  \
+                       {--override-mapping-root}\n  \
+                       {--clear-needs-check-flag}\n  \
+                       {--ignore-non-fatal-errors}\n  \
+                       {--skip-mappings}\n  \
+                       {--super-block-only}";
+
+//-----------------------------------------
+
+struct ThinCheck;
+
+impl<'a> Program<'a> for ThinCheck {
+    fn name() -> &'a str {
+        "thin_check"
     }
 
-#[test]
+    fn path() -> &'a str {
-fn accepts_version() -> Result<()> {
+        THIN_CHECK
-    let stdout = thin_check!("--version").read()?;
-    assert!(stdout.contains(tools_version()));
-    Ok(())
     }
 
-const USAGE: &str = "Usage: thin_check [options] {device|file}\nOptions:\n  {-q|--quiet}\n  {-h|--help}\n  {-V|--version}\n  {-m|--metadata-snap}\n  {--auto-repair}\n  {--override-mapping-root}\n  {--clear-needs-check-flag}\n  {--ignore-non-fatal-errors}\n  {--skip-mappings}\n  {--super-block-only}";
+    fn usage() -> &'a str {
-
+        USAGE
-#[test]
-fn accepts_h() -> Result<()> {
-    let stdout = thin_check!("-h").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
-#[test]
+    fn arg_type() -> ArgType {
-fn accepts_help() -> Result<()> {
+        ArgType::InputArg
-    let stdout = thin_check!("--help").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
-#[test]
+    fn bad_option_hint(option: &str) -> String {
-fn rejects_bad_option() -> Result<()> {
+        msg::bad_option_hint(option)
-    let stderr = run_fail(thin_check!("--hedgehogs-only"))?;
+    }
-    assert!(stderr.contains("unrecognized option \'--hedgehogs-only\'"));
+}
+
+impl<'a> InputProgram<'a> for ThinCheck {
+    fn mk_valid_input(td: &mut TestDir) -> Result<std::path::PathBuf> {
+        mk_valid_md(td)
+    }
+
+    fn file_not_found() -> &'a str {
+        msg::FILE_NOT_FOUND
+    }
+
+    fn missing_input_arg() -> &'a str {
+        msg::MISSING_INPUT_ARG
+    }
+
+    fn corrupted_input() -> &'a str {
+        msg::BAD_SUPERBLOCK
+    }
+}
+
+impl<'a> BinaryInputProgram<'_> for ThinCheck {}
+
+//------------------------------------------
+
+test_accepts_help!(ThinCheck);
+test_accepts_version!(ThinCheck);
+test_rejects_bad_option!(ThinCheck);
+
+test_missing_input_arg!(ThinCheck);
+test_input_file_not_found!(ThinCheck);
+test_input_cannot_be_a_directory!(ThinCheck);
+test_unreadable_input_file!(ThinCheck);
+
+test_help_message_for_tiny_input_file!(ThinCheck);
+test_spot_xml_data!(ThinCheck);
+test_corrupted_input_data!(ThinCheck);
+
+//------------------------------------------
+// test exclusive flags
+
+fn accepts_flag(flag: &str) -> Result<()> {
+    let mut td = TestDir::new()?;
+    let md = mk_valid_md(&mut td)?;
+    let md_path = md.to_str().unwrap();
+    run_ok(THIN_CHECK, &[flag, md_path])?;
     Ok(())
 }
 
@@ -67,86 +114,74 @@ fn accepts_clear_needs_check_flag() -> Result<()> {
     accepts_flag("--clear-needs-check-flag")
 }
 
+#[test]
+fn accepts_auto_repair() -> Result<()> {
+    accepts_flag("--auto-repair")
+}
+
+//------------------------------------------
+// test the --quiet flag
+
 #[test]
 fn accepts_quiet() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
+    let md_path = md.to_str().unwrap();
 
-    let output = thin_check!("--quiet", &md).run()?;
+    let output = run_ok_raw(THIN_CHECK, &["--quiet", md_path])?;
-    assert!(output.status.success());
     assert_eq!(output.stdout.len(), 0);
     assert_eq!(output.stderr.len(), 0);
     Ok(())
 }
 
-#[test]
+//------------------------------------------
-fn accepts_auto_repair() -> Result<()> {
+// test superblock-block-only
-    accepts_flag("--auto-repair")
-}
 
 #[test]
 fn detects_corrupt_superblock_with_superblock_only() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_zeroed_md(&mut td)?;
-    let output = thin_check!("--super-block-only", &md).unchecked().run()?;
+    let md_path = md.to_str().unwrap();
-    assert!(!output.status.success());
+    let _stderr = run_fail(THIN_CHECK, &["--super-block-only", md_path])?;
     Ok(())
 }
 
-#[test]
+//------------------------------------------
-fn prints_help_message_for_tiny_metadata() -> Result<()> {
+// test info outputs
-    let mut td = TestDir::new()?;
-    let md = td.mk_path("meta.bin");
-    let _file = file_utils::create_sized_file(&md, 1024);
-    let stderr = run_fail(thin_check!(&md))?;
-    assert!(stderr.contains("Metadata device/file too small.  Is this binary metadata?"));
-    Ok(())
-}
-
-#[test]
-fn spot_xml_data() -> Result<()> {
-    let mut td = TestDir::new()?;
-    let xml = td.mk_path("meta.xml");
-
-    let mut gen = FragmentedS::new(4, 10240);
-    write_xml(&xml, &mut gen)?;
-
-    let stderr = run_fail(thin_check!(&xml))?;
-    eprintln!("{}", stderr);
-    assert!(
-        stderr.contains("This looks like XML.  thin_check only checks the binary metadata format.")
-    );
-    Ok(())
-}
 
 #[test]
 fn prints_info_fields() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
-    let stdout = thin_check!(&md).read()?;
+    let md_path = md.to_str().unwrap();
+    let stdout = run_ok(THIN_CHECK, &[md_path])?;
     assert!(stdout.contains("TRANSACTION_ID="));
     assert!(stdout.contains("METADATA_FREE_BLOCKS="));
     Ok(())
 }
 
+//------------------------------------------
+// test compatibility between options
+
 #[test]
 fn auto_repair_incompatible_opts() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
-    run_fail(thin_check!("--auto-repair", "-m", &md))?;
+    let md_path = md.to_str().unwrap();
-    run_fail(thin_check!(
+    run_fail(THIN_CHECK, &["--auto-repair", "-m", md_path])?;
-        "--auto-repair",
+    run_fail(
-        "--override-mapping-root",
+        THIN_CHECK,
-        "123",
+        &["--auto-repair", "--override-mapping-root", "123", md_path],
-        &md
+    )?;
-    ))?;
+    run_fail(
-    run_fail(thin_check!("--auto-repair", "--super-block-only", &md))?;
+        THIN_CHECK,
-    run_fail(thin_check!("--auto-repair", "--skip-mappings", &md))?;
+        &["--auto-repair", "--super-block-only", md_path],
-    run_fail(thin_check!(
+    )?;
-        "--auto-repair",
+    run_fail(THIN_CHECK, &["--auto-repair", "--skip-mappings", md_path])?;
-        "--ignore-non-fatal-errors",
+    run_fail(
-        &md
+        THIN_CHECK,
-    ))?;
+        &["--auto-repair", "--ignore-non-fatal-errors", md_path],
+    )?;
     Ok(())
 }
 
@@ -154,36 +189,45 @@ fn auto_repair_incompatible_opts() -> Result<()> {
 fn clear_needs_check_incompatible_opts() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
-    run_fail(thin_check!("--clear-needs-check-flag", "-m", &md))?;
+    let md_path = md.to_str().unwrap();
-    run_fail(thin_check!(
+    run_fail(THIN_CHECK, &["--clear-needs-check-flag", "-m", md_path])?;
+    run_fail(
+        THIN_CHECK,
+        &[
             "--clear-needs-check-flag",
             "--override-mapping-root",
             "123",
-        &md
+            md_path,
-    ))?;
+        ],
-    run_fail(thin_check!(
+    )?;
-        "--clear-needs-check-flag",
+    run_fail(
-        "--super-block-only",
+        THIN_CHECK,
-        &md
+        &["--clear-needs-check-flag", "--super-block-only", md_path],
-    ))?;
+    )?;
-    run_fail(thin_check!(
+    run_fail(
+        THIN_CHECK,
+        &[
             "--clear-needs-check-flag",
             "--ignore-non-fatal-errors",
-        &md
+            md_path,
-    ))?;
+        ],
+    )?;
     Ok(())
 }
 
 //------------------------------------------
+// test clear-needs-check
 
 #[test]
 fn clear_needs_check() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = prep_metadata(&mut td)?;
+    let md_path = md.to_str().unwrap();
+
     set_needs_check(&md)?;
 
     assert!(get_needs_check(&md)?);
-    thin_check!("--clear-needs-check-flag", &md).run()?;
+    run_ok(THIN_CHECK, &["--clear-needs-check-flag", md_path])?;
     assert!(!get_needs_check(&md)?);
     Ok(())
 }
@@ -192,9 +236,11 @@ fn clear_needs_check() -> Result<()> {
 fn no_clear_needs_check_if_error() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = prep_metadata(&mut td)?;
+    let md_path = md.to_str().unwrap();
+
     set_needs_check(&md)?;
     generate_metadata_leaks(&md, 1, 0, 1)?;
-    run_fail(thin_check!("--clear-needs-check-flag", &md))?;
+    run_fail(THIN_CHECK, &["--clear-needs-check-flag", md_path])?;
     assert!(get_needs_check(&md)?);
     Ok(())
 }
@@ -203,22 +249,31 @@ fn no_clear_needs_check_if_error() -> Result<()> {
 fn clear_needs_check_if_skip_mappings() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = prep_metadata(&mut td)?;
+    let md_path = md.to_str().unwrap();
+
     set_needs_check(&md)?;
     generate_metadata_leaks(&md, 1, 0, 1)?;
 
     assert!(get_needs_check(&md)?);
-    thin_check!("--clear-needs-check-flag", "--skip-mappings", &md).run()?;
+    run_ok(
+        THIN_CHECK,
+        &["--clear-needs-check-flag", "--skip-mappings", md_path],
+    )?;
     assert!(!get_needs_check(&md)?);
     Ok(())
 }
 
+//------------------------------------------
+// test ignore-non-fatal-errors
+
 #[test]
 fn metadata_leaks_are_non_fatal() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = prep_metadata(&mut td)?;
+    let md_path = md.to_str().unwrap();
     generate_metadata_leaks(&md, 1, 0, 1)?;
-    run_fail(thin_check!(&md))?;
+    run_fail(THIN_CHECK, &[md_path])?;
-    thin_check!("--ignore-non-fatal-errors", &md).run()?;
+    run_ok(THIN_CHECK, &["--ignore-non-fatal-errors", md_path])?;
     Ok(())
 }
 
@@ -226,28 +281,34 @@ fn metadata_leaks_are_non_fatal() -> Result<()> {
 fn fatal_errors_cant_be_ignored() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = prep_metadata(&mut td)?;
+    let md_path = md.to_str().unwrap();
+
     generate_metadata_leaks(&md, 1, 1, 0)?;
     ensure_untouched(&md, || {
-        run_fail(thin_check!("--ignore-non-fatal-errors", &md))?;
+        run_fail(THIN_CHECK, &["--ignore-non-fatal-errors", md_path])?;
         Ok(())
     })
 }
 
+//------------------------------------------
+// test auto-repair
+
 #[test]
 fn auto_repair() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = prep_metadata(&mut td)?;
+    let md_path = md.to_str().unwrap();
 
     // auto-repair should have no effect on good metadata.
     ensure_untouched(&md, || {
-        thin_check!("--auto-repair", &md).run()?;
+        run_ok(THIN_CHECK, &["--auto-repair", md_path])?;
         Ok(())
     })?;
 
     generate_metadata_leaks(&md, 16, 0, 1)?;
-    run_fail(thin_check!(&md))?;
+    run_fail(THIN_CHECK, &[md_path])?;
-    thin_check!("--auto-repair", &md).run()?;
+    run_ok(THIN_CHECK, &["--auto-repair", md_path])?;
-    thin_check!(&md).run()?;
+    run_ok(THIN_CHECK, &[md_path])?;
     Ok(())
 }
 
@@ -255,10 +316,11 @@ fn auto_repair() -> Result<()> {
 fn auto_repair_has_limits() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = prep_metadata(&mut td)?;
-    generate_metadata_leaks(&md, 16, 1, 0)?;
+    let md_path = md.to_str().unwrap();
 
+    generate_metadata_leaks(&md, 16, 1, 0)?;
     ensure_untouched(&md, || {
-        run_fail(thin_check!("--auto-repair", &md))?;
+        run_fail(THIN_CHECK, &["--auto-repair", md_path])?;
         Ok(())
     })?;
     Ok(())
@@ -268,8 +330,10 @@ fn auto_repair_has_limits() -> Result<()> {
 fn auto_repair_clears_needs_check() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = prep_metadata(&mut td)?;
+    let md_path = md.to_str().unwrap();
+
     set_needs_check(&md)?;
-    thin_check!("--auto-repair", &md).run()?;
+    run_ok(THIN_CHECK, &["--auto-repair", md_path])?;
     assert!(!get_needs_check(&md)?);
     Ok(())
 }

tests/thin_delta.rs

@@ -1,54 +1,61 @@
 use anyhow::Result;
-use thinp::version::tools_version;
 
 mod common;
+
+use common::common_args::*;
 use common::test_dir::*;
 use common::*;
 
 //------------------------------------------
 
-#[test]
+const USAGE: &str = "Usage: thin_delta [options] <device or file>\n\
-fn accepts_v() -> Result<()> {
+                     Options:\n  \
-    let stdout = thin_delta!("-V").read()?;
+                       {--thin1, --snap1, --root1}\n  \
-    assert!(stdout.contains(tools_version()));
+                       {--thin2, --snap2, --root2}\n  \
-    Ok(())
+                       {-m, --metadata-snap} [block#]\n  \
+                       {--verbose}\n  \
+                       {-h|--help}\n  \
+                       {-V|--version}";
+
+//------------------------------------------
+
+struct ThinDelta;
+
+impl<'a> Program<'a> for ThinDelta {
+    fn name() -> &'a str {
+        "thin_delta"
     }
 
-#[test]
+    fn path() -> &'a str {
-fn accepts_version() -> Result<()> {
+        THIN_DELTA
-    let stdout = thin_delta!("--version").read()?;
-    assert!(stdout.contains(tools_version()));
-    Ok(())
     }
 
-const USAGE: &str = "Usage: thin_delta [options] <device or file>\nOptions:\n  {--thin1, --snap1, --root1}\n  {--thin2, --snap2, --root2}\n  {-m, --metadata-snap} [block#]\n  {--verbose}\n  {-h|--help}\n  {-V|--version}";
+    fn usage() -> &'a str {
-
+        USAGE
-#[test]
-fn accepts_h() -> Result<()> {
-    let stdout = thin_delta!("-h").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
-#[test]
+    fn arg_type() -> ArgType {
-fn accepts_help() -> Result<()> {
+        ArgType::InputArg
-    let stdout = thin_delta!("--help").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
-#[test]
+    fn bad_option_hint(option: &str) -> String {
-fn rejects_bad_option() -> Result<()> {
+        cpp_msg::bad_option_hint(option)
-    let stderr = run_fail(thin_delta!("--hedgehogs-only"))?;
-    assert!(stderr.contains("unrecognized option \'--hedgehogs-only\'"));
-    Ok(())
     }
+}
+
+//------------------------------------------
+
+test_accepts_help!(ThinDelta);
+test_accepts_version!(ThinDelta);
+test_rejects_bad_option!(ThinDelta);
+
+//------------------------------------------
 
 #[test]
 fn snap1_unspecified() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
-    let stderr = run_fail(thin_delta!("--snap2", "45", &md))?;
+    let stderr = run_fail(THIN_DELTA, &["--snap2", "45", md.to_str().unwrap()])?;
     assert!(stderr.contains("--snap1 or --root1 not specified"));
     Ok(())
 }
@@ -57,15 +64,17 @@ fn snap1_unspecified() -> Result<()> {
 fn snap2_unspecified() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
-    let stderr = run_fail(thin_delta!("--snap1", "45", &md))?;
+    let stderr = run_fail(THIN_DELTA, &["--snap1", "45", md.to_str().unwrap()])?;
     assert!(stderr.contains("--snap2 or --root2 not specified"));
     Ok(())
 }
 
 #[test]
 fn dev_unspecified() -> Result<()> {
-    let stderr = run_fail(thin_delta!("--snap1", "45", "--snap2", "46"))?;
+    let stderr = run_fail(THIN_DELTA, &["--snap1", "45", "--snap2", "46"])?;
     // TODO: replace with msg::MISSING_INPUT_ARG once the rust version is ready
     assert!(stderr.contains("No input file provided"));
     Ok(())
 }
+
+//------------------------------------------

tests/thin_dump.rs

@@ -2,29 +2,92 @@ use anyhow::Result;
 use std::fs::OpenOptions;
 use std::io::Write;
 use std::str::from_utf8;
-use thinp::file_utils;
 
 mod common;
+
+use common::common_args::*;
+use common::input_arg::*;
 use common::test_dir::*;
 use common::*;
 
 //------------------------------------------
 
-#[test]
+const USAGE: &str = "Usage: thin_dump [options] {device|file}\n\
-fn small_input_file() -> Result<()> {
+                     Options:\n  \
-    let mut td = TestDir::new()?;
+                       {-h|--help}\n  \
-    let md = td.mk_path("meta.bin");
+                       {-f|--format} {xml|human_readable|custom}\n  \
-    file_utils::create_sized_file(&md, 512)?;
+                       {-r|--repair}\n  \
-    let _stderr = run_fail(thin_dump!(&md))?;
+                       {-m|--metadata-snap} [block#]\n  \
-    Ok(())
+                       {-o <xml file>}\n  \
+                       {--dev-id} <dev-id>\n  \
+                       {--skip-mappings}\n  \
+                       {-V|--version}";
+
+//-----------------------------------------
+
+struct ThinDump;
+
+impl<'a> Program<'a> for ThinDump {
+    fn name() -> &'a str {
+        "thin_dump"
     }
 
+    fn path() -> &'a str {
+        THIN_DUMP
+    }
+
+    fn usage() -> &'a str {
+        USAGE
+    }
+
+    fn arg_type() -> ArgType {
+        ArgType::InputArg
+    }
+
+    fn bad_option_hint(option: &str) -> String {
+        msg::bad_option_hint(option)
+    }
+}
+
+impl<'a> InputProgram<'a> for ThinDump {
+    fn mk_valid_input(td: &mut TestDir) -> Result<std::path::PathBuf> {
+        mk_valid_md(td)
+    }
+
+    fn file_not_found() -> &'a str {
+        msg::FILE_NOT_FOUND
+    }
+
+    fn missing_input_arg() -> &'a str {
+        msg::MISSING_INPUT_ARG
+    }
+
+    fn corrupted_input() -> &'a str {
+        msg::BAD_SUPERBLOCK
+    }
+}
+
+//------------------------------------------
+
+test_accepts_help!(ThinDump);
+test_accepts_version!(ThinDump);
+test_rejects_bad_option!(ThinDump);
+
+test_missing_input_arg!(ThinDump);
+test_input_file_not_found!(ThinDump);
+test_input_cannot_be_a_directory!(ThinDump);
+test_unreadable_input_file!(ThinDump);
+
+//------------------------------------------
+// test dump & restore cycle
+
 #[test]
 fn dump_restore_cycle() -> Result<()> {
     let mut td = TestDir::new()?;
 
     let md = mk_valid_md(&mut td)?;
-    let output = thin_dump!(&md).run()?;
+    let md_path = md.to_str().unwrap();
+    let output = run_ok_raw(THIN_DUMP, &[md_path])?;
 
     let xml = td.mk_path("meta.xml");
     let mut file = OpenOptions::new()
@@ -36,29 +99,40 @@ fn dump_restore_cycle() -> Result<()> {
     drop(file);
 
     let md2 = mk_zeroed_md(&mut td)?;
-    thin_restore!("-i", &xml, "-o", &md2).run()?;
+    let md2_path = md2.to_str().unwrap();
+    let xml_path = xml.to_str().unwrap();
+    run_ok(THIN_RESTORE, &["-i", xml_path, "-o", md2_path])?;
 
-    let output2 = thin_dump!(&md2).run()?;
+    let output2 = run_ok_raw(THIN_DUMP, &[md2_path])?;
     assert_eq!(output.stdout, output2.stdout);
 
     Ok(())
 }
 
+//------------------------------------------
+// test no stderr with a normal dump
+
 #[test]
 fn no_stderr() -> Result<()> {
     let mut td = TestDir::new()?;
 
     let md = mk_valid_md(&mut td)?;
-    let output = thin_dump!(&md).run()?;
+    let md_path = md.to_str().unwrap();
+    let output = run_ok_raw(THIN_DUMP, &[md_path])?;
 
     assert_eq!(output.stderr.len(), 0);
     Ok(())
 }
 
+//------------------------------------------
+// test superblock overriding & repair
+// TODO: share with thin_repair
+
 fn override_something(flag: &str, value: &str, pattern: &str) -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
-    let output = thin_dump!(&md, flag, value).run()?;
+    let md_path = md.to_str().unwrap();
+    let output = run_ok_raw(THIN_DUMP, &[md_path, flag, value])?;
 
     assert_eq!(output.stderr.len(), 0);
     assert!(from_utf8(&output.stdout[0..])?.contains(pattern));
@@ -80,44 +154,56 @@ fn override_nr_data_blocks() -> Result<()> {
     override_something("--nr-data-blocks", "234500", "nr_data_blocks=\"234500\"")
 }
 
+// FIXME: duplicate with superblock_succeeds in thin_repair.rs
 #[test]
 fn repair_superblock() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
-    let before = thin_dump!(
+    let before = run_ok_raw(
+        THIN_DUMP,
+        &[
             "--transaction-id=5",
             "--data-block-size=128",
             "--nr-data-blocks=4096000",
-        &md
+            md.to_str().unwrap(),
-    )
+        ],
-    .run()?;
+    )?;
     damage_superblock(&md)?;
 
-    let after = thin_dump!(
+    let after = run_ok_raw(
+        THIN_DUMP,
+        &[
             "--repair",
             "--transaction-id=5",
             "--data-block-size=128",
             "--nr-data-blocks=4096000",
-        &md
+            md.to_str().unwrap(),
-    )
+        ],
-    .run()?;
+    )?;
     assert_eq!(after.stderr.len(), 0);
     assert_eq!(before.stdout, after.stdout);
 
     Ok(())
 }
 
+//------------------------------------------
+// test compatibility between options
+// TODO: share with thin_repair
+
 #[test]
 fn missing_transaction_id() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
     damage_superblock(&md)?;
-    let stderr = run_fail(thin_dump!(
+    let stderr = run_fail(
+        THIN_DUMP,
+        &[
             "--repair",
             "--data-block-size=128",
             "--nr-data-blocks=4096000",
-        &md
+            md.to_str().unwrap(),
-    ))?;
+        ],
+    )?;
     assert!(stderr.contains("transaction id"));
     Ok(())
 }
@@ -127,12 +213,15 @@ fn missing_data_block_size() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
     damage_superblock(&md)?;
-    let stderr = run_fail(thin_dump!(
+    let stderr = run_fail(
+        THIN_DUMP,
+        &[
             "--repair",
             "--transaction-id=5",
             "--nr-data-blocks=4096000",
-        &md
+            md.to_str().unwrap(),
-    ))?;
+        ],
+    )?;
     assert!(stderr.contains("data block size"));
     Ok(())
 }
@@ -142,12 +231,17 @@ fn missing_nr_data_blocks() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
     damage_superblock(&md)?;
-    let stderr = run_fail(thin_dump!(
+    let stderr = run_fail(
+        THIN_DUMP,
+        &[
             "--repair",
             "--transaction-id=5",
             "--data-block-size=128",
-        &md
+            md.to_str().unwrap(),
-    ))?;
+        ],
+    )?;
     assert!(stderr.contains("nr data blocks"));
     Ok(())
 }
+
+//------------------------------------------

tests/thin_metadata_pack.rs

@@ -1,77 +1,94 @@
 use anyhow::Result;
-use thinp::version::tools_version;
 
 mod common;
+
+use common::common_args::*;
+use common::input_arg::*;
+use common::output_option::*;
 use common::test_dir::*;
 use common::*;
 
 //------------------------------------------
 
-#[test]
+const USAGE: &str = concat!(
-fn accepts_v() -> Result<()> {
+    "thin_metadata_pack ",
-    let stdout = thin_metadata_pack!("-V").read()?;
+    include_str!("../VERSION"),
-    assert!(stdout.contains(tools_version()));
+    "Produces a compressed file of thin metadata.  Only packs metadata blocks that are actually used.\n\
-    Ok(())
+     \n\
-}
+     USAGE:\n    \
-
+         thin_metadata_pack -i <DEV> -o <FILE>\n\
-#[test]
+     \n\
-fn accepts_version() -> Result<()> {
+     FLAGS:\n    \
-    let stdout = thin_metadata_pack!("--version").read()?;
+         -h, --help       Prints help information\n    \
-    assert!(stdout.contains(tools_version()));
+         -V, --version    Prints version information\n\
-    Ok(())
+     \n\
-}
+     OPTIONS:\n    \
-
+         -i <DEV>         Specify thinp metadata binary device/file\n    \
-const USAGE: &str = "thin_metadata_pack 0.9.0\nProduces a compressed file of thin metadata.  Only packs metadata blocks that are actually used.\n\nUSAGE:\n    thin_metadata_pack -i <DEV> -o <FILE>\n\nFLAGS:\n    -h, --help       Prints help information\n    -V, --version    Prints version information\n\nOPTIONS:\n    -i <DEV>         Specify thinp metadata binary device/file\n    -o <FILE>        Specify packed output file";
+         -o <FILE>        Specify packed output file"
-
-#[test]
-fn accepts_h() -> Result<()> {
-    let stdout = thin_metadata_pack!("-h").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
-}
-
-#[test]
-fn accepts_help() -> Result<()> {
-    let stdout = thin_metadata_pack!("--help").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
-}
-
-#[test]
-fn rejects_bad_option() -> Result<()> {
-    let stderr = run_fail(thin_metadata_pack!("--hedgehogs-only"))?;
-    assert!(stderr.contains("Found argument \'--hedgehogs-only\'"));
-    Ok(())
-}
-
-#[test]
-fn missing_input_file() -> Result<()> {
-    let mut td = TestDir::new()?;
-    let md = mk_zeroed_md(&mut td)?;
-    let stderr = run_fail(thin_metadata_pack!("-o", &md))?;
-    assert!(
-        stderr.contains("error: The following required arguments were not provided:\n    -i <DEV>")
 );
-    Ok(())
+
+//------------------------------------------
+
+struct ThinMetadataPack;
+
+impl<'a> Program<'a> for ThinMetadataPack {
+    fn name() -> &'a str {
+        "thin_metadata_pack"
     }
 
-#[test]
+    fn path() -> &'a str {
-fn no_such_input_file() -> Result<()> {
+        THIN_METADATA_PACK
-    let mut td = TestDir::new()?;
-    let md = mk_zeroed_md(&mut td)?;
-    let stderr = run_fail(thin_metadata_pack!("-i", "no-such-file", "-o", &md))?;
-    assert!(stderr.contains("Couldn't find input file"));
-    Ok(())
     }
 
-#[test]
+    fn usage() -> &'a str {
-fn missing_output_file() -> Result<()> {
+        USAGE
-    let mut td = TestDir::new()?;
+    }
-    let md = mk_zeroed_md(&mut td)?;
+
-    let stderr = run_fail(thin_metadata_pack!("-i", &md))?;
+    fn arg_type() -> ArgType {
-    assert!(stderr
+        ArgType::IoOptions
-        .contains("error: The following required arguments were not provided:\n    -o <FILE>"));
+    }
-    Ok(())
+
+    fn bad_option_hint(option: &str) -> String {
+        rust_msg::bad_option_hint(option)
+    }
+}
+
+impl<'a> InputProgram<'a> for ThinMetadataPack {
+    fn mk_valid_input(td: &mut TestDir) -> Result<std::path::PathBuf> {
+        mk_valid_md(td)
+    }
+
+    fn file_not_found() -> &'a str {
+        rust_msg::FILE_NOT_FOUND
+    }
+
+    fn missing_input_arg() -> &'a str {
+        rust_msg::MISSING_INPUT_ARG
+    }
+
+    fn corrupted_input() -> &'a str {
+        rust_msg::BAD_SUPERBLOCK
+    }
+}
+
+impl<'a> OutputProgram<'a> for ThinMetadataPack {
+    fn file_not_found() -> &'a str {
+        rust_msg::FILE_NOT_FOUND
+    }
+
+    fn missing_output_arg() -> &'a str {
+        rust_msg::MISSING_OUTPUT_ARG
+    }
 }
 
 //------------------------------------------
+
+test_accepts_help!(ThinMetadataPack);
+test_accepts_version!(ThinMetadataPack);
+test_rejects_bad_option!(ThinMetadataPack);
+
+test_missing_input_option!(ThinMetadataPack);
+test_missing_output_option!(ThinMetadataPack);
+test_input_file_not_found!(ThinMetadataPack);
+
+//-----------------------------------------

tests/thin_metadata_unpack.rs

@@ -1,98 +1,118 @@
 use anyhow::Result;
-use thinp::version::tools_version;
 
 mod common;
+
+use common::common_args::*;
+use common::input_arg::*;
+use common::output_option::*;
 use common::test_dir::*;
 use common::*;
 
 //------------------------------------------
 
-#[test]
+const USAGE: &str = concat!(
-fn accepts_v() -> Result<()> {
+    "thin_metadata_unpack ",
-    let stdout = thin_metadata_unpack!("-V").read()?;
+    include_str!("../VERSION"),
-    assert!(stdout.contains(tools_version()));
+    "Unpack a compressed file of thin metadata.\n\
-    Ok(())
+     \n\
-}
+     USAGE:\n    \
-
+         thin_metadata_unpack -i <DEV> -o <FILE>\n\
-#[test]
+     \n\
-fn accepts_version() -> Result<()> {
+     FLAGS:\n    \
-    let stdout = thin_metadata_unpack!("--version").read()?;
+         -h, --help       Prints help information\n    \
-    assert!(stdout.contains(tools_version()));
+         -V, --version    Prints version information\n\
-    Ok(())
+     \n\
-}
+     OPTIONS:\n    \
-
+         -i <DEV>         Specify thinp metadata binary device/file\n    \
-const USAGE: &str = "thin_metadata_unpack 0.9.0\nUnpack a compressed file of thin metadata.\n\nUSAGE:\n    thin_metadata_unpack -i <DEV> -o <FILE>\n\nFLAGS:\n    -h, --help       Prints help information\n    -V, --version    Prints version information\n\nOPTIONS:\n    -i <DEV>         Specify thinp metadata binary device/file\n    -o <FILE>        Specify packed output file";
+         -o <FILE>        Specify packed output file"
-
-#[test]
-fn accepts_h() -> Result<()> {
-    let stdout = thin_metadata_unpack!("-h").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
-}
-
-#[test]
-fn accepts_help() -> Result<()> {
-    let stdout = thin_metadata_unpack!("--help").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
-}
-
-#[test]
-fn rejects_bad_option() -> Result<()> {
-    let stderr = run_fail(thin_metadata_unpack!("--hedgehogs-only"))?;
-    assert!(stderr.contains("Found argument \'--hedgehogs-only\'"));
-    Ok(())
-}
-
-#[test]
-fn missing_input_file() -> Result<()> {
-    let mut td = TestDir::new()?;
-    let md = mk_zeroed_md(&mut td)?;
-    let stderr = run_fail(thin_metadata_unpack!("-o", &md))?;
-    assert!(
-        stderr.contains("error: The following required arguments were not provided:\n    -i <DEV>")
 );
-    Ok(())
+
+//------------------------------------------
+
+struct ThinMetadataUnpack;
+
+impl<'a> Program<'a> for ThinMetadataUnpack {
+    fn name() -> &'a str {
+        "thin_metadata_pack"
     }
 
-#[test]
+    fn path() -> &'a str {
-fn no_such_input_file() -> Result<()> {
+        THIN_METADATA_UNPACK
-    let mut td = TestDir::new()?;
-    let md = mk_zeroed_md(&mut td)?;
-    let stderr = run_fail(thin_metadata_unpack!("-i", "no-such-file", "-o", &md))?;
-    assert!(stderr.contains("Couldn't find input file"));
-    Ok(())
     }
 
-#[test]
+    fn usage() -> &'a str {
-fn missing_output_file() -> Result<()> {
+        USAGE
-    let mut td = TestDir::new()?;
-    let md = mk_zeroed_md(&mut td)?;
-    let stderr = run_fail(thin_metadata_unpack!("-i", &md))?;
-    assert!(stderr
-        .contains("error: The following required arguments were not provided:\n    -o <FILE>"));
-    Ok(())
     }
 
-#[test]
+    fn arg_type() -> ArgType {
-fn garbage_input_file() -> Result<()> {
+        ArgType::IoOptions
-    let mut td = TestDir::new()?;
-    let md = mk_zeroed_md(&mut td)?;
-    let stderr = run_fail(thin_metadata_unpack!("-i", &md, "-o", "junk"))?;
-    assert!(stderr.contains("Not a pack file."));
-    Ok(())
     }
 
+    fn bad_option_hint(option: &str) -> String {
+        rust_msg::bad_option_hint(option)
+    }
+}
+
+impl<'a> InputProgram<'a> for ThinMetadataUnpack {
+    fn mk_valid_input(td: &mut TestDir) -> Result<std::path::PathBuf> {
+        mk_zeroed_md(td) // FIXME: make a real pack file
+    }
+
+    fn file_not_found() -> &'a str {
+        rust_msg::FILE_NOT_FOUND
+    }
+
+    fn missing_input_arg() -> &'a str {
+        rust_msg::MISSING_INPUT_ARG
+    }
+
+    fn corrupted_input() -> &'a str {
+        "Not a pack file"
+    }
+}
+
+impl<'a> OutputProgram<'a> for ThinMetadataUnpack {
+    fn file_not_found() -> &'a str {
+        rust_msg::FILE_NOT_FOUND
+    }
+
+    fn missing_output_arg() -> &'a str {
+        rust_msg::MISSING_OUTPUT_ARG
+    }
+}
+
+//------------------------------------------
+
+test_accepts_help!(ThinMetadataUnpack);
+test_accepts_version!(ThinMetadataUnpack);
+test_rejects_bad_option!(ThinMetadataUnpack);
+
+test_missing_input_option!(ThinMetadataUnpack);
+test_input_file_not_found!(ThinMetadataUnpack);
+test_corrupted_input_data!(ThinMetadataUnpack);
+
+test_missing_output_option!(ThinMetadataUnpack);
+
+//------------------------------------------
+
+// TODO: share with thin_restore/cache_restore/era_restore
+
 #[test]
 fn end_to_end() -> Result<()> {
     let mut td = TestDir::new()?;
     let md_in = mk_valid_md(&mut td)?;
     let md_out = mk_zeroed_md(&mut td)?;
-    thin_metadata_pack!("-i", &md_in, "-o", "meta.pack").run()?;
+    run_ok(
-    thin_metadata_unpack!("-i", "meta.pack", "-o", &md_out).run()?;
+        THIN_METADATA_PACK,
+        &["-i", md_in.to_str().unwrap(), "-o", "meta.pack"],
+    )?;
+    run_ok(
+        THIN_METADATA_UNPACK,
+        &["-i", "meta.pack", "-o", md_out.to_str().unwrap()],
+    )?;
 
-    let dump1 = thin_dump!(&md_in).read()?;
+    let dump1 = run_ok(THIN_DUMP, &[md_in.to_str().unwrap()])?;
-    let dump2 = thin_dump!(&md_out).read()?;
+    let dump2 = run_ok(THIN_DUMP, &[md_out.to_str().unwrap()])?;
     assert_eq!(dump1, dump2);
     Ok(())
 }

tests/thin_repair.rs

@@ -1,91 +1,122 @@
 use anyhow::Result;
-use std::str::from_utf8;
-use thinp::version::tools_version;
 
 mod common;
+
+use common::common_args::*;
+use common::input_arg::*;
+use common::output_option::*;
 use common::test_dir::*;
 use common::*;
 
 //------------------------------------------
 
-#[test]
+const USAGE: &str = "Usage: thin_repair [options] {device|file}\n\
-fn accepts_v() -> Result<()> {
+                     Options:\n  \
-    let stdout = thin_repair!("-V").read()?;
+                       {-h|--help}\n  \
-    assert!(stdout.contains(tools_version()));
+                       {-i|--input} <input metadata (binary format)>\n  \
-    Ok(())
+                       {-o|--output} <output metadata (binary format)>\n  \
+                       {--transaction-id} <natural>\n  \
+                       {--data-block-size} <natural>\n  \
+                       {--nr-data-blocks} <natural>\n  \
+                       {-V|--version}";
+
+//-----------------------------------------
+
+struct ThinRepair;
+
+impl<'a> Program<'a> for ThinRepair {
+    fn name() -> &'a str {
+        "thin_repair"
     }
 
-#[test]
+    fn path() -> &'a str {
-fn accepts_version() -> Result<()> {
+        THIN_REPAIR
-    let stdout = thin_repair!("--version").read()?;
-    assert!(stdout.contains(tools_version()));
-    Ok(())
     }
 
-const USAGE: &str = "Usage: thin_repair [options] {device|file}\nOptions:\n  {-h|--help}\n  {-i|--input} <input metadata (binary format)>\n  {-o|--output} <output metadata (binary format)>\n  {--transaction-id} <natural>\n  {--data-block-size} <natural>\n  {--nr-data-blocks} <natural>\n  {-V|--version}";
+    fn usage() -> &'a str {
-
+        USAGE
-#[test]
-fn accepts_h() -> Result<()> {
-    let stdout = thin_repair!("-h").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
-#[test]
+    fn arg_type() -> ArgType {
-fn accepts_help() -> Result<()> {
+        ArgType::IoOptions
-    let stdout = thin_repair!("--help").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
+    fn bad_option_hint(option: &str) -> String {
+        cpp_msg::bad_option_hint(option)
+    }
+}
+
+impl<'a> InputProgram<'a> for ThinRepair {
+    fn mk_valid_input(td: &mut TestDir) -> Result<std::path::PathBuf> {
+        mk_valid_md(td)
+    }
+
+    fn file_not_found() -> &'a str {
+        cpp_msg::FILE_NOT_FOUND
+    }
+
+    fn missing_input_arg() -> &'a str {
+        cpp_msg::MISSING_INPUT_ARG
+    }
+
+    fn corrupted_input() -> &'a str {
+        "The following field needs to be provided on the command line due to corruption in the superblock"
+    }
+}
+
+impl<'a> OutputProgram<'a> for ThinRepair {
+    fn file_not_found() -> &'a str {
+        cpp_msg::FILE_NOT_FOUND
+    }
+
+    fn missing_output_arg() -> &'a str {
+        cpp_msg::MISSING_OUTPUT_ARG
+    }
+}
+
+//-----------------------------------------
+
+test_accepts_help!(ThinRepair);
+test_accepts_version!(ThinRepair);
+test_rejects_bad_option!(ThinRepair);
+
+test_input_file_not_found!(ThinRepair);
+test_input_cannot_be_a_directory!(ThinRepair);
+test_corrupted_input_data!(ThinRepair);
+
+test_missing_output_option!(ThinRepair);
+
+//-----------------------------------------
+// test output to a small file
+
+// TODO: share with thin_restore
+
 #[test]
 fn dont_repair_xml() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_zeroed_md(&mut td)?;
     let xml = mk_valid_xml(&mut td)?;
-    run_fail(thin_repair!("-i", &xml, "-o", &md))?;
+    let xml_path = xml.to_str().unwrap();
+    let md_path = md.to_str().unwrap();
+    run_fail(THIN_REPAIR, &["-i", xml_path, "-o", md_path])?;
     Ok(())
 }
 
-#[test]
+//-----------------------------------------
-fn missing_input_file() -> Result<()> {
-    let mut td = TestDir::new()?;
-    let md = mk_zeroed_md(&mut td)?;
-    let stderr = run_fail(thin_repair!("-i", "no-such-file", "-o", &md))?;
-    assert!(superblock_all_zeroes(&md)?);
-    // TODO: replace with msg::FILE_NOT_FOUND once the rust version is ready
-    assert!(stderr.contains("Couldn't stat file"));
-    Ok(())
-}
 
-#[test]
+// TODO: share with thin_dump
-fn garbage_input_file() -> Result<()> {
-    let mut td = TestDir::new()?;
-    let md = mk_zeroed_md(&mut td)?;
-    let md2 = mk_zeroed_md(&mut td)?;
-    run_fail(thin_repair!("-i", &md, "-o", &md2))?;
-    assert!(superblock_all_zeroes(&md2)?);
-    Ok(())
-}
-
-#[test]
-fn missing_output_file() -> Result<()> {
-    let mut td = TestDir::new()?;
-    let md = mk_valid_md(&mut td)?;
-    let stderr = run_fail(thin_repair!("-i", &md))?;
-    // TODO: replace with msg::MISSING_OUTPUT_ARG once the rust version is ready
-    assert!(stderr.contains("No output file provided."));
-    Ok(())
-}
 
 fn override_thing(flag: &str, val: &str, pattern: &str) -> Result<()> {
     let mut td = TestDir::new()?;
     let md1 = mk_valid_md(&mut td)?;
     let md2 = mk_zeroed_md(&mut td)?;
-    let output = thin_repair!(flag, val, "-i", &md1, "-o", &md2).run()?;
+    let md1_path = md1.to_str().unwrap();
+    let md2_path = md2.to_str().unwrap();
+    let output = run_ok_raw(THIN_REPAIR, &[flag, val, "-i", md1_path, "-o", md2_path])?;
     assert_eq!(output.stderr.len(), 0);
-    let output = thin_dump!(&md2).run()?;
+    let md2_path = md2.to_str().unwrap();
-    assert!(from_utf8(&output.stdout[0..])?.contains(pattern));
+    let output = run_ok(THIN_DUMP, &[md2_path])?;
+    assert!(output.contains(pattern));
     Ok(())
 }
 
@@ -104,42 +135,63 @@ fn override_nr_data_blocks() -> Result<()> {
     override_thing("--nr-data-blocks", "234500", "nr_data_blocks=\"234500\"")
 }
 
+// FIXME: that's repair_superblock in thin_dump.rs
 #[test]
 fn superblock_succeeds() -> Result<()> {
     let mut td = TestDir::new()?;
     let md1 = mk_valid_md(&mut td)?;
-    let original = thin_dump!(
+    let md1_path = md1.to_str().unwrap();
+    let original = run_ok_raw(
+        THIN_DUMP,
+        &[
             "--transaction-id=5",
             "--data-block-size=128",
             "--nr-data-blocks=4096000",
-        &md1
+            md1_path,
-    )
+        ],
-    .run()?;
+    )?;
     assert_eq!(original.stderr.len(), 0);
     damage_superblock(&md1)?;
     let md2 = mk_zeroed_md(&mut td)?;
-    thin_repair!(
+    let md2_path = md2.to_str().unwrap();
+    run_ok(
+        THIN_REPAIR,
+        &[
             "--transaction-id=5",
             "--data-block-size=128",
             "--nr-data-blocks=4096000",
             "-i",
-        &md1,
+            md1_path,
             "-o",
-        &md2
+            md2_path,
-    )
+        ],
-    .run()?;
+    )?;
-    let repaired = thin_dump!(&md2).run()?;
+    let repaired = run_ok_raw(THIN_DUMP, &[md2_path])?;
     assert_eq!(repaired.stderr.len(), 0);
     assert_eq!(original.stdout, repaired.stdout);
     Ok(())
 }
 
+//-----------------------------------------
+
+// TODO: share with thin_dump
+
 fn missing_thing(flag1: &str, flag2: &str, pattern: &str) -> Result<()> {
     let mut td = TestDir::new()?;
     let md1 = mk_valid_md(&mut td)?;
     damage_superblock(&md1)?;
     let md2 = mk_zeroed_md(&mut td)?;
-    let stderr = run_fail(thin_repair!(flag1, flag2, "-i", &md1, "-o", &md2))?;
+    let stderr = run_fail(
+        THIN_REPAIR,
+        &[
+            flag1,
+            flag2,
+            "-i",
+            md1.to_str().unwrap(),
+            "-o",
+            md2.to_str().unwrap(),
+        ],
+    )?;
     assert!(stderr.contains(pattern));
     Ok(())
 }
@@ -170,3 +222,5 @@ fn missing_nr_data_blocks() -> Result<()> {
         "nr data blocks",
     )
 }
+
+//-----------------------------------------

tests/thin_restore.rs

@@ -1,101 +1,107 @@
 use anyhow::Result;
-use std::str::from_utf8;
-use thinp::file_utils;
-use thinp::version::tools_version;
 
 mod common;
+
+use common::common_args::*;
+use common::input_arg::*;
+use common::output_option::*;
 use common::test_dir::*;
 use common::*;
 
 //------------------------------------------
 
-#[test]
+const USAGE: &str = "Usage: thin_restore [options]\n\
-fn accepts_v() -> Result<()> {
+                     Options:\n  \
-    let stdout = thin_restore!("-V").read()?;
+                       {-h|--help}\n  \
-    assert!(stdout.contains(tools_version()));
+                       {-i|--input} <input xml file>\n  \
-    Ok(())
+                       {-o|--output} <output device or file>\n  \
+                       {--transaction-id} <natural>\n  \
+                       {--data-block-size} <natural>\n  \
+                       {--nr-data-blocks} <natural>\n  \
+                       {-q|--quiet}\n  \
+                       {-V|--version}";
+
+//------------------------------------------
+
+struct ThinRestore;
+
+impl<'a> Program<'a> for ThinRestore {
+    fn name() -> &'a str {
+        "thin_restore"
     }
 
-#[test]
+    fn path() -> &'a str {
-fn accepts_version() -> Result<()> {
+        THIN_RESTORE
-    let stdout = thin_restore!("--version").read()?;
-    assert!(stdout.contains(tools_version()));
-    Ok(())
     }
 
-const USAGE: &str = "Usage: thin_restore [options]\nOptions:\n  {-h|--help}\n  {-i|--input} <input xml file>\n  {-o|--output} <output device or file>\n  {--transaction-id} <natural>\n  {--data-block-size} <natural>\n  {--nr-data-blocks} <natural>\n  {-q|--quiet}\n  {-V|--version}";
+    fn usage() -> &'a str {
-
+        USAGE
-#[test]
-fn accepts_h() -> Result<()> {
-    let stdout = thin_restore!("-h").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
-#[test]
+    fn arg_type() -> ArgType {
-fn accepts_help() -> Result<()> {
+        ArgType::IoOptions
-    let stdout = thin_restore!("--help").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
-#[test]
+    fn bad_option_hint(option: &str) -> String {
-fn no_input_file() -> Result<()> {
+        msg::bad_option_hint(option)
-    let mut td = TestDir::new()?;
+    }
-    let md = mk_zeroed_md(&mut td)?;
-    let stderr = run_fail(thin_restore!("-o", &md))?;
-    assert!(stderr.contains(msg::MISSING_INPUT_ARG));
-    Ok(())
 }
 
-#[test]
+impl<'a> InputProgram<'a> for ThinRestore {
-fn missing_input_file() -> Result<()> {
+    fn mk_valid_input(td: &mut TestDir) -> Result<std::path::PathBuf> {
-    let mut td = TestDir::new()?;
+        mk_valid_md(td)
-    let md = mk_zeroed_md(&mut td)?;
-    let stderr = run_fail(thin_restore!("-i", "no-such-file", "-o", &md))?;
-    assert!(superblock_all_zeroes(&md)?);
-    assert!(stderr.contains(msg::FILE_NOT_FOUND));
-    Ok(())
     }
 
-#[test]
+    fn file_not_found() -> &'a str {
-fn garbage_input_file() -> Result<()> {
+        msg::FILE_NOT_FOUND
-    let mut td = TestDir::new()?;
-    let xml = mk_zeroed_md(&mut td)?;
-    let md = mk_zeroed_md(&mut td)?;
-    let _stderr = run_fail(thin_restore!("-i", &xml, "-o", &md))?;
-    assert!(superblock_all_zeroes(&md)?);
-    Ok(())
     }
 
-#[test]
+    fn missing_input_arg() -> &'a str {
-fn no_output_file() -> Result<()> {
+        msg::MISSING_INPUT_ARG
-    let mut td = TestDir::new()?;
-    let xml = mk_valid_xml(&mut td)?;
-    let stderr = run_fail(thin_restore!("-i", &xml))?;
-    assert!(stderr.contains(msg::MISSING_OUTPUT_ARG));
-    Ok(())
     }
 
-#[test]
+    fn corrupted_input() -> &'a str {
-fn tiny_output_file() -> Result<()> {
+        "" // we don't intent to verify error messages of XML parsing
-    let mut td = TestDir::new()?;
-    let xml = mk_valid_xml(&mut td)?;
-    let md = td.mk_path("meta.bin");
-    let _file = file_utils::create_sized_file(&md, 4096);
-    let stderr = run_fail(thin_restore!("-i", &xml, "-o", &md))?;
-    assert!(stderr.contains("Output file too small"));
-    Ok(())
     }
+}
+
+impl<'a> OutputProgram<'a> for ThinRestore {
+    fn file_not_found() -> &'a str {
+        msg::FILE_NOT_FOUND
+    }
+
+    fn missing_output_arg() -> &'a str {
+        msg::MISSING_OUTPUT_ARG
+    }
+}
+
+impl<'a> BinaryOutputProgram<'_> for ThinRestore {}
+
+//-----------------------------------------
+
+test_accepts_help!(ThinRestore);
+test_accepts_version!(ThinRestore);
+
+test_missing_input_option!(ThinRestore);
+test_input_file_not_found!(ThinRestore);
+test_corrupted_input_data!(ThinRestore);
+
+test_missing_output_option!(ThinRestore);
+test_tiny_output_file!(ThinRestore);
+
+//-----------------------------------------
+
+// TODO: share with cache_restore, era_restore
 
 fn quiet_flag(flag: &str) -> Result<()> {
     let mut td = TestDir::new()?;
     let xml = mk_valid_xml(&mut td)?;
     let md = mk_zeroed_md(&mut td)?;
 
-    let output = thin_restore!("-i", &xml, "-o", &md, flag).run()?;
+    let xml_path = xml.to_str().unwrap();
+    let md_path = md.to_str().unwrap();
+    let output = run_ok_raw(THIN_RESTORE, &["-i", xml_path, "-o", md_path, flag])?;
 
-    assert!(output.status.success());
     assert_eq!(output.stdout.len(), 0);
     assert_eq!(output.stderr.len(), 0);
     Ok(())
@@ -111,15 +117,21 @@ fn accepts_quiet() -> Result<()> {
     quiet_flag("--quiet")
 }
 
+//-----------------------------------------
+
+// TODO: share with thin_dump
+
 fn override_something(flag: &str, value: &str, pattern: &str) -> Result<()> {
     let mut td = TestDir::new()?;
     let xml = mk_valid_xml(&mut td)?;
     let md = mk_zeroed_md(&mut td)?;
 
-    thin_restore!("-i", &xml, "-o", &md, flag, value).run()?;
+    let xml_path = xml.to_str().unwrap();
+    let md_path = md.to_str().unwrap();
+    run_ok(THIN_RESTORE, &["-i", xml_path, "-o", md_path, flag, value])?;
 
-    let output = thin_dump!(&md).run()?;
+    let output = run_ok(THIN_DUMP, &[md_path])?;
-    assert!(from_utf8(&output.stdout)?.contains(pattern));
+    assert!(output.contains(pattern));
     Ok(())
 }
 
@@ -137,3 +149,5 @@ fn override_data_block_size() -> Result<()> {
 fn override_nr_data_blocks() -> Result<()> {
     override_something("--nr-data-blocks", "234500", "nr_data_blocks=\"234500\"")
 }
+
+//-----------------------------------------

tests/thin_rmap.rs

@@ -1,54 +1,62 @@
 use anyhow::Result;
-use thinp::version::tools_version;
 
 mod common;
+
+use common::common_args::*;
 use common::test_dir::*;
 use common::*;
 
 //------------------------------------------
 
-#[test]
+const USAGE: &str = "Usage: thin_rmap [options] {device|file}\n\
-fn accepts_v() -> Result<()> {
+                     Options:\n  \
-    let stdout = thin_rmap!("-V").read()?;
+                       {-h|--help}\n  \
-    assert!(stdout.contains(tools_version()));
+                       {-V|--version}\n  \
-    Ok(())
+                       {--region <block range>}*\n\
+                     Where:\n  \
+                       <block range> is of the form <begin>..<one-past-the-end>\n  \
+                       for example 5..45 denotes blocks 5 to 44 inclusive, but not block 45";
+
+//------------------------------------------
+
+struct ThinRmap;
+
+impl<'a> Program<'a> for ThinRmap {
+    fn name() -> &'a str {
+        "thin_rmap"
     }
 
-#[test]
+    fn path() -> &'a str {
-fn accepts_version() -> Result<()> {
+        THIN_RMAP
-    let stdout = thin_rmap!("--version").read()?;
-    assert!(stdout.contains(tools_version()));
-    Ok(())
     }
 
-const USAGE: &str = "Usage: thin_rmap [options] {device|file}\nOptions:\n  {-h|--help}\n  {-V|--version}\n  {--region <block range>}*\nWhere:\n  <block range> is of the form <begin>..<one-past-the-end>\n  for example 5..45 denotes blocks 5 to 44 inclusive, but not block 45";
+    fn usage() -> &'a str {
-
+        USAGE
-#[test]
-fn accepts_h() -> Result<()> {
-    let stdout = thin_rmap!("-h").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
-#[test]
+    fn arg_type() -> ArgType {
-fn accepts_help() -> Result<()> {
+        ArgType::InputArg
-    let stdout = thin_rmap!("--help").read()?;
-    assert_eq!(stdout, USAGE);
-    Ok(())
     }
 
-#[test]
+    fn bad_option_hint(option: &str) -> String {
-fn rejects_bad_option() -> Result<()> {
+        cpp_msg::bad_option_hint(option)
-    let stderr = run_fail(thin_rmap!("--hedgehogs-only"))?;
-    assert!(stderr.contains("unrecognized option \'--hedgehogs-only\'"));
-    Ok(())
     }
+}
+
+//------------------------------------------
+
+test_accepts_help!(ThinRmap);
+test_accepts_version!(ThinRmap);
+test_rejects_bad_option!(ThinRmap);
+
+//------------------------------------------
 
 #[test]
 fn valid_region_format_should_pass() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
-    thin_rmap!("--region", "23..7890", &md).run()?;
+    let md_path = md.to_str().unwrap();
+    run_ok(THIN_RMAP, &["--region", "23..7890", md_path])?;
     Ok(())
 }
 
@@ -67,7 +75,7 @@ fn invalid_regions_should_fail() -> Result<()> {
     for r in &invalid_regions {
         let mut td = TestDir::new()?;
         let md = mk_valid_md(&mut td)?;
-        run_fail(thin_rmap!(r, &md))?;
+        run_fail(THIN_RMAP, &[&r.to_string(), md.to_str().unwrap()])?;
     }
     Ok(())
 }
@@ -76,7 +84,16 @@ fn invalid_regions_should_fail() -> Result<()> {
 fn multiple_regions_should_pass() -> Result<()> {
     let mut td = TestDir::new()?;
     let md = mk_valid_md(&mut td)?;
-    thin_rmap!("--region", "1..23", "--region", "45..78", &md).run()?;
+    run_ok(
+        THIN_RMAP,
+        &[
+            "--region",
+            "1..23",
+            "--region",
+            "45..78",
+            md.to_str().unwrap(),
+        ],
+    )?;
     Ok(())
 }
 
@@ -84,7 +101,7 @@ fn multiple_regions_should_pass() -> Result<()> {
 fn junk_input() -> Result<()> {
     let mut td = TestDir::new()?;
     let xml = mk_valid_xml(&mut td)?;
-    run_fail(thin_rmap!("--region", "0..-1", &xml))?;
+    run_fail(THIN_RMAP, &["--region", "0..-1", xml.to_str().unwrap()])?;
     Ok(())
 }