Merge pull request #2081 from Gilnaa/refactor-cksum-tests

Remove direct usage of CmdResult fields in tests (cksum, pinky, mktemp, shred, stat, tail)

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();
+    // 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");
     
-    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);
+    // 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);
 }

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));

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]

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());
 }

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);

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));

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);

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());
 }

tests/common/util.rs

@@ -71,7 +71,7 @@ pub struct CmdResult {
     //tmpd is used for convenience functions for asserts against fixtures
     tmpd: Option<Rc<TempDir>>,
     /// exit status for command (if there is one)
-    pub code: Option<i32>,
+    code: Option<i32>,
     /// zero-exit from running the Command?
     /// see [`success`]
     pub success: bool,
@@ -220,6 +220,13 @@ impl CmdResult {
         self
     }
 
+    /// Like `stdout_is` but newlines are normalized to `\n`.
+    pub fn normalized_newlines_stdout_is<T: AsRef<str>>(&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<T: AsRef<[u8]>>(&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");
+    }
 }