From fc300dda24faa4d659a3f8039a7beb14bb9615b1 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Tue, 9 Nov 2021 00:09:18 -0300 Subject: [PATCH 1/6] tests/common: UCommand::new rename arg to bin_path Merge and remove unecessary `.as_ref()` --- tests/common/util.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/common/util.rs b/tests/common/util.rs index e71f18573..22c87a95c 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -838,12 +838,17 @@ pub struct UCommand { } impl UCommand { - pub fn new, U: AsRef>(arg: T, curdir: U, env_clear: bool) -> UCommand { + pub fn new, U: AsRef>( + bin_path: T, + curdir: U, + env_clear: bool, + ) -> UCommand { + let bin_path = bin_path.as_ref(); UCommand { tmpd: None, has_run: false, raw: { - let mut cmd = Command::new(arg.as_ref()); + let mut cmd = Command::new(bin_path); cmd.current_dir(curdir.as_ref()); if env_clear { if cfg!(windows) { @@ -863,7 +868,7 @@ impl UCommand { } cmd }, - comm_string: String::from(arg.as_ref().to_str().unwrap()), + comm_string: String::from(bin_path.to_str().unwrap()), ignore_stdin_write_error: false, bytes_into_stdin: None, stdin: None, @@ -874,9 +879,13 @@ impl UCommand { } } - pub fn new_from_tmp>(arg: T, tmpd: Rc, env_clear: bool) -> UCommand { + pub fn new_from_tmp>( + bin_path: T, + tmpd: Rc, + env_clear: bool, + ) -> UCommand { let tmpd_path_buf = String::from(&(*tmpd.as_ref().path().to_str().unwrap())); - let mut ucmd: UCommand = UCommand::new(arg.as_ref(), tmpd_path_buf, env_clear); + let mut ucmd: UCommand = UCommand::new(bin_path, tmpd_path_buf, env_clear); ucmd.tmpd = Some(tmpd); ucmd } From d4ca4371d7bf53cf3dfa6c60b3b9ef20c5599ee3 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Tue, 9 Nov 2021 02:16:06 -0300 Subject: [PATCH 2/6] tests/common: add util_name+bin_path to UCommand --- tests/common/util.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/tests/common/util.rs b/tests/common/util.rs index 22c87a95c..fd2a00c4e 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -788,7 +788,7 @@ impl TestScenario { /// Returns builder for invoking any system command. Paths given are treated /// relative to the environment's unique temporary test directory. pub fn cmd>(&self, bin: S) -> UCommand { - UCommand::new_from_tmp(bin, self.tmpd.clone(), true) + UCommand::new_from_tmp::(bin, None, self.tmpd.clone(), true) } /// Returns builder for invoking any uutils command. Paths given are treated @@ -812,7 +812,7 @@ impl TestScenario { /// Differs from the builder returned by `cmd` in that `cmd_keepenv` does not call /// `Command::env_clear` (Clears the entire environment map for the child process.) pub fn cmd_keepenv>(&self, bin: S) -> UCommand { - UCommand::new_from_tmp(bin, self.tmpd.clone(), false) + UCommand::new_from_tmp::(bin, None, self.tmpd.clone(), false) } } @@ -826,6 +826,8 @@ impl TestScenario { pub struct UCommand { pub raw: Command, comm_string: String, + bin_path: String, + util_name: Option, tmpd: Option>, has_run: bool, ignore_stdin_write_error: bool, @@ -838,13 +840,16 @@ pub struct UCommand { } impl UCommand { - pub fn new, U: AsRef>( + pub fn new, S: AsRef, U: AsRef>( bin_path: T, + util_name: Option, curdir: U, env_clear: bool, ) -> UCommand { let bin_path = bin_path.as_ref(); - UCommand { + let util_name = util_name.as_ref().map(|un| un.as_ref()); + + let mut ucmd = UCommand { tmpd: None, has_run: false, raw: { @@ -869,6 +874,8 @@ impl UCommand { cmd }, comm_string: String::from(bin_path.to_str().unwrap()), + bin_path: bin_path.to_str().unwrap().to_string(), + util_name: util_name.map(|un| un.to_str().unwrap().to_string()), ignore_stdin_write_error: false, bytes_into_stdin: None, stdin: None, @@ -876,16 +883,23 @@ impl UCommand { stderr: None, #[cfg(target_os = "linux")] limits: vec![], + }; + + if let Some(un) = util_name { + ucmd.arg(un); } + + ucmd } - pub fn new_from_tmp>( + pub fn new_from_tmp, S: AsRef>( bin_path: T, + util_name: Option, tmpd: Rc, env_clear: bool, ) -> UCommand { let tmpd_path_buf = String::from(&(*tmpd.as_ref().path().to_str().unwrap())); - let mut ucmd: UCommand = UCommand::new(bin_path, tmpd_path_buf, env_clear); + let mut ucmd: UCommand = UCommand::new(bin_path, util_name, tmpd_path_buf, env_clear); ucmd.tmpd = Some(tmpd); ucmd } From ab4573bde9ee7af533a132c8817d0c28a52d8b3d Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Tue, 9 Nov 2021 04:20:43 -0300 Subject: [PATCH 3/6] tests/common: create TestScenario::composite_cmd This is made to call UCommand::new with Some(util_name) --- tests/common/util.rs | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tests/common/util.rs b/tests/common/util.rs index fd2a00c4e..d649cd7ca 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -780,9 +780,18 @@ impl TestScenario { /// Returns builder for invoking the target uutils binary. Paths given are /// treated relative to the environment's unique temporary test directory. pub fn ucmd(&self) -> UCommand { - let mut cmd = self.cmd(&self.bin_path); - cmd.arg(&self.util_name); - cmd + self.composite_cmd(&self.bin_path, &self.util_name, true) + } + + /// Returns builder for invoking the target uutils binary. Paths given are + /// treated relative to the environment's unique temporary test directory. + pub fn composite_cmd, T: AsRef>( + &self, + bin: S, + util_name: T, + env_clear: bool, + ) -> UCommand { + UCommand::new_from_tmp(bin, Some(util_name), self.tmpd.clone(), env_clear) } /// Returns builder for invoking any system command. Paths given are treated @@ -794,17 +803,13 @@ impl TestScenario { /// Returns builder for invoking any uutils command. Paths given are treated /// relative to the environment's unique temporary test directory. pub fn ccmd>(&self, bin: S) -> UCommand { - let mut cmd = self.cmd(&self.bin_path); - cmd.arg(bin); - cmd + self.composite_cmd(&self.bin_path, bin, true) } // different names are used rather than an argument // because the need to keep the environment is exceedingly rare. pub fn ucmd_keepenv(&self) -> UCommand { - let mut cmd = self.cmd_keepenv(&self.bin_path); - cmd.arg(&self.util_name); - cmd + self.composite_cmd(&self.bin_path, &self.util_name, false) } /// Returns builder for invoking any system command. Paths given are treated From 0bbc805e43f99b2760bd9d3a0791808b015cc04b Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Tue, 9 Nov 2021 14:37:38 -0300 Subject: [PATCH 4/6] tests/common: add util_name+bin_path to CmdResult --- tests/common/util.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/common/util.rs b/tests/common/util.rs index d649cd7ca..d860e0c0b 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -62,6 +62,10 @@ fn read_scenario_fixture>(tmpd: &Option>, file_rel_p /// within a struct which has convenience assertion functions about those outputs #[derive(Debug, Clone)] pub struct CmdResult { + /// bin_path provided by `TestScenario` or `UCommand` + bin_path: String, + /// util_name provided by `TestScenario` or `UCommand` + util_name: Option, //tmpd is used for convenience functions for asserts against fixtures tmpd: Option>, /// exit status for command (if there is one) @@ -77,6 +81,8 @@ pub struct CmdResult { impl CmdResult { pub fn new( + bin_path: String, + util_name: Option, tmpd: Option>, code: Option, success: bool, @@ -84,6 +90,8 @@ impl CmdResult { stderr: &[u8], ) -> CmdResult { CmdResult { + bin_path, + util_name, tmpd, code, success, @@ -1049,6 +1057,8 @@ impl UCommand { let prog = self.run_no_wait().wait_with_output().unwrap(); CmdResult { + bin_path: self.bin_path.clone(), + util_name: self.util_name.clone(), tmpd: self.tmpd.clone(), code: prog.status.code(), success: prog.status.success(), @@ -1296,6 +1306,8 @@ pub fn expected_result(ts: &TestScenario, args: &[&str]) -> std::result::Result< }; Ok(CmdResult::new( + ts.bin_path.as_os_str().to_str().unwrap().to_string(), + Some(ts.util_name.clone()), Some(result.tmpd()), Some(result.code()), result.succeeded(), @@ -1313,6 +1325,8 @@ mod tests { #[test] fn test_code_is() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: Some(32), success: false, @@ -1326,6 +1340,8 @@ mod tests { #[should_panic] fn test_code_is_fail() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: Some(32), success: false, @@ -1338,6 +1354,8 @@ mod tests { #[test] fn test_failure() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: false, @@ -1351,6 +1369,8 @@ mod tests { #[should_panic] fn test_failure_fail() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1363,6 +1383,8 @@ mod tests { #[test] fn test_success() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1376,6 +1398,8 @@ mod tests { #[should_panic] fn test_success_fail() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: false, @@ -1388,6 +1412,8 @@ mod tests { #[test] fn test_no_stderr_output() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1402,6 +1428,8 @@ mod tests { #[should_panic] fn test_no_stderr_fail() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1416,6 +1444,8 @@ mod tests { #[should_panic] fn test_no_stdout_fail() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1429,6 +1459,8 @@ mod tests { #[test] fn test_std_does_not_contain() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1443,6 +1475,8 @@ mod tests { #[should_panic] fn test_stdout_does_not_contain_fail() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1457,6 +1491,8 @@ mod tests { #[should_panic] fn test_stderr_does_not_contain_fail() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1470,6 +1506,8 @@ mod tests { #[test] fn test_stdout_matches() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1486,6 +1524,8 @@ mod tests { #[should_panic] fn test_stdout_matches_fail() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1501,6 +1541,8 @@ mod tests { #[should_panic] fn test_stdout_not_matches_fail() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1515,6 +1557,8 @@ mod tests { #[test] fn test_normalized_newlines_stdout_is() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, @@ -1531,6 +1575,8 @@ mod tests { #[should_panic] fn test_normalized_newlines_stdout_is_fail() { let res = CmdResult { + bin_path: "".into(), + util_name: None, tmpd: None, code: None, success: true, From f43dfa9a61ece3cde6c434e2164b0046dcc7d047 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Tue, 9 Nov 2021 14:43:55 -0300 Subject: [PATCH 5/6] tests/common: implement CmdResult::usage_error --- tests/common/util.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/tests/common/util.rs b/tests/common/util.rs index d860e0c0b..cfde5f229 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -365,6 +365,23 @@ impl CmdResult { self } + /// asserts that + /// 1. the command resulted in stderr stream output that equals the + /// the following format when both are trimmed of trailing whitespace + /// `"{util_name}: {msg}\nTry '{bin_path} {util_name} --help' for more information."` + /// This the expected format when a UUsageError is returned or when show_error! is called + /// `msg` should be the same as the one provided to UUsageError::new or show_error! + /// + /// 2. the command resulted in empty (zero-length) stdout stream output + pub fn usage_error>(&self, msg: T) -> &CmdResult { + self.stderr_only(format!( + "{0}: {2}\nTry '{1} {0} --help' for more information.", + self.util_name.as_ref().unwrap(), // This shouldn't be called using a normal command + self.bin_path, + msg.as_ref() + )) + } + pub fn stdout_contains>(&self, cmp: T) -> &CmdResult { assert!( self.stdout_str().contains(cmp.as_ref()), From c9624725ab532f513d64392d38e5dcbab92e722e Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Tue, 9 Nov 2021 17:23:41 -0300 Subject: [PATCH 6/6] tests: use CmdResult::usage_error --- tests/by-util/test_base32.rs | 10 ++-------- tests/by-util/test_base64.rs | 10 ++-------- tests/by-util/test_basename.rs | 17 +++++------------ tests/by-util/test_cp.rs | 10 +++------- tests/by-util/test_more.rs | 13 ++++--------- tests/by-util/test_mv.rs | 10 +++------- tests/by-util/test_nice.rs | 13 ++++--------- tests/by-util/test_seq.rs | 28 +++++++++------------------- tests/by-util/test_stdbuf.rs | 10 ++-------- 9 files changed, 34 insertions(+), 87 deletions(-) diff --git a/tests/by-util/test_base32.rs b/tests/by-util/test_base32.rs index ffe2cf74c..4d244704d 100644 --- a/tests/by-util/test_base32.rs +++ b/tests/by-util/test_base32.rs @@ -113,18 +113,12 @@ fn test_wrap_bad_arg() { #[test] fn test_base32_extra_operand() { - let ts = TestScenario::new(util_name!()); - // Expect a failure when multiple files are specified. - ts.ucmd() + new_ucmd!() .arg("a.txt") .arg("b.txt") .fails() - .stderr_only(format!( - "{0}: extra operand 'b.txt'\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + .usage_error("extra operand 'b.txt'"); } #[test] diff --git a/tests/by-util/test_base64.rs b/tests/by-util/test_base64.rs index 87aa0db44..9a7d525bb 100644 --- a/tests/by-util/test_base64.rs +++ b/tests/by-util/test_base64.rs @@ -95,18 +95,12 @@ fn test_wrap_bad_arg() { #[test] fn test_base64_extra_operand() { - let ts = TestScenario::new(util_name!()); - // Expect a failure when multiple files are specified. - ts.ucmd() + new_ucmd!() .arg("a.txt") .arg("b.txt") .fails() - .stderr_only(format!( - "{0}: extra operand 'b.txt'\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + .usage_error("extra operand 'b.txt'"); } #[test] diff --git a/tests/by-util/test_basename.rs b/tests/by-util/test_basename.rs index 141745ac3..962d7373d 100644 --- a/tests/by-util/test_basename.rs +++ b/tests/by-util/test_basename.rs @@ -114,12 +114,7 @@ fn test_no_args() { #[test] fn test_no_args_output() { - let ts = TestScenario::new(util_name!()); - ts.ucmd().fails().stderr_is(&format!( - "{0}: missing operand\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + new_ucmd!().fails().usage_error("missing operand"); } #[test] @@ -129,12 +124,10 @@ fn test_too_many_args() { #[test] fn test_too_many_args_output() { - let ts = TestScenario::new(util_name!()); - ts.ucmd().args(&["a", "b", "c"]).fails().stderr_is(format!( - "{0}: extra operand 'c'\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + new_ucmd!() + .args(&["a", "b", "c"]) + .fails() + .usage_error("extra operand 'c'"); } #[cfg(any(unix, target_os = "redox"))] diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index e86f35833..50abfe967 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -563,17 +563,13 @@ fn test_cp_backup_off() { #[test] fn test_cp_backup_no_clobber_conflicting_options() { - let ts = TestScenario::new(util_name!()); - ts.ucmd() + new_ucmd!() .arg("--backup") .arg("--no-clobber") .arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_HOW_ARE_YOU_SOURCE) - .fails().stderr_is(&format!( - "{0}: options --backup and --no-clobber are mutually exclusive\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + .fails() + .usage_error("options --backup and --no-clobber are mutually exclusive"); } #[test] diff --git a/tests/by-util/test_more.rs b/tests/by-util/test_more.rs index 4b2719d8f..5c2b3c0f6 100644 --- a/tests/by-util/test_more.rs +++ b/tests/by-util/test_more.rs @@ -15,15 +15,10 @@ fn test_more_dir_arg() { // Maybe we could capture the error, i.e. "Device not found" in that case // but I am leaving this for later if atty::is(atty::Stream::Stdout) { - let ts = TestScenario::new(util_name!()); - let result = ts.ucmd().arg(".").run(); - result.failure(); - let expected_error_message = &format!( - "{0}: '.' is a directory.\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - ); - assert_eq!(result.stderr_str().trim(), expected_error_message); + new_ucmd!() + .arg(".") + .fails() + .usage_error("'.' is a directory."); } else { } } diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 8d9b00664..f6650cdba 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -522,17 +522,13 @@ fn test_mv_backup_off() { #[test] fn test_mv_backup_no_clobber_conflicting_options() { - let ts = TestScenario::new(util_name!()); - - ts.ucmd().arg("--backup") + new_ucmd!() + .arg("--backup") .arg("--no-clobber") .arg("file1") .arg("file2") .fails() - .stderr_is(&format!("{0}: options --backup and --no-clobber are mutually exclusive\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + .usage_error("options --backup and --no-clobber are mutually exclusive"); } #[test] diff --git a/tests/by-util/test_nice.rs b/tests/by-util/test_nice.rs index 7a99a333d..4a77ae24e 100644 --- a/tests/by-util/test_nice.rs +++ b/tests/by-util/test_nice.rs @@ -22,15 +22,10 @@ fn test_negative_adjustment() { #[test] fn test_adjustment_with_no_command_should_error() { - let ts = TestScenario::new(util_name!()); - - ts.ucmd() - .args(&["-n", "19"]) - .run() - .stderr_is(&format!("{0}: A command must be given with an adjustment.\nTry '{1} {0} --help' for more information.\n", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + new_ucmd!() + .args(&["-n", "19"]) + .fails() + .usage_error("A command must be given with an adjustment."); } #[test] diff --git a/tests/by-util/test_seq.rs b/tests/by-util/test_seq.rs index 2a2e31f83..90166de92 100644 --- a/tests/by-util/test_seq.rs +++ b/tests/by-util/test_seq.rs @@ -66,24 +66,18 @@ fn test_hex_identifier_in_wrong_place() { #[test] fn test_rejects_nan() { - let ts = TestScenario::new(util_name!()); - - ts.ucmd().args(&["NaN"]).fails().stderr_only(format!( - "{0}: invalid 'not-a-number' argument: 'NaN'\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + new_ucmd!() + .arg("NaN") + .fails() + .usage_error("invalid 'not-a-number' argument: 'NaN'"); } #[test] fn test_rejects_non_floats() { - let ts = TestScenario::new(util_name!()); - - ts.ucmd().args(&["foo"]).fails().stderr_only(&format!( - "{0}: invalid floating point argument: 'foo'\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + new_ucmd!() + .arg("foo") + .fails() + .usage_error("invalid floating point argument: 'foo'"); } #[test] @@ -547,11 +541,7 @@ fn test_trailing_whitespace_error() { new_ucmd!() .arg("1 ") .fails() - .no_stdout() - .stderr_contains("seq: invalid floating point argument: '1 '") - // FIXME The second line of the error message is "Try 'seq - // --help' for more information." - .stderr_contains("for more information."); + .usage_error("invalid floating point argument: '1 '"); } #[test] diff --git a/tests/by-util/test_stdbuf.rs b/tests/by-util/test_stdbuf.rs index c05b65d70..3b03a1d4c 100644 --- a/tests/by-util/test_stdbuf.rs +++ b/tests/by-util/test_stdbuf.rs @@ -53,16 +53,10 @@ fn test_stdbuf_trailing_var_arg() { #[cfg(not(target_os = "windows"))] #[test] fn test_stdbuf_line_buffering_stdin_fails() { - let ts = TestScenario::new(util_name!()); - - ts.ucmd() + new_ucmd!() .args(&["-i", "L", "head"]) .fails() - .stderr_is(&format!( - "{0}: line buffering stdin is meaningless\nTry '{1} {0} --help' for more information.", - ts.util_name, - ts.bin_path.to_string_lossy() - )); + .usage_error("line buffering stdin is meaningless"); } #[cfg(not(target_os = "windows"))]