From 48121daf94217924942a1f3ec07f6413e2dce144 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Sat, 17 Apr 2021 22:29:07 +0200 Subject: [PATCH 1/4] whoami/id: refactor tests for #1982 --- tests/by-util/test_id.rs | 130 +++++++++++++++++++++-------------- tests/by-util/test_whoami.rs | 67 ++++++++++-------- 2 files changed, 117 insertions(+), 80 deletions(-) diff --git a/tests/by-util/test_id.rs b/tests/by-util/test_id.rs index 719cfd876..534736a32 100644 --- a/tests/by-util/test_id.rs +++ b/tests/by-util/test_id.rs @@ -1,11 +1,32 @@ use crate::common::util::*; +// Apparently some CI environments have configuration issues, e.g. with 'whoami' and 'id'. +// If we are running inside the CI and "needle" is in "stderr" skipping this test is +// considered okay. If we are not inside the CI this calls assert!(result.success). +// +// From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)" +// stderr: "whoami: cannot find name for user ID 1001" +// Maybe: "adduser --uid 1001 username" can put things right? +// stderr = id: error: Could not find uid 1001: No such id: 1001 +fn skipping_test_is_okay(result: &CmdResult, needle: &str) -> bool { + if !result.succeeded() { + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + if is_ci() && result.stderr_str().contains(needle) { + println!("test skipped:"); + return true; + } else { + result.success(); + } + } + false +} + fn return_whoami_username() -> String { let scene = TestScenario::new("whoami"); let result = scene.cmd("whoami").run(); - if is_ci() && result.stderr.contains("cannot find name for user ID") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") { + println!("test skipped:"); return String::from(""); } @@ -14,40 +35,41 @@ fn return_whoami_username() -> String { #[test] fn test_id() { - let result = new_ucmd!().arg("-u").run(); - if result.stderr.contains("cannot find name for user ID") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + let scene = TestScenario::new(util_name!()); + + let result = scene.ucmd().arg("-u").succeeds(); + let uid = result.stdout_str().trim(); + + let result = scene.ucmd().run(); + if skipping_test_is_okay(&result, "Could not find uid") { return; } - let uid = result.success().stdout_str().trim(); - let result = new_ucmd!().run(); - if is_ci() && result.stderr.contains("cannot find name for user ID") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it - return; - } - - if !result.stderr_str().contains("Could not find uid") { - // Verify that the id found by --user/-u exists in the list - result.success().stdout_contains(&uid); - } + // Verify that the id found by --user/-u exists in the list + result.stdout_contains(uid); } #[test] fn test_id_from_name() { let username = return_whoami_username(); - if username == "" { - // Sometimes, the CI is failing here + if username.is_empty() { + return; + } + + let scene = TestScenario::new(util_name!()); + let result = scene.ucmd().arg(&username).run(); + if skipping_test_is_okay(&result, "Could not find uid") { return; } - let result = new_ucmd!().arg(&username).succeeds(); let uid = result.stdout_str().trim(); - new_ucmd!() - .succeeds() + let result = scene.ucmd().run(); + if skipping_test_is_okay(&result, "Could not find uid") { + return; + } + + result // Verify that the id found by --user/-u exists in the list .stdout_contains(uid) // Verify that the username found by whoami exists in the list @@ -56,48 +78,42 @@ fn test_id_from_name() { #[test] fn test_id_name_from_id() { - let result = new_ucmd!().arg("-u").succeeds(); - let uid = result.stdout_str().trim(); + let result = new_ucmd!().arg("-nu").run(); - let result = new_ucmd!().arg("-nu").arg(uid).run(); - if is_ci() && result.stderr.contains("No such user/group") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + let username_id = result.stdout_str().trim(); + + let username_whoami = return_whoami_username(); + if username_whoami.is_empty() { return; } - let username_id = result.success().stdout_str().trim(); - - let scene = TestScenario::new("whoami"); - let result = scene.cmd("whoami").succeeds(); - - let username_whoami = result.stdout_str().trim(); - assert_eq!(username_id, username_whoami); } #[test] fn test_id_group() { - let mut result = new_ucmd!().arg("-g").succeeds(); + let scene = TestScenario::new(util_name!()); + + let mut result = scene.ucmd().arg("-g").succeeds(); let s1 = result.stdout_str().trim(); assert!(s1.parse::().is_ok()); - result = new_ucmd!().arg("--group").succeeds(); + result = scene.ucmd().arg("--group").succeeds(); let s1 = result.stdout_str().trim(); assert!(s1.parse::().is_ok()); } #[test] fn test_id_groups() { - let result = new_ucmd!().arg("-G").succeeds(); - assert!(result.success); + let scene = TestScenario::new(util_name!()); + + let result = scene.ucmd().arg("-G").succeeds(); let groups = result.stdout_str().trim().split_whitespace(); for s in groups { assert!(s.parse::().is_ok()); } - let result = new_ucmd!().arg("--groups").succeeds(); - assert!(result.success); + let result = scene.ucmd().arg("--groups").succeeds(); let groups = result.stdout_str().trim().split_whitespace(); for s in groups { assert!(s.parse::().is_ok()); @@ -106,11 +122,13 @@ fn test_id_groups() { #[test] fn test_id_user() { - let mut result = new_ucmd!().arg("-u").succeeds(); + let scene = TestScenario::new(util_name!()); + + let result = scene.ucmd().arg("-u").succeeds(); let s1 = result.stdout_str().trim(); assert!(s1.parse::().is_ok()); - result = new_ucmd!().arg("--user").succeeds(); + let result = scene.ucmd().arg("--user").succeeds(); let s1 = result.stdout_str().trim(); assert!(s1.parse::().is_ok()); } @@ -118,28 +136,34 @@ fn test_id_user() { #[test] fn test_id_pretty_print() { let username = return_whoami_username(); - if username == "" { - // Sometimes, the CI is failing here + if username.is_empty() { return; } - let result = new_ucmd!().arg("-p").run(); - if result.stdout_str().trim() == "" { - // Sometimes, the CI is failing here with - // old rust versions on Linux + let scene = TestScenario::new(util_name!()); + let result = scene.ucmd().arg("-p").run(); + if result.stdout_str().trim().is_empty() { + // this fails only on: "MinRustV (ubuntu-latest, feat_os_unix)" + // `rustc 1.40.0 (73528e339 2019-12-16)` + // run: /home/runner/work/coreutils/coreutils/target/debug/coreutils id -p + // thread 'test_id::test_id_pretty_print' panicked at 'Command was expected to succeed. + // stdout = + // stderr = ', tests/common/util.rs:157:13 + println!("test skipped:"); return; } + result.success().stdout_contains(username); } #[test] fn test_id_password_style() { let username = return_whoami_username(); - if username == "" { - // Sometimes, the CI is failing here + if username.is_empty() { return; } let result = new_ucmd!().arg("-P").succeeds(); + assert!(result.stdout_str().starts_with(&username)); } diff --git a/tests/by-util/test_whoami.rs b/tests/by-util/test_whoami.rs index c6ec6990f..dc6a1ceed 100644 --- a/tests/by-util/test_whoami.rs +++ b/tests/by-util/test_whoami.rs @@ -1,50 +1,63 @@ use crate::common::util::*; -use std::env; + +// Apparently some CI environments have configuration issues, e.g. with 'whoami' and 'id'. +// If we are running inside the CI and "needle" is in "stderr" skipping this test is +// considered okay. If we are not inside the CI this calls assert!(result.success). +// +// From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)" +// stderr: "whoami: error: failed to get username" +// Maybe: "adduser --uid 1001 username" can put things right? +fn skipping_test_is_okay(result: &CmdResult, needle: &str) -> bool { + if !result.succeeded() { + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + if is_ci() && result.stderr_str().contains(needle) { + println!("test skipped:"); + return true; + } else { + result.success(); + } + } + false +} #[test] fn test_normal() { let (_, mut ucmd) = at_and_ucmd!(); let result = ucmd.run(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - println!("env::var(CI).is_ok() = {}", env::var("CI").is_ok()); - for (key, value) in env::vars() { - println!("{}: {}", key, value); - } - if is_ci() && result.stderr.contains("failed to get username") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + // use std::env; + // println!("env::var(CI).is_ok() = {}", env::var("CI").is_ok()); + // for (key, value) in env::vars() { + // println!("{}: {}", key, value); + // } + + if skipping_test_is_okay(&result, "failed to get username") { return; } - assert!(result.success); - assert!(!result.stdout.trim().is_empty()); + result.no_stderr(); + assert!(!result.stdout_str().trim().is_empty()); } #[test] #[cfg(not(windows))] fn test_normal_compare_id() { - let (_, mut ucmd) = at_and_ucmd!(); + let scene = TestScenario::new(util_name!()); - let result = ucmd.run(); - - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - if is_ci() && result.stderr.contains("failed to get username") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + let result_ucmd = scene.ucmd().run(); + if skipping_test_is_okay(&result_ucmd, "failed to get username") { return; } - assert!(result.success); - let ts = TestScenario::new("id"); - let id = ts.cmd("id").arg("-un").run(); - if is_ci() && id.stderr.contains("cannot find name for user ID") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + let result_cmd = scene.cmd("id").arg("-un").run(); + if skipping_test_is_okay(&result_cmd, "cannot find name for user ID") { return; } - assert_eq!(result.stdout.trim(), id.stdout.trim()); + + assert_eq!( + result_ucmd.stdout_str().trim(), + result_cmd.stdout_str().trim() + ); } From 519b9d34a68ccd282c6f47235e060d5783783173 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 17 Apr 2021 22:40:13 +0200 Subject: [PATCH 2/4] sort: use unstable sort when possible (#2076) * sort: use unstable sort when possible This results in a very minor performance (speed) improvement. It does however result in a memory usage reduction, because unstable sort does not allocate auxiliary memory. There's also an improvement in overall CPU usage. * add benchmarking instructions * add user time * fix typo --- src/uu/sort/BENCHMARKING.md | 41 +++++++++++++++++++++++++++++++++++++ src/uu/sort/src/sort.rs | 6 +++++- 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/src/uu/sort/BENCHMARKING.md b/src/uu/sort/BENCHMARKING.md index 78c2e2b2d..1caea0326 100644 --- a/src/uu/sort/BENCHMARKING.md +++ b/src/uu/sort/BENCHMARKING.md @@ -90,3 +90,44 @@ duplicate the string you passed to hyperfine but remove the `target/release/core Example: `hyperfine "target/release/coreutils sort shuffled_numbers_si.txt -h -o output.txt"` becomes `hyperfine "target/release/coreutils sort shuffled_numbers_si.txt -h -o output.txt" "sort shuffled_numbers_si.txt -h -o output.txt"` (This assumes GNU sort is installed as `sort`) + +## Memory and CPU usage + +The above benchmarks use hyperfine to measure the speed of sorting. There are however other useful metrics to determine overall +resource usage. One way to measure them is the `time` command. This is not to be confused with the `time` that is built in to the bash shell. +You may have to install `time` first, then you have to run it with `/bin/time -v` to give it precedence over the built in `time`. + +
+ Example output + + Command being timed: "target/release/coreutils sort shuffled_numbers.txt" + User time (seconds): 0.10 + System time (seconds): 0.00 + Percent of CPU this job got: 365% + Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.02 + Average shared text size (kbytes): 0 + Average unshared data size (kbytes): 0 + Average stack size (kbytes): 0 + Average total size (kbytes): 0 + Maximum resident set size (kbytes): 25360 + Average resident set size (kbytes): 0 + Major (requiring I/O) page faults: 0 + Minor (reclaiming a frame) page faults: 5802 + Voluntary context switches: 462 + Involuntary context switches: 73 + Swaps: 0 + File system inputs: 1184 + File system outputs: 0 + Socket messages sent: 0 + Socket messages received: 0 + Signals delivered: 0 + Page size (bytes): 4096 + Exit status: 0 + +
+ +Useful metrics to look at could be: + +- User time +- Percent of CPU this job got +- Maximum resident set size diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index c097861fc..07b852921 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -985,7 +985,11 @@ fn exec_check_file(unwrapped_lines: &[Line], settings: &GlobalSettings) -> i32 { } fn sort_by(lines: &mut Vec, settings: &GlobalSettings) { - lines.par_sort_by(|a, b| compare_by(a, b, &settings)) + if settings.stable || settings.unique { + lines.par_sort_by(|a, b| compare_by(a, b, &settings)) + } else { + lines.par_sort_unstable_by(|a, b| compare_by(a, b, &settings)) + } } fn compare_by(a: &Line, b: &Line, global_settings: &GlobalSettings) -> Ordering { From b91fadd8f4f18ef861da155434f5a9f707c73234 Mon Sep 17 00:00:00 2001 From: Gilad Naaman Date: Sun, 18 Apr 2021 02:28:06 +0300 Subject: [PATCH 3/4] Refactored tests for more utilities --- tests/by-util/test_hostid.rs | 6 +----- tests/by-util/test_hostname.rs | 4 +--- tests/by-util/test_nproc.rs | 34 ++++++++++++----------------- tests/by-util/test_pinky.rs | 2 +- tests/by-util/test_printenv.rs | 16 +++++++------- tests/by-util/test_readlink.rs | 15 ++++++++----- tests/by-util/test_sort.rs | 14 ++++++------ tests/by-util/test_sync.rs | 19 ++++++++++------- tests/by-util/test_tsort.rs | 4 ++-- tests/by-util/test_uname.rs | 39 +++++++++------------------------- tests/by-util/test_uptime.rs | 26 +++++++---------------- tests/by-util/test_users.rs | 13 +++++------- 12 files changed, 77 insertions(+), 115 deletions(-) diff --git a/tests/by-util/test_hostid.rs b/tests/by-util/test_hostid.rs index b5b668901..3ea818480 100644 --- a/tests/by-util/test_hostid.rs +++ b/tests/by-util/test_hostid.rs @@ -4,10 +4,6 @@ use self::regex::Regex; #[test] fn test_normal() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.run(); - - assert!(result.success); let re = Regex::new(r"^[0-9a-f]{8}").unwrap(); - assert!(re.is_match(&result.stdout_str())); + new_ucmd!().succeeds().stdout_matches(&re); } diff --git a/tests/by-util/test_hostname.rs b/tests/by-util/test_hostname.rs index c9dc99040..3fcb1ae8b 100644 --- a/tests/by-util/test_hostname.rs +++ b/tests/by-util/test_hostname.rs @@ -14,9 +14,7 @@ fn test_hostname() { #[cfg(not(target_vendor = "apple"))] #[test] fn test_hostname_ip() { - let result = new_ucmd!().arg("-i").run(); - println!("{:#?}", result); - assert!(result.success); + let result = new_ucmd!().arg("-i").succeeds(); assert!(!result.stdout_str().trim().is_empty()); } diff --git a/tests/by-util/test_nproc.rs b/tests/by-util/test_nproc.rs index 055b4890d..abf758829 100644 --- a/tests/by-util/test_nproc.rs +++ b/tests/by-util/test_nproc.rs @@ -2,54 +2,46 @@ use crate::common::util::*; #[test] fn test_nproc() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.run(); - assert!(result.success); - let nproc: u8 = result.stdout.trim().parse().unwrap(); + let nproc: u8 = new_ucmd!().succeeds().stdout_str().trim().parse().unwrap(); assert!(nproc > 0); } #[test] fn test_nproc_all_omp() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("--all").run(); - assert!(result.success); - let nproc: u8 = result.stdout.trim().parse().unwrap(); + let result = new_ucmd!().arg("--all").succeeds(); + + let nproc: u8 = result.stdout_str().trim().parse().unwrap(); assert!(nproc > 0); let result = TestScenario::new(util_name!()) .ucmd_keepenv() .env("OMP_NUM_THREADS", "1") - .run(); - assert!(result.success); - let nproc_omp: u8 = result.stdout.trim().parse().unwrap(); + .succeeds(); + + let nproc_omp: u8 = result.stdout_str().trim().parse().unwrap(); assert!(nproc - 1 == nproc_omp); let result = TestScenario::new(util_name!()) .ucmd_keepenv() .env("OMP_NUM_THREADS", "1") // Has no effect .arg("--all") - .run(); - assert!(result.success); - let nproc_omp: u8 = result.stdout.trim().parse().unwrap(); + .succeeds(); + let nproc_omp: u8 = result.stdout_str().trim().parse().unwrap(); assert!(nproc == nproc_omp); } #[test] fn test_nproc_ignore() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.run(); - assert!(result.success); - let nproc: u8 = result.stdout.trim().parse().unwrap(); + let result = new_ucmd!().succeeds(); + let nproc: u8 = result.stdout_str().trim().parse().unwrap(); if nproc > 1 { // Ignore all CPU but one let result = TestScenario::new(util_name!()) .ucmd_keepenv() .arg("--ignore") .arg((nproc - 1).to_string()) - .run(); - assert!(result.success); - let nproc: u8 = result.stdout.trim().parse().unwrap(); + .succeeds(); + let nproc: u8 = result.stdout_str().trim().parse().unwrap(); assert!(nproc == 1); } } diff --git a/tests/by-util/test_pinky.rs b/tests/by-util/test_pinky.rs index 161054b2c..7a4a3f3df 100644 --- a/tests/by-util/test_pinky.rs +++ b/tests/by-util/test_pinky.rs @@ -75,5 +75,5 @@ fn expected_result(args: &[&str]) -> String { .env("LANGUAGE", "C") .args(args) .run() - .stdout + .stdout_move_str() } diff --git a/tests/by-util/test_printenv.rs b/tests/by-util/test_printenv.rs index 9d90051ea..bc0bc0f3c 100644 --- a/tests/by-util/test_printenv.rs +++ b/tests/by-util/test_printenv.rs @@ -7,10 +7,11 @@ fn test_get_all() { env::set_var(key, "VALUE"); assert_eq!(env::var(key), Ok("VALUE".to_string())); - let result = TestScenario::new(util_name!()).ucmd_keepenv().run(); - assert!(result.success); - assert!(result.stdout.contains("HOME=")); - assert!(result.stdout.contains("KEY=VALUE")); + TestScenario::new(util_name!()) + .ucmd_keepenv() + .succeeds() + .stdout_contains("HOME=") + .stdout_contains("KEY=VALUE"); } #[test] @@ -22,9 +23,8 @@ fn test_get_var() { let result = TestScenario::new(util_name!()) .ucmd_keepenv() .arg("KEY") - .run(); + .succeeds(); - assert!(result.success); - assert!(!result.stdout.is_empty()); - assert!(result.stdout.trim() == "VALUE"); + assert!(!result.stdout_str().is_empty()); + assert!(result.stdout_str().trim() == "VALUE"); } diff --git a/tests/by-util/test_readlink.rs b/tests/by-util/test_readlink.rs index 84747b24c..cae5eafee 100644 --- a/tests/by-util/test_readlink.rs +++ b/tests/by-util/test_readlink.rs @@ -5,7 +5,7 @@ static GIBBERISH: &'static str = "supercalifragilisticexpialidocious"; #[test] fn test_canonicalize() { let (at, mut ucmd) = at_and_ucmd!(); - let actual = ucmd.arg("-f").arg(".").run().stdout; + let actual = ucmd.arg("-f").arg(".").run().stdout_move_str(); let expect = at.root_dir_resolved() + "\n"; println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -15,7 +15,7 @@ fn test_canonicalize() { #[test] fn test_canonicalize_existing() { let (at, mut ucmd) = at_and_ucmd!(); - let actual = ucmd.arg("-e").arg(".").run().stdout; + let actual = ucmd.arg("-e").arg(".").run().stdout_move_str(); let expect = at.root_dir_resolved() + "\n"; println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -25,7 +25,7 @@ fn test_canonicalize_existing() { #[test] fn test_canonicalize_missing() { let (at, mut ucmd) = at_and_ucmd!(); - let actual = ucmd.arg("-m").arg(GIBBERISH).run().stdout; + let actual = ucmd.arg("-m").arg(GIBBERISH).run().stdout_move_str(); let expect = path_concat!(at.root_dir_resolved(), GIBBERISH) + "\n"; println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -37,7 +37,7 @@ fn test_long_redirection_to_current_dir() { let (at, mut ucmd) = at_and_ucmd!(); // Create a 256-character path to current directory let dir = path_concat!(".", ..128); - let actual = ucmd.arg("-n").arg("-m").arg(dir).run().stdout; + let actual = ucmd.arg("-n").arg("-m").arg(dir).run().stdout_move_str(); let expect = at.root_dir_resolved(); println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -48,7 +48,12 @@ fn test_long_redirection_to_current_dir() { fn test_long_redirection_to_root() { // Create a 255-character path to root let dir = path_concat!("..", ..85); - let actual = new_ucmd!().arg("-n").arg("-m").arg(dir).run().stdout; + let actual = new_ucmd!() + .arg("-n") + .arg("-m") + .arg(dir) + .run() + .stdout_move_str(); let expect = get_root_path(); println!("actual: {:?}", actual); println!("expect: {:?}", expect); diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index aacc34eb0..a4a9a383c 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -65,7 +65,7 @@ fn test_random_shuffle_len() { // check whether output is the same length as the input const FILE: &'static str = "default_unsorted_ints.expected"; let (at, _ucmd) = at_and_ucmd!(); - let result = new_ucmd!().arg("-R").arg(FILE).run().stdout; + let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); let expected = at.read(FILE); assert_ne!(result, expected); @@ -77,9 +77,9 @@ fn test_random_shuffle_contains_all_lines() { // check whether lines of input are all in output const FILE: &'static str = "default_unsorted_ints.expected"; let (at, _ucmd) = at_and_ucmd!(); - let result = new_ucmd!().arg("-R").arg(FILE).run().stdout; + let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); let expected = at.read(FILE); - let result_sorted = new_ucmd!().pipe_in(result.clone()).run().stdout; + let result_sorted = new_ucmd!().pipe_in(result.clone()).run().stdout_move_str(); assert_ne!(result, expected); assert_eq!(result_sorted, expected); @@ -92,9 +92,9 @@ fn test_random_shuffle_two_runs_not_the_same() { // as the starting order, or if both random sorts end up having the same order. const FILE: &'static str = "default_unsorted_ints.expected"; let (at, _ucmd) = at_and_ucmd!(); - let result = new_ucmd!().arg("-R").arg(FILE).run().stdout; + let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); let expected = at.read(FILE); - let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout; + let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); assert_ne!(result, expected); assert_ne!(result, unexpected); @@ -107,9 +107,9 @@ fn test_random_shuffle_contains_two_runs_not_the_same() { // as the starting order, or if both random sorts end up having the same order. const FILE: &'static str = "default_unsorted_ints.expected"; let (at, _ucmd) = at_and_ucmd!(); - let result = new_ucmd!().arg("-R").arg(FILE).run().stdout; + let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); let expected = at.read(FILE); - let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout; + let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str(); assert_ne!(result, expected); assert_ne!(result, unexpected); diff --git a/tests/by-util/test_sync.rs b/tests/by-util/test_sync.rs index ddd6969a3..436bfdef3 100644 --- a/tests/by-util/test_sync.rs +++ b/tests/by-util/test_sync.rs @@ -5,8 +5,7 @@ use tempfile::tempdir; #[test] fn test_sync_default() { - let result = new_ucmd!().run(); - assert!(result.success); + new_ucmd!().succeeds(); } #[test] @@ -18,8 +17,10 @@ fn test_sync_incorrect_arg() { fn test_sync_fs() { let temporary_directory = tempdir().unwrap(); let temporary_path = fs::canonicalize(temporary_directory.path()).unwrap(); - let result = new_ucmd!().arg("--file-system").arg(&temporary_path).run(); - assert!(result.success); + new_ucmd!() + .arg("--file-system") + .arg(&temporary_path) + .succeeds(); } #[test] @@ -27,12 +28,14 @@ fn test_sync_data() { // Todo add a second arg let temporary_directory = tempdir().unwrap(); let temporary_path = fs::canonicalize(temporary_directory.path()).unwrap(); - let result = new_ucmd!().arg("--data").arg(&temporary_path).run(); - assert!(result.success); + new_ucmd!().arg("--data").arg(&temporary_path).succeeds(); } #[test] fn test_sync_no_existing_files() { - let result = new_ucmd!().arg("--data").arg("do-no-exist").fails(); - assert!(result.stderr.contains("error: cannot stat")); + new_ucmd!() + .arg("--data") + .arg("do-no-exist") + .fails() + .stderr_contains("error: cannot stat"); } diff --git a/tests/by-util/test_tsort.rs b/tests/by-util/test_tsort.rs index 159b80025..0ea6de281 100644 --- a/tests/by-util/test_tsort.rs +++ b/tests/by-util/test_tsort.rs @@ -28,7 +28,7 @@ fn test_version_flag() { let version_short = new_ucmd!().arg("-V").run(); let version_long = new_ucmd!().arg("--version").run(); - assert_eq!(version_short.stdout, version_long.stdout); + assert_eq!(version_short.stdout(), version_long.stdout()); } #[test] @@ -36,7 +36,7 @@ fn test_help_flag() { let help_short = new_ucmd!().arg("-h").run(); let help_long = new_ucmd!().arg("--help").run(); - assert_eq!(help_short.stdout, help_long.stdout); + assert_eq!(help_short.stdout(), help_long.stdout()); } #[test] diff --git a/tests/by-util/test_uname.rs b/tests/by-util/test_uname.rs index f0e32b430..6b8d2d59d 100644 --- a/tests/by-util/test_uname.rs +++ b/tests/by-util/test_uname.rs @@ -2,60 +2,41 @@ use crate::common::util::*; #[test] fn test_uname_compatible() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-a").run(); - assert!(result.success); + new_ucmd!().arg("-a").succeeds(); } #[test] fn test_uname_name() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-n").run(); - assert!(result.success); + new_ucmd!().arg("-n").succeeds(); } #[test] fn test_uname_processor() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-p").run(); - assert!(result.success); - assert_eq!(result.stdout.trim_end(), "unknown"); + let result = new_ucmd!().arg("-p").succeeds(); + assert_eq!(result.stdout_str().trim_end(), "unknown"); } #[test] fn test_uname_hwplatform() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-i").run(); - assert!(result.success); - assert_eq!(result.stdout.trim_end(), "unknown"); + let result = new_ucmd!().arg("-i").succeeds(); + assert_eq!(result.stdout_str().trim_end(), "unknown"); } #[test] fn test_uname_machine() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-m").run(); - assert!(result.success); + new_ucmd!().arg("-m").succeeds(); } #[test] fn test_uname_kernel_version() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-v").run(); - assert!(result.success); + new_ucmd!().arg("-v").succeeds(); } #[test] fn test_uname_kernel() { let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("-o").run(); - assert!(result.success); + let result = ucmd.arg("-o").succeeds(); #[cfg(target_os = "linux")] - assert!(result.stdout.to_lowercase().contains("linux")); + assert!(result.stdout_str().to_lowercase().contains("linux")); } diff --git a/tests/by-util/test_uptime.rs b/tests/by-util/test_uptime.rs index c8f6a11d3..d20ad90c9 100644 --- a/tests/by-util/test_uptime.rs +++ b/tests/by-util/test_uptime.rs @@ -4,33 +4,23 @@ use crate::common::util::*; #[test] fn test_uptime() { - let result = TestScenario::new(util_name!()).ucmd_keepenv().run(); + TestScenario::new(util_name!()) + .ucmd_keepenv() + .succeeds() + .stdout_contains("load average:") + .stdout_contains(" up "); - println!("stdout = {}", result.stdout); - println!("stderr = {}", result.stderr); - - assert!(result.success); - assert!(result.stdout.contains("load average:")); - assert!(result.stdout.contains(" up ")); // Don't check for users as it doesn't show in some CI } #[test] fn test_uptime_since() { - let scene = TestScenario::new(util_name!()); - - let result = scene.ucmd().arg("--since").succeeds(); - - println!("stdout = {}", result.stdout); - println!("stderr = {}", result.stderr); - - assert!(result.success); let re = Regex::new(r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}").unwrap(); - assert!(re.is_match(&result.stdout.trim())); + + new_ucmd!().arg("--since").succeeds().stdout_matches(&re); } #[test] fn test_failed() { - let (_at, mut ucmd) = at_and_ucmd!(); - ucmd.arg("willfail").fails(); + new_ucmd!().arg("willfail").fails(); } diff --git a/tests/by-util/test_users.rs b/tests/by-util/test_users.rs index cb444b8be..3c5789820 100644 --- a/tests/by-util/test_users.rs +++ b/tests/by-util/test_users.rs @@ -3,14 +3,11 @@ use std::env; #[test] fn test_users_noarg() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.run(); - assert!(result.success); + new_ucmd!().succeeds(); } #[test] fn test_users_check_name() { - let result = TestScenario::new(util_name!()).ucmd_keepenv().run(); - assert!(result.success); + let result = TestScenario::new(util_name!()).ucmd_keepenv().succeeds(); // Expectation: USER is often set let key = "USER"; @@ -21,9 +18,9 @@ fn test_users_check_name() { // Check if "users" contains the name of the user { println!("username found {}", &username); - println!("result.stdout {}", &result.stdout); - if !&result.stdout.is_empty() { - assert!(result.stdout.contains(&username)) + // println!("result.stdout {}", &result.stdout); + if !result.stdout_str().is_empty() { + result.stdout_contains(&username); } } } From 93b03bf9a608ef0602ab29b6fa6796171a8e46c1 Mon Sep 17 00:00:00 2001 From: Gilad Naaman Date: Sun, 18 Apr 2021 02:33:52 +0300 Subject: [PATCH 4/4] Ran cargo fmt --- tests/by-util/test_cksum.rs | 8 +++++--- tests/by-util/test_mktemp.rs | 3 +-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs index c8e60f8a9..592e45c58 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -62,14 +62,16 @@ fn test_invalid_file() { let folder_name = "asdf"; // First check when file doesn't exist - ts.ucmd().arg(folder_name) + 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) + ts.ucmd() + .arg(folder_name) .fails() .no_stdout() .stderr_contains("cksum: error: 'asdf' Is a directory"); diff --git a/tests/by-util/test_mktemp.rs b/tests/by-util/test_mktemp.rs index aa3ff5f1f..c273c407c 100644 --- a/tests/by-util/test_mktemp.rs +++ b/tests/by-util/test_mktemp.rs @@ -388,8 +388,7 @@ fn test_mktemp_tmpdir_one_arg() { .arg("--tmpdir") .arg("apt-key-gpghome.XXXXXXXXXX") .succeeds(); - result.no_stderr() - .stdout_contains("apt-key-gpghome."); + result.no_stderr().stdout_contains("apt-key-gpghome."); assert!(PathBuf::from(result.stdout_str().trim()).is_file()); }