From 0d1946a5d27e4ff79196d18cc51835a1836da8f9 Mon Sep 17 00:00:00 2001 From: Gilad Naaman Date: Sat, 17 Apr 2021 15:01:52 +0300 Subject: [PATCH 1/4] cksum: Remove direct usage of CmdResult fields in tests --- tests/by-util/test_cksum.rs | 54 ++++++++++++++++++------------------- tests/common/util.rs | 36 +++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 27 deletions(-) diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs index 1a0915cd5..c8e60f8a9 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -31,7 +31,10 @@ fn test_empty() { at.touch("a"); - ucmd.arg("a").succeeds().stdout.ends_with("0 a"); + ucmd.arg("a") + .succeeds() + .no_stderr() + .normalized_newlines_stdout_is("4294967295 0 a\n"); } #[test] @@ -41,36 +44,35 @@ fn test_arg_overrides_stdin() { at.touch("a"); - let result = ucmd - .arg("a") + ucmd.arg("a") .pipe_in(input.as_bytes()) // the command might have exited before all bytes have been pipe in. // in that case, we don't care about the error (broken pipe) .ignore_stdin_write_error() - .run(); - - println!("{}, {}", result.stdout, result.stderr); - - assert!(result.stdout.ends_with("0 a\n")) + .succeeds() + .no_stderr() + .normalized_newlines_stdout_is("4294967295 0 a\n"); } #[test] fn test_invalid_file() { - let (_, mut ucmd) = at_and_ucmd!(); + let ts = TestScenario::new(util_name!()); + let at = ts.fixtures.clone(); - let ls = TestScenario::new("ls"); - let files = ls.cmd("ls").arg("-l").run(); - println!("{:?}", files.stdout); - println!("{:?}", files.stderr); + let folder_name = "asdf"; - let folder_name = "asdf".to_string(); - - let result = ucmd.arg(&folder_name).run(); - - println!("stdout: {:?}", result.stdout); - println!("stderr: {:?}", result.stderr); - assert!(result.stderr.contains("cksum: error: 'asdf'")); - assert!(!result.success); + // First check when file doesn't exist + ts.ucmd().arg(folder_name) + .fails() + .no_stdout() + .stderr_contains("cksum: error: 'asdf' No such file or directory"); + + // Then check when the file is of an invalid type + at.mkdir(folder_name); + ts.ucmd().arg(folder_name) + .fails() + .no_stdout() + .stderr_contains("cksum: error: 'asdf' Is a directory"); } // Make sure crc is correct for files larger than 32 bytes @@ -79,14 +81,13 @@ fn test_invalid_file() { fn test_crc_for_bigger_than_32_bytes() { let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("chars.txt").run(); + let result = ucmd.arg("chars.txt").succeeds(); - let mut stdout_splitted = result.stdout.split(" "); + let mut stdout_splitted = result.stdout_str().split(" "); let cksum: i64 = stdout_splitted.next().unwrap().parse().unwrap(); let bytes_cnt: i64 = stdout_splitted.next().unwrap().parse().unwrap(); - assert!(result.success); assert_eq!(cksum, 586047089); assert_eq!(bytes_cnt, 16); } @@ -95,14 +96,13 @@ fn test_crc_for_bigger_than_32_bytes() { fn test_stdin_larger_than_128_bytes() { let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("larger_than_2056_bytes.txt").run(); + let result = ucmd.arg("larger_than_2056_bytes.txt").succeeds(); - let mut stdout_splitted = result.stdout.split(" "); + let mut stdout_splitted = result.stdout_str().split(" "); let cksum: i64 = stdout_splitted.next().unwrap().parse().unwrap(); let bytes_cnt: i64 = stdout_splitted.next().unwrap().parse().unwrap(); - assert!(result.success); assert_eq!(cksum, 945881979); assert_eq!(bytes_cnt, 2058); } diff --git a/tests/common/util.rs b/tests/common/util.rs index c7f46c2a9..b54c8ee39 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -220,6 +220,13 @@ impl CmdResult { self } + /// Like `stdout_is` but newlines are normalized to `\n`. + pub fn normalized_newlines_stdout_is>(&self, msg: T) -> &CmdResult { + let msg = msg.as_ref().replace("\r\n", "\n"); + assert_eq!(self.stdout.replace("\r\n", "\n"), msg); + self + } + /// asserts that the command resulted in stdout stream output, /// whose bytes equal those of the passed in slice pub fn stdout_is_bytes>(&self, msg: T) -> &CmdResult { @@ -1096,4 +1103,33 @@ mod tests { res.stdout_does_not_match(&positive); } + + #[test] + fn test_normalized_newlines_stdout_is() { + let res = CmdResult { + tmpd: None, + code: None, + success: true, + stdout: "A\r\nB\nC".into(), + stderr: "".into(), + }; + + res.normalized_newlines_stdout_is("A\r\nB\nC"); + res.normalized_newlines_stdout_is("A\nB\nC"); + res.normalized_newlines_stdout_is("A\nB\r\nC"); + } + + #[test] + #[should_panic] + fn test_normalized_newlines_stdout_is_fail() { + let res = CmdResult { + tmpd: None, + code: None, + success: true, + stdout: "A\r\nB\nC".into(), + stderr: "".into(), + }; + + res.normalized_newlines_stdout_is("A\r\nB\nC\n"); + } } From 7c7e64e79c41cffce6ce00c02f97f53e911e11d0 Mon Sep 17 00:00:00 2001 From: Gilad Naaman Date: Sat, 17 Apr 2021 15:14:49 +0300 Subject: [PATCH 2/4] pinky, mktemp: Remove direct usage of CmdResult fields in test --- tests/by-util/test_mktemp.rs | 24 +++++++++--------------- tests/by-util/test_pinky.rs | 12 ++++-------- 2 files changed, 13 insertions(+), 23 deletions(-) diff --git a/tests/by-util/test_mktemp.rs b/tests/by-util/test_mktemp.rs index 2639a2c2f..aa3ff5f1f 100644 --- a/tests/by-util/test_mktemp.rs +++ b/tests/by-util/test_mktemp.rs @@ -113,17 +113,14 @@ fn test_mktemp_mktemp_t() { .arg("-t") .arg(TEST_TEMPLATE7) .succeeds(); - let result = scene + scene .ucmd() .env(TMPDIR, &pathname) .arg("-t") .arg(TEST_TEMPLATE8) - .fails(); - println!("stdout {}", result.stdout); - println!("stderr {}", result.stderr); - assert!(result - .stderr - .contains("error: suffix cannot contain any path separators")); + .fails() + .no_stdout() + .stderr_contains("error: suffix cannot contain any path separators"); } #[test] @@ -391,10 +388,9 @@ fn test_mktemp_tmpdir_one_arg() { .arg("--tmpdir") .arg("apt-key-gpghome.XXXXXXXXXX") .succeeds(); - println!("stdout {}", result.stdout); - println!("stderr {}", result.stderr); - assert!(result.stdout.contains("apt-key-gpghome.")); - assert!(PathBuf::from(result.stdout.trim()).is_file()); + result.no_stderr() + .stdout_contains("apt-key-gpghome."); + assert!(PathBuf::from(result.stdout_str().trim()).is_file()); } #[test] @@ -407,8 +403,6 @@ fn test_mktemp_directory_tmpdir() { .arg("--tmpdir") .arg("apt-key-gpghome.XXXXXXXXXX") .succeeds(); - println!("stdout {}", result.stdout); - println!("stderr {}", result.stderr); - assert!(result.stdout.contains("apt-key-gpghome.")); - assert!(PathBuf::from(result.stdout.trim()).is_dir()); + result.no_stderr().stdout_contains("apt-key-gpghome."); + assert!(PathBuf::from(result.stdout_str().trim()).is_dir()); } diff --git a/tests/by-util/test_pinky.rs b/tests/by-util/test_pinky.rs index c8e8334ab..161054b2c 100644 --- a/tests/by-util/test_pinky.rs +++ b/tests/by-util/test_pinky.rs @@ -43,11 +43,9 @@ fn test_short_format_i() { let actual = TestScenario::new(util_name!()) .ucmd() .args(&args) - .run() - .stdout; + .succeeds() + .stdout_move_str(); let expect = expected_result(&args); - println!("actual: {:?}", actual); - println!("expect: {:?}", expect); let v_actual: Vec<&str> = actual.split_whitespace().collect(); let v_expect: Vec<&str> = expect.split_whitespace().collect(); assert_eq!(v_actual, v_expect); @@ -62,11 +60,9 @@ fn test_short_format_q() { let actual = TestScenario::new(util_name!()) .ucmd() .args(&args) - .run() - .stdout; + .succeeds() + .stdout_move_str(); let expect = expected_result(&args); - println!("actual: {:?}", actual); - println!("expect: {:?}", expect); let v_actual: Vec<&str> = actual.split_whitespace().collect(); let v_expect: Vec<&str> = expect.split_whitespace().collect(); assert_eq!(v_actual, v_expect); From 600bab52ffcfa90ec69c1ec45fc2e4ec7280a19c Mon Sep 17 00:00:00 2001 From: Gilad Naaman Date: Sat, 17 Apr 2021 15:22:20 +0300 Subject: [PATCH 3/4] shred, stat, tail: Remove direct usage of CmdResult fields in test --- tests/by-util/test_shred.rs | 4 +--- tests/by-util/test_stat.rs | 6 +++--- tests/by-util/test_tail.rs | 8 ++++---- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/tests/by-util/test_shred.rs b/tests/by-util/test_shred.rs index de54fae5b..b29b9bfec 100644 --- a/tests/by-util/test_shred.rs +++ b/tests/by-util/test_shred.rs @@ -36,9 +36,7 @@ fn test_shred_force() { at.set_readonly(file); // Try shred -u. - let result = scene.ucmd().arg("-u").arg(file).run(); - println!("stderr = {:?}", result.stderr); - println!("stdout = {:?}", result.stdout); + scene.ucmd().arg("-u").arg(file).run(); // file_a was not deleted because it is readonly. assert!(at.file_exists(file)); diff --git a/tests/by-util/test_stat.rs b/tests/by-util/test_stat.rs index 225ea52cd..376b3db51 100644 --- a/tests/by-util/test_stat.rs +++ b/tests/by-util/test_stat.rs @@ -194,7 +194,7 @@ fn test_terse_normal_format() { // note: contains birth/creation date which increases test fragility // * results may vary due to built-in `stat` limitations as well as linux kernel and rust version capability variations let args = ["-t", "/"]; - let actual = new_ucmd!().args(&args).run().stdout; + let actual = new_ucmd!().args(&args).succeeds().stdout_move_str(); let expect = expected_result(&args); println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -216,7 +216,7 @@ fn test_terse_normal_format() { #[cfg(target_os = "linux")] fn test_format_created_time() { let args = ["-c", "%w", "/boot"]; - let actual = new_ucmd!().args(&args).run().stdout; + let actual = new_ucmd!().args(&args).succeeds().stdout_move_str(); let expect = expected_result(&args); println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -240,7 +240,7 @@ fn test_format_created_time() { #[cfg(target_os = "linux")] fn test_format_created_seconds() { let args = ["-c", "%W", "/boot"]; - let actual = new_ucmd!().args(&args).run().stdout; + let actual = new_ucmd!().args(&args).succeeds().stdout_move_str(); let expect = expected_result(&args); println!("actual: {:?}", actual); println!("expect: {:?}", expect); diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index 5edff4d55..6e9eb4a17 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -226,8 +226,8 @@ fn test_bytes_big() { .arg(FILE) .arg("-c") .arg(format!("{}", N_ARG)) - .run() - .stdout; + .succeeds() + .stdout_move_str(); let expected = at.read(EXPECTED_FILE); assert_eq!(result.len(), expected.len()); @@ -340,6 +340,6 @@ fn test_negative_indexing() { let negative_bytes_index = new_ucmd!().arg("-c").arg("-20").arg(FOOBAR_TXT).run(); - assert_eq!(positive_lines_index.stdout, negative_lines_index.stdout); - assert_eq!(positive_bytes_index.stdout, negative_bytes_index.stdout); + assert_eq!(positive_lines_index.stdout(), negative_lines_index.stdout()); + assert_eq!(positive_bytes_index.stdout(), negative_bytes_index.stdout()); } From c5d7f63b3ca9214f2aa5dc07f378eab8752d7cbf Mon Sep 17 00:00:00 2001 From: Gilad Naaman Date: Sat, 17 Apr 2021 16:48:23 +0300 Subject: [PATCH 4/4] Make CmdResult::code private --- tests/by-util/test_cp.rs | 8 +++----- tests/by-util/test_install.rs | 18 ++++++++++++------ tests/common/util.rs | 2 +- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 07880d5c0..f4aabff3e 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -1029,7 +1029,7 @@ fn test_cp_one_file_system() { at_src.mkdir(TEST_MOUNT_MOUNTPOINT); let mountpoint_path = &at_src.plus_as_string(TEST_MOUNT_MOUNTPOINT); - let _r = scene + scene .cmd("mount") .arg("-t") .arg("tmpfs") @@ -1037,8 +1037,7 @@ fn test_cp_one_file_system() { .arg("size=640k") // ought to be enough .arg("tmpfs") .arg(mountpoint_path) - .run(); - assert!(_r.code == Some(0), "{}", _r.stderr); + .succeeds(); at_src.touch(TEST_MOUNT_OTHER_FILESYSTEM_FILE); @@ -1051,8 +1050,7 @@ fn test_cp_one_file_system() { .run(); // Ditch the mount before the asserts - let _r = scene.cmd("umount").arg(mountpoint_path).run(); - assert!(_r.code == Some(0), "{}", _r.stderr); + scene.cmd("umount").arg(mountpoint_path).succeeds(); assert!(result.success); assert!(!at_dst.file_exists(TEST_MOUNT_OTHER_FILESYSTEM_FILE)); diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 32df8d460..dfaaabce6 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -443,9 +443,12 @@ fn test_install_failing_omitting_directory() { at.mkdir(dir2); at.touch(file1); - let r = ucmd.arg(dir1).arg(file1).arg(dir2).run(); - assert!(r.code == Some(1)); - assert!(r.stderr.contains("omitting directory")); + ucmd.arg(dir1) + .arg(file1) + .arg(dir2) + .fails() + .code_is(1) + .stderr_contains("omitting directory"); } #[test] @@ -458,9 +461,12 @@ fn test_install_failing_no_such_file() { at.mkdir(dir1); at.touch(file1); - let r = ucmd.arg(file1).arg(file2).arg(dir1).run(); - assert!(r.code == Some(1)); - assert!(r.stderr.contains("No such file or directory")); + ucmd.arg(file1) + .arg(file2) + .arg(dir1) + .fails() + .code_is(1) + .stderr_contains("No such file or directory"); } #[test] diff --git a/tests/common/util.rs b/tests/common/util.rs index b54c8ee39..55e121737 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -71,7 +71,7 @@ pub struct CmdResult { //tmpd is used for convenience functions for asserts against fixtures tmpd: Option>, /// exit status for command (if there is one) - pub code: Option, + code: Option, /// zero-exit from running the Command? /// see [`success`] pub success: bool,