From d51ca4098678b4109c5c608da3ee65ba81682543 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reto=20Habl=C3=BCtzel?= Date: Fri, 9 Apr 2021 11:08:31 +0200 Subject: [PATCH 01/15] allow ignoring stdin write errors in tests * if we want to test an irregular scenario, ignoring errors caused by writing to stdin of the command can be uselful. * for example, when writing some text to stdin of cksum in a scenario where it doesn't consume this input, the child process might have exited before the text was written. therefore, this test sometimes fails with a 'Broken pipe'. --- tests/by-util/test_cksum.rs | 9 +++++++-- tests/common/util.rs | 29 ++++++++++++++++++++++++----- 2 files changed, 31 insertions(+), 7 deletions(-) diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs index 8b41c782c..1a0915cd5 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -35,14 +35,19 @@ fn test_empty() { } #[test] -#[ignore] fn test_arg_overrides_stdin() { let (at, mut ucmd) = at_and_ucmd!(); let input = "foobarfoobar"; at.touch("a"); - let result = ucmd.arg("a").pipe_in(input.as_bytes()).run(); + let result = 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); diff --git a/tests/common/util.rs b/tests/common/util.rs index 8a09b71c1..d0f8083fd 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -33,6 +33,8 @@ static ALREADY_RUN: &str = " you have already run this UCommand, if you want to testing();"; static MULTIPLE_STDIN_MEANINGLESS: &str = "Ucommand is designed around a typical use case of: provide args and input stream -> spawn process -> block until completion -> return output streams. For verifying that a particular section of the input stream is what causes a particular behavior, use the Command type directly."; +static NO_STDIN_MEANINGLESS: &str = "Setting this flag has no effect if there is no stdin"; + /// Test if the program is running under CI pub fn is_ci() -> bool { std::env::var("CI") @@ -624,6 +626,7 @@ pub struct UCommand { tmpd: Option>, has_run: bool, stdin: Option>, + ignore_stdin_write_error: bool, } impl UCommand { @@ -653,6 +656,7 @@ impl UCommand { }, comm_string: String::from(arg.as_ref().to_str().unwrap()), stdin: None, + ignore_stdin_write_error: false, } } @@ -705,6 +709,17 @@ impl UCommand { self.pipe_in(contents) } + /// Ignores error caused by feeding stdin to the command. + /// This is typically useful to test non-standard workflows + /// like feeding something to a command that does not read it + pub fn ignore_stdin_write_error(&mut self) -> &mut UCommand { + if self.stdin.is_none() { + panic!("{}", NO_STDIN_MEANINGLESS); + } + self.ignore_stdin_write_error = true; + self + } + pub fn env(&mut self, key: K, val: V) -> &mut UCommand where K: AsRef, @@ -725,7 +740,7 @@ impl UCommand { } self.has_run = true; log_info("run", &self.comm_string); - let mut result = self + let mut child = self .raw .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -734,15 +749,19 @@ impl UCommand { .unwrap(); if let Some(ref input) = self.stdin { - result + let write_result = child .stdin .take() .unwrap_or_else(|| panic!("Could not take child process stdin")) - .write_all(input) - .unwrap_or_else(|e| panic!("{}", e)); + .write_all(input); + if !self.ignore_stdin_write_error { + if let Err(e) = write_result { + panic!("failed to write to stdin of child: {}", e) + } + } } - result + child } /// Spawns the command, feeds the stdin if any, waits for the result From 81d42aa2b36186bb6b1d58e92968a8d0f06373b3 Mon Sep 17 00:00:00 2001 From: Gilad Naaman Date: Mon, 5 Apr 2021 23:03:43 +0300 Subject: [PATCH 02/15] Fix some tests to not use CmdResult fields --- tests/by-util/test_arch.rs | 14 ++- tests/by-util/test_basename.rs | 2 +- tests/by-util/test_chgrp.rs | 2 +- tests/by-util/test_chmod.rs | 100 ++++++++------------- tests/by-util/test_chown.rs | 157 +++++++++++++++++---------------- tests/by-util/test_chroot.rs | 16 ++-- tests/by-util/test_cp.rs | 30 +++---- tests/by-util/test_date.rs | 20 ++--- tests/by-util/test_du.rs | 52 +++++------ tests/by-util/test_echo.rs | 57 ++++++------ tests/by-util/test_env.rs | 71 ++++++--------- tests/by-util/test_expand.rs | 63 +++++++------ tests/by-util/test_factor.rs | 7 +- tests/by-util/test_fmt.rs | 6 +- tests/by-util/test_groups.rs | 31 ++++--- tests/by-util/test_hashsum.rs | 4 +- tests/by-util/test_hostid.rs | 2 +- tests/by-util/test_hostname.rs | 14 +-- tests/by-util/test_id.rs | 126 +++++++++----------------- tests/by-util/test_install.rs | 35 ++------ tests/by-util/test_ln.rs | 10 +-- tests/by-util/test_logname.rs | 12 +-- 22 files changed, 353 insertions(+), 478 deletions(-) diff --git a/tests/by-util/test_arch.rs b/tests/by-util/test_arch.rs index d2ec138d9..909e0ee80 100644 --- a/tests/by-util/test_arch.rs +++ b/tests/by-util/test_arch.rs @@ -2,17 +2,13 @@ use crate::common::util::*; #[test] fn test_arch() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.run(); - assert!(result.success); + new_ucmd!().succeeds(); } #[test] fn test_arch_help() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("--help").run(); - assert!(result.success); - assert!(result.stdout.contains("architecture name")); + new_ucmd!() + .arg("--help") + .succeeds() + .stdout_contains("architecture name"); } diff --git a/tests/by-util/test_basename.rs b/tests/by-util/test_basename.rs index fa599644d..3483e800c 100644 --- a/tests/by-util/test_basename.rs +++ b/tests/by-util/test_basename.rs @@ -66,7 +66,7 @@ fn test_zero_param() { } fn expect_error(input: Vec<&str>) { - assert!(new_ucmd!().args(&input).fails().no_stdout().stderr.len() > 0); + assert!(new_ucmd!().args(&input).fails().no_stdout().stderr().len() > 0); } #[test] diff --git a/tests/by-util/test_chgrp.rs b/tests/by-util/test_chgrp.rs index 613f52fd2..343b336a6 100644 --- a/tests/by-util/test_chgrp.rs +++ b/tests/by-util/test_chgrp.rs @@ -149,7 +149,7 @@ fn test_big_h() { .arg("bin") .arg("/proc/self/fd") .fails() - .stderr + .stderr_str() .lines() .fold(0, |acc, _| acc + 1) > 1 diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index b85567166..d60b8a50b 100644 --- a/tests/by-util/test_chmod.rs +++ b/tests/by-util/test_chmod.rs @@ -48,7 +48,7 @@ fn run_single_test(test: &TestCase, at: AtPath, mut ucmd: UCommand) { } let r = ucmd.run(); if !r.success { - println!("{}", r.stderr); + println!("{}", r.stderr_str()); panic!("{:?}: failed", ucmd.raw); } @@ -297,13 +297,14 @@ fn test_chmod_recursive() { mkfile(&at.plus_as_string("a/b/c/c"), 0o100444); mkfile(&at.plus_as_string("z/y"), 0o100444); - let result = ucmd - .arg("-R") + ucmd.arg("-R") .arg("--verbose") .arg("-r,a+w") .arg("a") .arg("z") - .succeeds(); + .succeeds() + .stderr_contains(&"to 333 (-wx-wx-wx)") + .stderr_contains(&"to 222 (-w--w--w-)"); assert_eq!(at.metadata("z/y").permissions().mode(), 0o100222); assert_eq!(at.metadata("a/a").permissions().mode(), 0o100222); @@ -312,8 +313,6 @@ fn test_chmod_recursive() { println!("mode {:o}", at.metadata("a").permissions().mode()); assert_eq!(at.metadata("a").permissions().mode(), 0o40333); assert_eq!(at.metadata("z").permissions().mode(), 0o40333); - assert!(result.stderr.contains("to 333 (-wx-wx-wx)")); - assert!(result.stderr.contains("to 222 (-w--w--w-)")); unsafe { umask(original_umask); @@ -322,30 +321,24 @@ fn test_chmod_recursive() { #[test] fn test_chmod_non_existing_file() { - let (_at, mut ucmd) = at_and_ucmd!(); - let result = ucmd + new_ucmd!() .arg("-R") .arg("--verbose") .arg("-r,a+w") .arg("dont-exist") - .fails(); - assert!(result - .stderr - .contains("cannot access 'dont-exist': No such file or directory")); + .fails() + .stderr_contains(&"cannot access 'dont-exist': No such file or directory"); } #[test] fn test_chmod_preserve_root() { - let (_at, mut ucmd) = at_and_ucmd!(); - let result = ucmd + new_ucmd!() .arg("-R") .arg("--preserve-root") .arg("755") .arg("/") - .fails(); - assert!(result - .stderr - .contains("chmod: error: it is dangerous to operate recursively on '/'")); + .fails() + .stderr_contains(&"chmod: error: it is dangerous to operate recursively on '/'"); } #[test] @@ -362,33 +355,27 @@ fn test_chmod_symlink_non_existing_file() { let expected_stderr = &format!("cannot operate on dangling symlink '{}'", test_symlink); at.symlink_file(non_existing, test_symlink); - let mut result; // this cannot succeed since the symbolic link dangles - result = scene.ucmd().arg("755").arg("-v").arg(test_symlink).fails(); - - println!("stdout = {:?}", result.stdout); - println!("stderr = {:?}", result.stderr); - - assert!(result.stdout.contains(expected_stdout)); - assert!(result.stderr.contains(expected_stderr)); - assert_eq!(result.code, Some(1)); + scene.ucmd() + .arg("755") + .arg("-v") + .arg(test_symlink) + .fails() + .code_is(1) + .stdout_contains(expected_stdout) + .stderr_contains(expected_stderr); // this should be the same than with just '-v' but without stderr - result = scene - .ucmd() + scene.ucmd() .arg("755") .arg("-v") .arg("-f") .arg(test_symlink) - .fails(); - - println!("stdout = {:?}", result.stdout); - println!("stderr = {:?}", result.stderr); - - assert!(result.stdout.contains(expected_stdout)); - assert!(result.stderr.is_empty()); - assert_eq!(result.code, Some(1)); + .run() + .code_is(1) + .no_stderr() + .stdout_contains(expected_stdout); } #[test] @@ -405,18 +392,15 @@ fn test_chmod_symlink_non_existing_file_recursive() { non_existing, &format!("{}/{}", test_directory, test_symlink), ); - let mut result; // this should succeed - result = scene - .ucmd() + scene.ucmd() .arg("-R") .arg("755") .arg(test_directory) - .succeeds(); - assert_eq!(result.code, Some(0)); - assert!(result.stdout.is_empty()); - assert!(result.stderr.is_empty()); + .succeeds() + .no_stderr() + .no_stdout(); let expected_stdout = &format!( "mode of '{}' retained as 0755 (rwxr-xr-x)\nneither symbolic link '{}/{}' nor referent has been changed", @@ -424,37 +408,25 @@ fn test_chmod_symlink_non_existing_file_recursive() { ); // '-v': this should succeed without stderr - result = scene - .ucmd() + scene.ucmd() .arg("-R") .arg("-v") .arg("755") .arg(test_directory) - .succeeds(); - - println!("stdout = {:?}", result.stdout); - println!("stderr = {:?}", result.stderr); - - assert!(result.stdout.contains(expected_stdout)); - assert!(result.stderr.is_empty()); - assert_eq!(result.code, Some(0)); + .succeeds() + .stdout_contains(expected_stdout) + .no_stderr(); // '-vf': this should be the same than with just '-v' - result = scene - .ucmd() + scene.ucmd() .arg("-R") .arg("-v") .arg("-f") .arg("755") .arg(test_directory) - .succeeds(); - - println!("stdout = {:?}", result.stdout); - println!("stderr = {:?}", result.stderr); - - assert!(result.stdout.contains(expected_stdout)); - assert!(result.stderr.is_empty()); - assert_eq!(result.code, Some(0)); + .succeeds() + .stdout_contains(expected_stdout) + .no_stderr(); } #[test] diff --git a/tests/by-util/test_chown.rs b/tests/by-util/test_chown.rs index 7b663e9c9..e27fba3d4 100644 --- a/tests/by-util/test_chown.rs +++ b/tests/by-util/test_chown.rs @@ -53,22 +53,22 @@ fn test_chown_myself() { // test chown username file.txt let scene = TestScenario::new(util_name!()); let result = scene.cmd("whoami").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("results {}", result.stdout); - let username = result.stdout.trim_end(); + println!("results {}", result.stdout_str()); + let username = result.stdout_str().trim_end(); let (at, mut ucmd) = at_and_ucmd!(); let file1 = "test_install_target_dir_file_a1"; at.touch(file1); let result = ucmd.arg(username).arg(file1).run(); - println!("results stdout {}", result.stdout); - println!("results stderr {}", result.stderr); - if is_ci() && result.stderr.contains("invalid user") { + println!("results stdout {}", result.stdout_str()); + println!("results stderr {}", result.stderr_str()); + if is_ci() && result.stderr_str().contains("invalid user") { // In the CI, some server are failing to return id. // As seems to be a configuration issue, ignoring it return; @@ -81,24 +81,24 @@ fn test_chown_myself_second() { // test chown username: file.txt let scene = TestScenario::new(util_name!()); let result = scene.cmd("whoami").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("results {}", result.stdout); + println!("results {}", result.stdout_str()); let (at, mut ucmd) = at_and_ucmd!(); let file1 = "test_install_target_dir_file_a1"; at.touch(file1); let result = ucmd - .arg(result.stdout.trim_end().to_owned() + ":") + .arg(result.stdout_str().trim_end().to_owned() + ":") .arg(file1) .run(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); assert!(result.success); } @@ -107,31 +107,31 @@ fn test_chown_myself_group() { // test chown username:group file.txt let scene = TestScenario::new(util_name!()); let result = scene.cmd("whoami").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("user name = {}", result.stdout); - let username = result.stdout.trim_end(); + println!("user name = {}", result.stdout_str()); + let username = result.stdout_str().trim_end(); let result = scene.cmd("id").arg("-gn").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("group name = {}", result.stdout); - let group = result.stdout.trim_end(); + println!("group name = {}", result.stdout_str()); + let group = result.stdout_str().trim_end(); let (at, mut ucmd) = at_and_ucmd!(); let file1 = "test_install_target_dir_file_a1"; let perm = username.to_owned() + ":" + group; at.touch(file1); let result = ucmd.arg(perm).arg(file1).run(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - if is_ci() && result.stderr.contains("chown: invalid group:") { + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + if is_ci() && result.stderr_str().contains("chown: invalid group:") { // With some Ubuntu into the CI, we can get this answer return; } @@ -143,27 +143,27 @@ fn test_chown_only_group() { // test chown :group file.txt let scene = TestScenario::new(util_name!()); let result = scene.cmd("whoami").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("results {}", result.stdout); + println!("results {}", result.stdout_str()); let (at, mut ucmd) = at_and_ucmd!(); let file1 = "test_install_target_dir_file_a1"; - let perm = ":".to_owned() + result.stdout.trim_end(); + let perm = ":".to_owned() + result.stdout_str().trim_end(); at.touch(file1); let result = ucmd.arg(perm).arg(file1).run(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); - if is_ci() && result.stderr.contains("Operation not permitted") { + if is_ci() && result.stderr_str().contains("Operation not permitted") { // With ubuntu with old Rust in the CI, we can get an error return; } - if is_ci() && result.stderr.contains("chown: invalid group:") { + if is_ci() && result.stderr_str().contains("chown: invalid group:") { // With mac into the CI, we can get this answer return; } @@ -174,14 +174,14 @@ fn test_chown_only_group() { fn test_chown_only_id() { // test chown 1111 file.txt let result = TestScenario::new("id").ucmd_keepenv().arg("-u").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - let id = String::from(result.stdout.trim()); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + let id = String::from(result.stdout_str().trim()); let (at, mut ucmd) = at_and_ucmd!(); let file1 = "test_install_target_dir_file_a1"; @@ -189,9 +189,9 @@ fn test_chown_only_id() { at.touch(file1); let result = ucmd.arg(id).arg(file1).run(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - if is_ci() && result.stderr.contains("chown: invalid user:") { + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + if is_ci() && result.stderr_str().contains("chown: invalid user:") { // With some Ubuntu into the CI, we can get this answer return; } @@ -202,14 +202,14 @@ fn test_chown_only_id() { fn test_chown_only_group_id() { // test chown :1111 file.txt let result = TestScenario::new("id").ucmd_keepenv().arg("-g").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - let id = String::from(result.stdout.trim()); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + let id = String::from(result.stdout_str().trim()); let (at, mut ucmd) = at_and_ucmd!(); let file1 = "test_install_target_dir_file_a1"; @@ -219,9 +219,9 @@ fn test_chown_only_group_id() { let result = ucmd.arg(perm).arg(file1).run(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - if is_ci() && result.stderr.contains("chown: invalid group:") { + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + if is_ci() && result.stderr_str().contains("chown: invalid group:") { // With mac into the CI, we can get this answer return; } @@ -232,24 +232,24 @@ fn test_chown_only_group_id() { fn test_chown_both_id() { // test chown 1111:1111 file.txt let result = TestScenario::new("id").ucmd_keepenv().arg("-u").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - let id_user = String::from(result.stdout.trim()); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + let id_user = String::from(result.stdout_str().trim()); let result = TestScenario::new("id").ucmd_keepenv().arg("-g").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - let id_group = String::from(result.stdout.trim()); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + let id_group = String::from(result.stdout_str().trim()); let (at, mut ucmd) = at_and_ucmd!(); let file1 = "test_install_target_dir_file_a1"; @@ -258,10 +258,10 @@ fn test_chown_both_id() { let perm = id_user + &":".to_owned() + &id_group; let result = ucmd.arg(perm).arg(file1).run(); - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); - if is_ci() && result.stderr.contains("invalid user") { + if is_ci() && result.stderr_str().contains("invalid user") { // In the CI, some server are failing to return id. // As seems to be a configuration issue, ignoring it return; @@ -274,24 +274,24 @@ fn test_chown_both_id() { fn test_chown_both_mix() { // test chown 1111:1111 file.txt let result = TestScenario::new("id").ucmd_keepenv().arg("-u").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - let id_user = String::from(result.stdout.trim()); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + let id_user = String::from(result.stdout_str().trim()); let result = TestScenario::new("id").ucmd_keepenv().arg("-gn").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - let group_name = String::from(result.stdout.trim()); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + let group_name = String::from(result.stdout_str().trim()); let (at, mut ucmd) = at_and_ucmd!(); let file1 = "test_install_target_dir_file_a1"; @@ -301,7 +301,7 @@ fn test_chown_both_mix() { let result = ucmd.arg(perm).arg(file1).run(); - if is_ci() && result.stderr.contains("invalid user") { + if is_ci() && result.stderr_str().contains("invalid user") { // In the CI, some server are failing to return id. // As seems to be a configuration issue, ignoring it return; @@ -313,14 +313,14 @@ fn test_chown_both_mix() { fn test_chown_recursive() { let scene = TestScenario::new(util_name!()); let result = scene.cmd("whoami").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - let username = result.stdout.trim_end(); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + let username = result.stdout_str().trim_end(); let (at, mut ucmd) = at_and_ucmd!(); at.mkdir("a"); @@ -339,31 +339,32 @@ fn test_chown_recursive() { .arg("a") .arg("z") .run(); - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - if is_ci() && result.stderr.contains("invalid user") { + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + if is_ci() && result.stderr_str().contains("invalid user") { // In the CI, some server are failing to return id. // As seems to be a configuration issue, ignoring it return; } - assert!(result.stderr.contains("ownership of 'a/a' retained as")); - assert!(result.stderr.contains("ownership of 'z/y' retained as")); - assert!(result.success); + result + .stderr_contains(&"ownership of 'a/a' retained as") + .stderr_contains(&"ownership of 'z/y' retained as") + .success(); } #[test] fn test_root_preserve() { let scene = TestScenario::new(util_name!()); let result = scene.cmd("whoami").run(); - if is_ci() && result.stderr.contains("No such user/group") { + if is_ci() && result.stderr_str().contains("No such user/group") { // In the CI, some server are failing to return whoami. // As seems to be a configuration issue, ignoring it return; } - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - let username = result.stdout.trim_end(); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + let username = result.stdout_str().trim_end(); let result = new_ucmd!() .arg("--preserve-root") @@ -371,9 +372,9 @@ fn test_root_preserve() { .arg(username) .arg("/") .fails(); - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - if is_ci() && result.stderr.contains("invalid user") { + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + if is_ci() && result.stderr_str().contains("invalid user") { // In the CI, some server are failing to return id. // As seems to be a configuration issue, ignoring it return; diff --git a/tests/by-util/test_chroot.rs b/tests/by-util/test_chroot.rs index 9a8fb71dd..05efd23ae 100644 --- a/tests/by-util/test_chroot.rs +++ b/tests/by-util/test_chroot.rs @@ -64,14 +64,14 @@ fn test_preference_of_userspec() { // As seems to be a configuration issue, ignoring it return; } - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - let username = result.stdout.trim_end(); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + let username = result.stdout_str().trim_end(); let ts = TestScenario::new("id"); let result = ts.cmd("id").arg("-g").arg("-n").run(); - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); if is_ci() && result.stderr.contains("cannot find name for user ID") { // In the CI, some server are failing to return id. @@ -79,7 +79,7 @@ fn test_preference_of_userspec() { return; } - let group_name = result.stdout.trim_end(); + let group_name = result.stdout_str().trim_end(); let (at, mut ucmd) = at_and_ucmd!(); at.mkdir("a"); @@ -93,6 +93,6 @@ fn test_preference_of_userspec() { .arg(format!("--userspec={}:{}", username, group_name)) .run(); - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 1fa8212ca..07880d5c0 100644 --- a/tests/by-util/test_cp.rs +++ b/tests/by-util/test_cp.rs @@ -275,8 +275,8 @@ fn test_cp_arg_no_clobber_twice() { .arg("dest.txt") .run(); - println!("stderr = {:?}", result.stderr); - println!("stdout = {:?}", result.stdout); + println!("stderr = {:?}", result.stderr_str()); + println!("stdout = {:?}", result.stdout_str()); assert!(result.success); assert!(result.stderr.is_empty()); assert_eq!(at.read("source.txt"), ""); @@ -317,8 +317,8 @@ fn test_cp_arg_force() { .arg(TEST_HELLO_WORLD_DEST) .run(); - println!("{:?}", result.stderr); - println!("{:?}", result.stdout); + println!("{:?}", result.stderr_str()); + println!("{:?}", result.stdout_str()); assert!(result.success); assert_eq!(at.read(TEST_HELLO_WORLD_DEST), "Hello, World!\n"); @@ -602,7 +602,7 @@ fn test_cp_deref_folder_to_folder() { .arg(TEST_COPY_FROM_FOLDER) .arg(TEST_COPY_TO_FOLDER_NEW) .run(); - println!("cp output {}", result.stdout); + println!("cp output {}", result.stdout_str()); // Check that the exit code represents a successful copy. assert!(result.success); @@ -611,12 +611,12 @@ fn test_cp_deref_folder_to_folder() { { let scene2 = TestScenario::new("ls"); let result = scene2.cmd("ls").arg("-al").arg(path_to_new_symlink).run(); - println!("ls source {}", result.stdout); + println!("ls source {}", result.stdout_str()); let path_to_new_symlink = at.subdir.join(TEST_COPY_TO_FOLDER_NEW); let result = scene2.cmd("ls").arg("-al").arg(path_to_new_symlink).run(); - println!("ls dest {}", result.stdout); + println!("ls dest {}", result.stdout_str()); } #[cfg(windows)] @@ -706,7 +706,7 @@ fn test_cp_no_deref_folder_to_folder() { .arg(TEST_COPY_FROM_FOLDER) .arg(TEST_COPY_TO_FOLDER_NEW) .run(); - println!("cp output {}", result.stdout); + println!("cp output {}", result.stdout_str()); // Check that the exit code represents a successful copy. assert!(result.success); @@ -715,12 +715,12 @@ fn test_cp_no_deref_folder_to_folder() { { let scene2 = TestScenario::new("ls"); let result = scene2.cmd("ls").arg("-al").arg(path_to_new_symlink).run(); - println!("ls source {}", result.stdout); + println!("ls source {}", result.stdout_str()); let path_to_new_symlink = at.subdir.join(TEST_COPY_TO_FOLDER_NEW); let result = scene2.cmd("ls").arg("-al").arg(path_to_new_symlink).run(); - println!("ls dest {}", result.stdout); + println!("ls dest {}", result.stdout_str()); } #[cfg(windows)] @@ -809,7 +809,7 @@ fn test_cp_archive() { let scene2 = TestScenario::new("ls"); let result = scene2.cmd("ls").arg("-al").arg(at.subdir).run(); - println!("ls dest {}", result.stdout); + println!("ls dest {}", result.stdout_str()); assert_eq!(creation, creation2); assert!(result.success); } @@ -863,7 +863,7 @@ fn test_cp_archive_recursive() { .arg(&at.subdir.join(TEST_COPY_TO_FOLDER)) .run(); - println!("ls dest {}", result.stdout); + println!("ls dest {}", result.stdout_str()); let scene2 = TestScenario::new("ls"); let result = scene2 @@ -872,7 +872,7 @@ fn test_cp_archive_recursive() { .arg(&at.subdir.join(TEST_COPY_TO_FOLDER_NEW)) .run(); - println!("ls dest {}", result.stdout); + println!("ls dest {}", result.stdout_str()); assert!(at.file_exists( &at.subdir .join(TEST_COPY_TO_FOLDER_NEW) @@ -946,7 +946,7 @@ fn test_cp_preserve_timestamps() { let scene2 = TestScenario::new("ls"); let result = scene2.cmd("ls").arg("-al").arg(at.subdir).run(); - println!("ls dest {}", result.stdout); + println!("ls dest {}", result.stdout_str()); assert_eq!(creation, creation2); assert!(result.success); } @@ -984,7 +984,7 @@ fn test_cp_dont_preserve_timestamps() { let scene2 = TestScenario::new("ls"); let result = scene2.cmd("ls").arg("-al").arg(at.subdir).run(); - println!("ls dest {}", result.stdout); + println!("ls dest {}", result.stdout_str()); println!("creation {:?} / {:?}", creation, creation2); assert_ne!(creation, creation2); diff --git a/tests/by-util/test_date.rs b/tests/by-util/test_date.rs index 5619aed94..1933fdba3 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -28,13 +28,13 @@ fn test_date_rfc_3339() { // Check that the output matches the regexp let rfc_regexp = r"(\d+)-(0[1-9]|1[012])-(0[1-9]|[12]\d|3[01])\s([01]\d|2[0-3]):([0-5]\d):([0-5]\d|60)(\.\d+)?(([Zz])|([\+|\-]([01]\d|2[0-3])))"; let re = Regex::new(rfc_regexp).unwrap(); - assert!(re.is_match(&result.stdout.trim())); + assert!(re.is_match(&result.stdout_str().trim())); result = scene.ucmd().arg("--rfc-3339=seconds").succeeds(); // Check that the output matches the regexp let re = Regex::new(rfc_regexp).unwrap(); - assert!(re.is_match(&result.stdout.trim())); + assert!(re.is_match(&result.stdout_str().trim())); } #[test] @@ -73,13 +73,13 @@ fn test_date_format_y() { assert!(result.success); let mut re = Regex::new(r"^\d{4}$").unwrap(); - assert!(re.is_match(&result.stdout.trim())); + assert!(re.is_match(&result.stdout_str().trim())); result = scene.ucmd().arg("+%y").succeeds(); assert!(result.success); re = Regex::new(r"^\d{2}$").unwrap(); - assert!(re.is_match(&result.stdout.trim())); + assert!(re.is_match(&result.stdout_str().trim())); } #[test] @@ -90,13 +90,13 @@ fn test_date_format_m() { assert!(result.success); let mut re = Regex::new(r"\S+").unwrap(); - assert!(re.is_match(&result.stdout.trim())); + assert!(re.is_match(&result.stdout_str().trim())); result = scene.ucmd().arg("+%m").succeeds(); assert!(result.success); re = Regex::new(r"^\d{2}$").unwrap(); - assert!(re.is_match(&result.stdout.trim())); + assert!(re.is_match(&result.stdout_str().trim())); } #[test] @@ -107,20 +107,20 @@ fn test_date_format_day() { assert!(result.success); let mut re = Regex::new(r"\S+").unwrap(); - assert!(re.is_match(&result.stdout.trim())); + assert!(re.is_match(&result.stdout_str().trim())); result = scene.ucmd().arg("+%A").succeeds(); assert!(result.success); re = Regex::new(r"\S+").unwrap(); - assert!(re.is_match(&result.stdout.trim())); + assert!(re.is_match(&result.stdout_str().trim())); result = scene.ucmd().arg("+%u").succeeds(); assert!(result.success); re = Regex::new(r"^\d{1}$").unwrap(); - assert!(re.is_match(&result.stdout.trim())); + assert!(re.is_match(&result.stdout_str().trim())); } #[test] @@ -131,7 +131,7 @@ fn test_date_format_full_day() { assert!(result.success); let re = Regex::new(r"\S+ \d{4}-\d{2}-\d{2}").unwrap(); - assert!(re.is_match(&result.stdout.trim())); + assert!(re.is_match(&result.stdout_str().trim())); } #[test] diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 30dcd9bb3..8f2cff65d 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -7,10 +7,9 @@ const SUB_LINK: &str = "subdir/links/sublink.txt"; #[test] fn test_du_basics() { - let (_at, mut ucmd) = at_and_ucmd!(); - let result = ucmd.run(); - assert!(result.success); - assert_eq!(result.stderr, ""); + new_ucmd!() + .succeeds() + .no_stderr(); } #[cfg(target_vendor = "apple")] fn _du_basics(s: String) { @@ -22,7 +21,7 @@ fn _du_basics(s: String) { assert_eq!(s, answer); } #[cfg(not(target_vendor = "apple"))] -fn _du_basics(s: String) { +fn _du_basics(s: &str) { let answer = "28\t./subdir 8\t./subdir/deeper 16\t./subdir/links @@ -38,19 +37,19 @@ fn test_du_basics_subdir() { let result = ucmd.arg(SUB_DIR).run(); assert!(result.success); assert_eq!(result.stderr, ""); - _du_basics_subdir(result.stdout); + _du_basics_subdir(result.stdout_str()); } #[cfg(target_vendor = "apple")] -fn _du_basics_subdir(s: String) { +fn _du_basics_subdir(s: &str) { assert_eq!(s, "4\tsubdir/deeper\n"); } #[cfg(target_os = "windows")] -fn _du_basics_subdir(s: String) { +fn _du_basics_subdir(s: &str) { assert_eq!(s, "0\tsubdir/deeper\n"); } #[cfg(all(not(target_vendor = "apple"), not(target_os = "windows")))] -fn _du_basics_subdir(s: String) { +fn _du_basics_subdir(s: &str) { // MS-WSL linux has altered expected output if !is_wsl() { assert_eq!(s, "8\tsubdir/deeper\n"); @@ -64,7 +63,7 @@ fn test_du_basics_bad_name() { let (_at, mut ucmd) = at_and_ucmd!(); let result = ucmd.arg("bad_name").run(); - assert_eq!(result.stdout, ""); + assert_eq!(result.stdout_str(), ""); assert_eq!( result.stderr, "du: error: bad_name: No such file or directory\n" @@ -81,20 +80,20 @@ fn test_du_soft_link() { let result = ts.ucmd().arg(SUB_DIR_LINKS).run(); assert!(result.success); assert_eq!(result.stderr, ""); - _du_soft_link(result.stdout); + _du_soft_link(result.stdout_str()); } #[cfg(target_vendor = "apple")] -fn _du_soft_link(s: String) { +fn _du_soft_link(s: &str) { // 'macos' host variants may have `du` output variation for soft links assert!((s == "12\tsubdir/links\n") || (s == "16\tsubdir/links\n")); } #[cfg(target_os = "windows")] -fn _du_soft_link(s: String) { +fn _du_soft_link(s: &str) { assert_eq!(s, "8\tsubdir/links\n"); } #[cfg(all(not(target_vendor = "apple"), not(target_os = "windows")))] -fn _du_soft_link(s: String) { +fn _du_soft_link(s: &str) { // MS-WSL linux has altered expected output if !is_wsl() { assert_eq!(s, "16\tsubdir/links\n"); @@ -114,19 +113,19 @@ fn test_du_hard_link() { assert!(result.success); assert_eq!(result.stderr, ""); // We do not double count hard links as the inodes are identical - _du_hard_link(result.stdout); + _du_hard_link(result.stdout_str()); } #[cfg(target_vendor = "apple")] -fn _du_hard_link(s: String) { +fn _du_hard_link(s: &str) { assert_eq!(s, "12\tsubdir/links\n") } #[cfg(target_os = "windows")] -fn _du_hard_link(s: String) { +fn _du_hard_link(s: &str) { assert_eq!(s, "8\tsubdir/links\n") } #[cfg(all(not(target_vendor = "apple"), not(target_os = "windows")))] -fn _du_hard_link(s: String) { +fn _du_hard_link(s: &str) { // MS-WSL linux has altered expected output if !is_wsl() { assert_eq!(s, "16\tsubdir/links\n"); @@ -142,19 +141,19 @@ fn test_du_d_flag() { let result = ts.ucmd().arg("-d").arg("1").run(); assert!(result.success); assert_eq!(result.stderr, ""); - _du_d_flag(result.stdout); + _du_d_flag(result.stdout_str()); } #[cfg(target_vendor = "apple")] -fn _du_d_flag(s: String) { +fn _du_d_flag(s: &str) { assert_eq!(s, "16\t./subdir\n20\t./\n"); } #[cfg(target_os = "windows")] -fn _du_d_flag(s: String) { +fn _du_d_flag(s: &str) { assert_eq!(s, "8\t./subdir\n8\t./\n"); } #[cfg(all(not(target_vendor = "apple"), not(target_os = "windows")))] -fn _du_d_flag(s: String) { +fn _du_d_flag(s: &str) { // MS-WSL linux has altered expected output if !is_wsl() { assert_eq!(s, "28\t./subdir\n36\t./\n"); @@ -167,10 +166,11 @@ fn _du_d_flag(s: String) { fn test_du_h_flag_empty_file() { let ts = TestScenario::new("du"); - let result = ts.ucmd().arg("-h").arg("empty.txt").run(); - assert!(result.success); - assert_eq!(result.stderr, ""); - assert_eq!(result.stdout, "0\tempty.txt\n"); + ts.ucmd() + .arg("-h") + .arg("empty.txt") + .succeeds() + .stdout_only("0\tempty.txt\n"); } #[cfg(feature = "touch")] diff --git a/tests/by-util/test_echo.rs b/tests/by-util/test_echo.rs index 7394ffc1e..99c8f3a1e 100644 --- a/tests/by-util/test_echo.rs +++ b/tests/by-util/test_echo.rs @@ -2,22 +2,20 @@ use crate::common::util::*; #[test] fn test_default() { - //CmdResult.stdout_only(...) trims trailing newlines - assert_eq!("hi\n", new_ucmd!().arg("hi").succeeds().no_stderr().stdout); + new_ucmd!() + .arg("hi") + .succeeds() + .stdout_only("hi\n"); } #[test] fn test_no_trailing_newline() { - //CmdResult.stdout_only(...) trims trailing newlines - assert_eq!( - "hi", - new_ucmd!() - .arg("-n") - .arg("hi") - .succeeds() - .no_stderr() - .stdout - ); + new_ucmd!() + .arg("-n") + .arg("hi") + .succeeds() + .no_stderr() + .stdout_only("hi"); } #[test] @@ -192,39 +190,38 @@ fn test_hyphen_values_inside_string() { new_ucmd!() .arg("'\"\n'CXXFLAGS=-g -O2'\n\"'") .succeeds() - .stdout - .contains("CXXFLAGS"); + .stdout_contains("CXXFLAGS"); } #[test] fn test_hyphen_values_at_start() { - let result = new_ucmd!() + new_ucmd!() .arg("-E") .arg("-test") .arg("araba") .arg("-merci") - .run(); - - assert!(result.success); - assert_eq!(false, result.stdout.contains("-E")); - assert_eq!(result.stdout, "-test araba -merci\n"); + .run() + .success() + .stdout_does_not_contain("-E") + .stdout_is("-test araba -merci\n"); } #[test] fn test_hyphen_values_between() { - let result = new_ucmd!().arg("test").arg("-E").arg("araba").run(); + new_ucmd!() + .arg("test") + .arg("-E") + .arg("araba") + .run() + .success() + .stdout_is("test -E araba\n"); - assert!(result.success); - assert_eq!(result.stdout, "test -E araba\n"); - - let result = new_ucmd!() + new_ucmd!() .arg("dumdum ") .arg("dum dum dum") .arg("-e") .arg("dum") - .run(); - - assert!(result.success); - assert_eq!(result.stdout, "dumdum dum dum dum -e dum\n"); - assert_eq!(true, result.stdout.contains("-e")); + .run() + .success() + .stdout_is("dumdum dum dum dum -e dum\n"); } diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index 2ffb2bc48..19ecd7afb 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -8,45 +8,35 @@ use tempfile::tempdir; #[test] fn test_env_help() { - assert!(new_ucmd!() + new_ucmd!() .arg("--help") .succeeds() .no_stderr() - .stdout - .contains("OPTIONS:")); + .stdout_contains("OPTIONS:"); } #[test] fn test_env_version() { - assert!(new_ucmd!() + new_ucmd!() .arg("--version") .succeeds() .no_stderr() - .stdout - .contains(util_name!())); + .stdout_contains(util_name!()); } #[test] fn test_echo() { - // assert!(new_ucmd!().arg("printf").arg("FOO-bar").succeeds().no_stderr().stdout.contains("FOO-bar")); - let mut cmd = new_ucmd!(); - cmd.arg("echo").arg("FOO-bar"); - println!("cmd={:?}", cmd); + let result = new_ucmd!() + .arg("echo") + .arg("FOO-bar") + .succeeds(); - let result = cmd.run(); - println!("success={:?}", result.success); - println!("stdout={:?}", result.stdout); - println!("stderr={:?}", result.stderr); - assert!(result.success); - - let out = result.stdout.trim_end(); - - assert_eq!(out, "FOO-bar"); + assert_eq!(result.stdout_str().trim(), "FOO-bar"); } #[test] fn test_file_option() { - let out = new_ucmd!().arg("-f").arg("vars.conf.txt").run().stdout; + let out = new_ucmd!().arg("-f").arg("vars.conf.txt").run().stdout_move_str(); assert_eq!( out.lines() @@ -63,7 +53,7 @@ fn test_combined_file_set() { .arg("vars.conf.txt") .arg("FOO=bar.alt") .run() - .stdout; + .stdout_move_str(); assert_eq!(out.lines().filter(|&line| line == "FOO=bar.alt").count(), 1); } @@ -76,8 +66,8 @@ fn test_combined_file_set_unset() { .arg("-f") .arg("vars.conf.txt") .arg("FOO=bar.alt") - .run() - .stdout; + .succeeds() + .stdout_move_str(); assert_eq!( out.lines() @@ -89,17 +79,17 @@ fn test_combined_file_set_unset() { #[test] fn test_single_name_value_pair() { - let out = new_ucmd!().arg("FOO=bar").run().stdout; + let out = new_ucmd!().arg("FOO=bar").run(); - assert!(out.lines().any(|line| line == "FOO=bar")); + assert!(out.stdout_str().lines().any(|line| line == "FOO=bar")); } #[test] fn test_multiple_name_value_pairs() { - let out = new_ucmd!().arg("FOO=bar").arg("ABC=xyz").run().stdout; + let out = new_ucmd!().arg("FOO=bar").arg("ABC=xyz").run(); assert_eq!( - out.lines() + out.stdout_str().lines() .filter(|&line| line == "FOO=bar" || line == "ABC=xyz") .count(), 2 @@ -110,13 +100,8 @@ fn test_multiple_name_value_pairs() { fn test_ignore_environment() { let scene = TestScenario::new(util_name!()); - let out = scene.ucmd().arg("-i").run().stdout; - - assert_eq!(out, ""); - - let out = scene.ucmd().arg("-").run().stdout; - - assert_eq!(out, ""); + scene.ucmd().arg("-i").run().no_stdout(); + scene.ucmd().arg("-").run().no_stdout(); } #[test] @@ -126,8 +111,8 @@ fn test_null_delimiter() { .arg("--null") .arg("FOO=bar") .arg("ABC=xyz") - .run() - .stdout; + .succeeds() + .stdout_move_str(); let mut vars: Vec<_> = out.split('\0').collect(); assert_eq!(vars.len(), 3); @@ -145,8 +130,8 @@ fn test_unset_variable() { .ucmd_keepenv() .arg("-u") .arg("HOME") - .run() - .stdout; + .succeeds() + .stdout_move_str(); assert_eq!(out.lines().any(|line| line.starts_with("HOME=")), false); } @@ -173,8 +158,8 @@ fn test_change_directory() { .arg("--chdir") .arg(&temporary_path) .arg(pwd) - .run() - .stdout; + .succeeds() + .stdout_move_str(); assert_eq!(out.trim(), temporary_path.as_os_str()) } @@ -193,8 +178,8 @@ fn test_change_directory() { .ucmd() .arg("--chdir") .arg(&temporary_path) - .run() - .stdout; + .succeeds() + .stdout_move_str(); assert_eq!( out.lines() .any(|line| line.ends_with(temporary_path.file_name().unwrap().to_str().unwrap())), @@ -214,6 +199,6 @@ fn test_fail_change_directory() { .arg(some_non_existing_path) .arg("pwd") .fails() - .stderr; + .stderr_move_str(); assert!(out.contains("env: cannot change directory to ")); } diff --git a/tests/by-util/test_expand.rs b/tests/by-util/test_expand.rs index 801bf9d98..834a09736 100644 --- a/tests/by-util/test_expand.rs +++ b/tests/by-util/test_expand.rs @@ -2,57 +2,54 @@ use crate::common::util::*; #[test] fn test_with_tab() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("with-tab.txt").run(); - assert!(result.success); - assert!(result.stdout.contains(" ")); - assert!(!result.stdout.contains("\t")); + new_ucmd!() + .arg("with-tab.txt") + .succeeds() + .stdout_contains(" ") + .stdout_does_not_contain("\t"); } #[test] fn test_with_trailing_tab() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("with-trailing-tab.txt").run(); - assert!(result.success); - assert!(result.stdout.contains("with tabs=> ")); - assert!(!result.stdout.contains("\t")); + new_ucmd!() + .arg("with-trailing-tab.txt") + .succeeds() + .stdout_contains("with tabs=> ") + .stdout_does_not_contain("\t"); } #[test] fn test_with_trailing_tab_i() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("with-trailing-tab.txt").arg("-i").run(); - assert!(result.success); - assert!(result.stdout.contains(" // with tabs=>\t")); + new_ucmd!() + .arg("with-trailing-tab.txt") + .arg("-i") + .succeeds() + .stdout_contains(" // with tabs=>\t"); } #[test] fn test_with_tab_size() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("with-tab.txt").arg("--tabs=10").run(); - assert!(result.success); - assert!(result.stdout.contains(" ")); + new_ucmd!() + .arg("with-tab.txt") + .arg("--tabs=10") + .succeeds() + .stdout_contains(" "); } #[test] fn test_with_space() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("with-spaces.txt").run(); - assert!(result.success); - assert!(result.stdout.contains(" return")); + new_ucmd!() + .arg("with-spaces.txt") + .succeeds() + .stdout_contains(" return"); } #[test] fn test_with_multiple_files() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("with-spaces.txt").arg("with-tab.txt").run(); - assert!(result.success); - assert!(result.stdout.contains(" return")); - assert!(result.stdout.contains(" ")); + new_ucmd!() + .arg("with-spaces.txt") + .arg("with-tab.txt") + .succeeds() + .stdout_contains(" return") + .stdout_contains(" "); } diff --git a/tests/by-util/test_factor.rs b/tests/by-util/test_factor.rs index 5bde17cdb..af2ff4ddb 100644 --- a/tests/by-util/test_factor.rs +++ b/tests/by-util/test_factor.rs @@ -32,13 +32,10 @@ fn test_first_100000_integers() { } println!("STDIN='{}'", instring); - let result = new_ucmd!().pipe_in(instring.as_bytes()).run(); - let stdout = result.stdout; - - assert!(result.success); + let result = new_ucmd!().pipe_in(instring.as_bytes()).succeeds(); // `seq 0 100000 | factor | sha1sum` => "4ed2d8403934fa1c76fe4b84c5d4b8850299c359" - let hash_check = sha1::Sha1::from(stdout.as_bytes()).hexdigest(); + let hash_check = sha1::Sha1::from(result.stdout()).hexdigest(); assert_eq!(hash_check, "4ed2d8403934fa1c76fe4b84c5d4b8850299c359"); } diff --git a/tests/by-util/test_fmt.rs b/tests/by-util/test_fmt.rs index 4533cdf24..f962a9137 100644 --- a/tests/by-util/test_fmt.rs +++ b/tests/by-util/test_fmt.rs @@ -5,7 +5,7 @@ fn test_fmt() { let result = new_ucmd!().arg("one-word-per-line.txt").run(); //.stdout_is_fixture("call_graph.expected"); assert_eq!( - result.stdout.trim(), + result.stdout_str().trim(), "this is a file with one word per line" ); } @@ -15,7 +15,7 @@ fn test_fmt_q() { let result = new_ucmd!().arg("-q").arg("one-word-per-line.txt").run(); //.stdout_is_fixture("call_graph.expected"); assert_eq!( - result.stdout.trim(), + result.stdout_str().trim(), "this is a file with one word per line" ); } @@ -42,7 +42,7 @@ fn test_fmt_w() { .arg("one-word-per-line.txt") .run(); //.stdout_is_fixture("call_graph.expected"); - assert_eq!(result.stdout.trim(), "this is a file with one word per line"); + assert_eq!(result.stdout_str().trim(), "this is a file with one word per line"); } diff --git a/tests/by-util/test_groups.rs b/tests/by-util/test_groups.rs index 5c326fe2d..32a16cc1a 100644 --- a/tests/by-util/test_groups.rs +++ b/tests/by-util/test_groups.rs @@ -2,26 +2,25 @@ use crate::common::util::*; #[test] fn test_groups() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.run(); - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - if is_ci() && result.stdout.trim().is_empty() { + let result = new_ucmd!().run(); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + if is_ci() && result.stdout_str().trim().is_empty() { // In the CI, some server are failing to return the group. // As seems to be a configuration issue, ignoring it return; } assert!(result.success); - assert!(!result.stdout.trim().is_empty()); + assert!(!result.stdout_str().trim().is_empty()); } #[test] fn test_groups_arg() { // get the username with the "id -un" command let result = TestScenario::new("id").ucmd_keepenv().arg("-un").run(); - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); - let s1 = String::from(result.stdout.trim()); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + let s1 = String::from(result.stdout_str().trim()); if is_ci() && s1.parse::().is_ok() { // In the CI, some server are failing to return id -un. // So, if we are getting a uid, just skip this test @@ -29,18 +28,18 @@ fn test_groups_arg() { return; } - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); assert!(result.success); - assert!(!result.stdout.is_empty()); - let username = result.stdout.trim(); + assert!(!result.stdout_str().is_empty()); + let username = result.stdout_str().trim(); // call groups with the user name to check that we // are getting something let (_, mut ucmd) = at_and_ucmd!(); let result = ucmd.arg(username).run(); - println!("result.stdout {}", result.stdout); - println!("result.stderr = {}", result.stderr); + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); assert!(result.success); - assert!(!result.stdout.is_empty()); + assert!(!result.stdout_str().is_empty()); } diff --git a/tests/by-util/test_hashsum.rs b/tests/by-util/test_hashsum.rs index 6e7d59107..f059e53f3 100644 --- a/tests/by-util/test_hashsum.rs +++ b/tests/by-util/test_hashsum.rs @@ -17,14 +17,14 @@ macro_rules! test_digest { fn test_single_file() { let ts = TestScenario::new("hashsum"); assert_eq!(ts.fixtures.read(EXPECTED_FILE), - get_hash!(ts.ucmd().arg(DIGEST_ARG).arg(BITS_ARG).arg("input.txt").succeeds().no_stderr().stdout)); + get_hash!(ts.ucmd().arg(DIGEST_ARG).arg(BITS_ARG).arg("input.txt").succeeds().no_stderr().stdout_str())); } #[test] fn test_stdin() { let ts = TestScenario::new("hashsum"); assert_eq!(ts.fixtures.read(EXPECTED_FILE), - get_hash!(ts.ucmd().arg(DIGEST_ARG).arg(BITS_ARG).pipe_in_fixture("input.txt").succeeds().no_stderr().stdout)); + get_hash!(ts.ucmd().arg(DIGEST_ARG).arg(BITS_ARG).pipe_in_fixture("input.txt").succeeds().no_stderr().stdout_str())); } } )*) diff --git a/tests/by-util/test_hostid.rs b/tests/by-util/test_hostid.rs index 17aad4aff..b5b668901 100644 --- a/tests/by-util/test_hostid.rs +++ b/tests/by-util/test_hostid.rs @@ -9,5 +9,5 @@ fn test_normal() { assert!(result.success); let re = Regex::new(r"^[0-9a-f]{8}").unwrap(); - assert!(re.is_match(&result.stdout.trim())); + assert!(re.is_match(&result.stdout_str())); } diff --git a/tests/by-util/test_hostname.rs b/tests/by-util/test_hostname.rs index 804d47642..9fa63241f 100644 --- a/tests/by-util/test_hostname.rs +++ b/tests/by-util/test_hostname.rs @@ -6,8 +6,8 @@ fn test_hostname() { let ls_short_res = new_ucmd!().arg("-s").succeeds(); let ls_domain_res = new_ucmd!().arg("-d").succeeds(); - assert!(ls_default_res.stdout.len() >= ls_short_res.stdout.len()); - assert!(ls_default_res.stdout.len() >= ls_domain_res.stdout.len()); + assert!(ls_default_res.stdout().len() >= ls_short_res.stdout().len()); + assert!(ls_default_res.stdout().len() >= ls_domain_res.stdout().len()); } // FixME: fails for "MacOS" @@ -17,14 +17,14 @@ fn test_hostname_ip() { let result = new_ucmd!().arg("-i").run(); println!("{:#?}", result); assert!(result.success); - assert!(!result.stdout.trim().is_empty()); + assert!(!result.stdout_str().trim().is_empty()); } #[test] fn test_hostname_full() { - let result = new_ucmd!().arg("-f").succeeds(); - assert!(!result.stdout.trim().is_empty()); - let ls_short_res = new_ucmd!().arg("-s").succeeds(); - assert!(result.stdout.trim().contains(ls_short_res.stdout.trim())); + assert!(!ls_short_res.stdout_str().trim().is_empty()); + + new_ucmd!().arg("-f").succeeds() + .stdout_contains(ls_short_res.stdout_str().trim()); } diff --git a/tests/by-util/test_id.rs b/tests/by-util/test_id.rs index 116c73995..7e2791467 100644 --- a/tests/by-util/test_id.rs +++ b/tests/by-util/test_id.rs @@ -9,33 +9,29 @@ fn return_whoami_username() -> String { return String::from(""); } - result.stdout.trim().to_string() + result.stdout_str().trim().to_string() } #[test] fn test_id() { - let scene = TestScenario::new(util_name!()); - - let mut result = scene.ucmd().arg("-u").run(); + 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 return; } - assert!(result.success); - let uid = String::from(result.stdout.trim()); - result = scene.ucmd().run(); + 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; } - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - if !result.stderr.contains("Could not find uid") { + + if !result.stderr_str().contains("Could not find uid") { // Verify that the id found by --user/-u exists in the list - assert!(result.stdout.contains(&uid)); + result.success().stdout_contains(&uid); } } @@ -47,88 +43,64 @@ fn test_id_from_name() { return; } - let scene = TestScenario::new(util_name!()); - let result = scene.ucmd().arg(&username).succeeds(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - assert!(result.success); - let uid = String::from(result.stdout.trim()); - let result = scene.ucmd().succeeds(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - // Verify that the id found by --user/-u exists in the list - assert!(result.stdout.contains(&uid)); - // Verify that the username found by whoami exists in the list - assert!(result.stdout.contains(&username)); + let result = new_ucmd!().arg(&username).succeeds(); + let uid = result.stdout_str().trim(); + + new_ucmd!().succeeds() + // 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 + .stdout_contains(username); } #[test] fn test_id_name_from_id() { - let mut scene = TestScenario::new(util_name!()); - let result = scene.ucmd().arg("-u").run(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - assert!(result.success); - let uid = String::from(result.stdout.trim()); + let result = new_ucmd!().arg("-u").succeeds(); + let uid = result.stdout_str().trim(); - scene = TestScenario::new(util_name!()); - let result = scene.ucmd().arg("-nu").arg(uid).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 return; } - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - assert!(result.success); - let username_id = String::from(result.stdout.trim()); + let username_id = result + .success() + .stdout_str() + .trim(); - scene = TestScenario::new("whoami"); - let result = scene.cmd("whoami").run(); + let scene = TestScenario::new("whoami"); + let result = scene.cmd("whoami").succeeds(); - let username_whoami = result.stdout.trim(); + let username_whoami = result.stdout_str().trim(); assert_eq!(username_id, username_whoami); } #[test] fn test_id_group() { - let scene = TestScenario::new(util_name!()); - - let mut result = scene.ucmd().arg("-g").succeeds(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - assert!(result.success); - let s1 = String::from(result.stdout.trim()); + let mut result = new_ucmd!().arg("-g").succeeds(); + let s1 = result.stdout_str().trim(); assert!(s1.parse::().is_ok()); - result = scene.ucmd().arg("--group").succeeds(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - assert!(result.success); - let s1 = String::from(result.stdout.trim()); + result = new_ucmd!().arg("--group").succeeds(); + let s1 = result.stdout_str().trim(); assert!(s1.parse::().is_ok()); } #[test] fn test_id_groups() { - let scene = TestScenario::new(util_name!()); - - let result = scene.ucmd().arg("-G").succeeds(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); + let result = new_ucmd!().arg("-G").succeeds(); assert!(result.success); - let groups = result.stdout.trim().split_whitespace(); + let groups = result.stdout_str().trim().split_whitespace(); for s in groups { assert!(s.parse::().is_ok()); } - let result = scene.ucmd().arg("--groups").succeeds(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); + let result = new_ucmd!().arg("--groups").succeeds(); assert!(result.success); - let groups = result.stdout.trim().split_whitespace(); + let groups = result.stdout_str().trim().split_whitespace(); for s in groups { assert!(s.parse::().is_ok()); } @@ -136,15 +108,12 @@ fn test_id_groups() { #[test] fn test_id_user() { - let scene = TestScenario::new(util_name!()); - - let mut result = scene.ucmd().arg("-u").succeeds(); - assert!(result.success); - let s1 = String::from(result.stdout.trim()); + let mut result = new_ucmd!().arg("-u").succeeds(); + let s1 = result.stdout_str().trim(); assert!(s1.parse::().is_ok()); - result = scene.ucmd().arg("--user").succeeds(); - assert!(result.success); - let s1 = String::from(result.stdout.trim()); + + result = new_ucmd!().arg("--user").succeeds(); + let s1 = result.stdout_str().trim(); assert!(s1.parse::().is_ok()); } @@ -156,17 +125,13 @@ fn test_id_pretty_print() { return; } - let scene = TestScenario::new(util_name!()); - let result = scene.ucmd().arg("-p").run(); - if result.stdout.trim() == "" { + let result = new_ucmd!().arg("-p").run(); + if result.stdout_str().trim() == "" { // Sometimes, the CI is failing here with // old rust versions on Linux return; } - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - assert!(result.success); - assert!(result.stdout.contains(&username)); + result.success().stdout_contains(username); } #[test] @@ -176,12 +141,7 @@ fn test_id_password_style() { // Sometimes, the CI is failing here return; } - let scene = TestScenario::new(util_name!()); - let result = scene.ucmd().arg("-P").succeeds(); - - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - assert!(result.success); - assert!(result.stdout.starts_with(&username)); + let result = new_ucmd!().arg("-P").succeeds(); + assert!(result.stdout_str().starts_with(&username)); } diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 840b2f6c7..32df8d460 100644 --- a/tests/by-util/test_install.rs +++ b/tests/by-util/test_install.rs @@ -195,12 +195,8 @@ fn test_install_mode_numeric() { let mode_arg = "-m 0333"; at.mkdir(dir2); - let result = scene.ucmd().arg(mode_arg).arg(file).arg(dir2).run(); + scene.ucmd().arg(mode_arg).arg(file).arg(dir2).succeeds(); - println!("stderr = {:?}", result.stderr); - println!("stdout = {:?}", result.stdout); - - assert!(result.success); let dest_file = &format!("{}/{}", dir2, file); assert!(at.file_exists(file)); assert!(at.file_exists(dest_file)); @@ -313,16 +309,13 @@ fn test_install_target_new_file_with_group() { .arg(format!("{}/{}", dir, file)) .run(); - println!("stderr = {:?}", result.stderr); - println!("stdout = {:?}", result.stdout); - - if is_ci() && result.stderr.contains("error: no such group:") { + if is_ci() && result.stderr_str().contains("error: no such group:") { // In the CI, some server are failing to return the group. // As seems to be a configuration issue, ignoring it return; } - assert!(result.success); + result.success(); assert!(at.file_exists(file)); assert!(at.file_exists(&format!("{}/{}", dir, file))); } @@ -343,16 +336,13 @@ fn test_install_target_new_file_with_owner() { .arg(format!("{}/{}", dir, file)) .run(); - println!("stderr = {:?}", result.stderr); - println!("stdout = {:?}", result.stdout); - if is_ci() && result.stderr.contains("error: no such user:") { // In the CI, some server are failing to return the user id. // As seems to be a configuration issue, ignoring it return; } - assert!(result.success); + result.success(); assert!(at.file_exists(file)); assert!(at.file_exists(&format!("{}/{}", dir, file))); } @@ -366,13 +356,10 @@ fn test_install_target_new_file_failing_nonexistent_parent() { at.touch(file1); - let err = ucmd - .arg(file1) + ucmd.arg(file1) .arg(format!("{}/{}", dir, file2)) .fails() - .stderr; - - assert!(err.contains("not a directory")) + .stderr_contains(&"not a directory"); } #[test] @@ -417,18 +404,12 @@ fn test_install_copy_file() { #[test] #[cfg(target_os = "linux")] fn test_install_target_file_dev_null() { - let scene = TestScenario::new(util_name!()); - let at = &scene.fixtures; + let (at, mut ucmd) = at_and_ucmd!(); let file1 = "/dev/null"; let file2 = "target_file"; - let result = scene.ucmd().arg(file1).arg(file2).run(); - - println!("stderr = {:?}", result.stderr); - println!("stdout = {:?}", result.stdout); - - assert!(result.success); + ucmd.arg(file1).arg(file2).succeeds(); assert!(at.file_exists(file2)); } diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index 89261036d..d7a13b0d4 100644 --- a/tests/by-util/test_ln.rs +++ b/tests/by-util/test_ln.rs @@ -520,10 +520,7 @@ fn test_symlink_no_deref_dir() { scene.ucmd().args(&["-sn", dir1, link]).fails(); // Try with the no-deref - let result = scene.ucmd().args(&["-sfn", dir1, link]).run(); - println!("stdout {}", result.stdout); - println!("stderr {}", result.stderr); - assert!(result.success); + scene.ucmd().args(&["-sfn", dir1, link]).succeeds(); assert!(at.dir_exists(dir1)); assert!(at.dir_exists(dir2)); assert!(at.is_symlink(link)); @@ -566,10 +563,7 @@ fn test_symlink_no_deref_file() { scene.ucmd().args(&["-sn", file1, link]).fails(); // Try with the no-deref - let result = scene.ucmd().args(&["-sfn", file1, link]).run(); - println!("stdout {}", result.stdout); - println!("stderr {}", result.stderr); - assert!(result.success); + scene.ucmd().args(&["-sfn", file1, link]).succeeds(); assert!(at.file_exists(file1)); assert!(at.file_exists(file2)); assert!(at.is_symlink(link)); diff --git a/tests/by-util/test_logname.rs b/tests/by-util/test_logname.rs index b15941c06..8d1996e63 100644 --- a/tests/by-util/test_logname.rs +++ b/tests/by-util/test_logname.rs @@ -3,23 +3,19 @@ use std::env; #[test] fn test_normal() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.run(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); + let result = new_ucmd!().run(); println!("env::var(CI).is_ok() = {}", env::var("CI").is_ok()); for (key, value) in env::vars() { println!("{}: {}", key, value); } - if (is_ci() || is_wsl()) && result.stderr.contains("error: no login name") { + if (is_ci() || is_wsl()) && result.stderr_str().contains("error: no login name") { // ToDO: investigate WSL failure // In the CI, some server are failing to return logname. // As seems to be a configuration issue, ignoring it return; } - assert!(result.success); - assert!(!result.stdout.trim().is_empty()); + result.success(); + assert!(!result.stdout_str().trim().is_empty()); } From 97d12d6e3c7b8afcde13555e34e35545b7f0a96a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reto=20Habl=C3=BCtzel?= Date: Sun, 11 Apr 2021 16:05:25 +0200 Subject: [PATCH 03/15] fix trivial warnings without features --- src/uu/basename/src/basename.rs | 10 +-- src/uu/cp/src/cp.rs | 16 ++--- src/uu/cut/src/cut.rs | 66 +++++++++---------- src/uu/du/src/du.rs | 2 +- src/uu/expr/src/expr.rs | 2 +- src/uu/expr/src/syntax_tree.rs | 36 +++++----- src/uu/fmt/src/parasplit.rs | 2 +- src/uu/fold/src/fold.rs | 2 +- src/uu/head/src/head.rs | 2 +- src/uu/ln/src/ln.rs | 14 ++-- src/uu/ls/src/ls.rs | 8 +-- src/uu/mv/src/mv.rs | 16 ++--- src/uu/od/src/od.rs | 2 +- src/uu/od/src/parse_inputs.rs | 6 +- .../num_format/formatters/base_conv/mod.rs | 6 +- src/uu/readlink/src/readlink.rs | 6 +- src/uu/realpath/src/realpath.rs | 4 +- src/uu/rm/src/rm.rs | 2 +- src/uu/seq/src/seq.rs | 8 +-- src/uu/shred/src/shred.rs | 10 ++- src/uu/sum/src/sum.rs | 2 +- src/uu/tac/src/tac.rs | 2 +- src/uu/test/src/test.rs | 24 +++---- src/uu/tr/src/expand.rs | 2 +- src/uu/tsort/src/tsort.rs | 17 +++-- src/uu/wc/src/count_bytes.rs | 31 ++++----- src/uu/wc/src/wc.rs | 19 +++--- 27 files changed, 151 insertions(+), 166 deletions(-) diff --git a/src/uu/basename/src/basename.rs b/src/uu/basename/src/basename.rs index 84521bdd1..b7f99af27 100644 --- a/src/uu/basename/src/basename.rs +++ b/src/uu/basename/src/basename.rs @@ -113,12 +113,8 @@ fn basename(fullname: &str, suffix: &str) -> String { fn strip_suffix(name: &str, suffix: &str) -> String { if name == suffix { - return name.to_owned(); + name.to_owned() + } else { + name.strip_suffix(suffix).unwrap_or(name).to_owned() } - - if name.ends_with(suffix) { - return name[..name.len() - suffix.len()].to_owned(); - } - - name.to_owned() } diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 569ee78bc..60484a79a 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -547,14 +547,13 @@ impl FromStr for Attribute { } fn add_all_attributes() -> Vec { - let mut attr = Vec::new(); + use Attribute::*; + + let mut attr = vec![Ownership, Timestamps, Context, Xattr, Links]; + #[cfg(unix)] - attr.push(Attribute::Mode); - attr.push(Attribute::Ownership); - attr.push(Attribute::Timestamps); - attr.push(Attribute::Context); - attr.push(Attribute::Xattr); - attr.push(Attribute::Links); + attr.insert(0, Mode); + attr } @@ -714,7 +713,7 @@ fn parse_path_args(path_args: &[String], options: &Options) -> CopyResult<(Vec, - source: &std::path::PathBuf, + source: &std::path::Path, dest: std::path::PathBuf, found_hard_link: &mut bool, ) -> CopyResult<()> { @@ -1068,6 +1067,7 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu } #[cfg(not(windows))] +#[allow(clippy::unnecessary_wraps)] // needed for windows version fn symlink_file(source: &Path, dest: &Path, context: &str) -> CopyResult<()> { match std::os::unix::fs::symlink(source, dest).context(context) { Ok(_) => Ok(()), diff --git a/src/uu/cut/src/cut.rs b/src/uu/cut/src/cut.rs index 6b09b91d9..5bf310daa 100644 --- a/src/uu/cut/src/cut.rs +++ b/src/uu/cut/src/cut.rs @@ -406,7 +406,7 @@ fn cut_files(mut filenames: Vec, mode: Mode) -> i32 { continue; } - if !path.metadata().is_ok() { + if path.metadata().is_err() { show_error!("{}: No such file or directory", filename); continue; } @@ -487,7 +487,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("filter field columns from the input source") .takes_value(true) .allow_hyphen_values(true) - .value_name("LIST") + .value_name("LIST") .display_order(4), ) .arg( @@ -535,40 +535,36 @@ pub fn uumain(args: impl uucore::Args) -> i32 { matches.value_of(options::CHARACTERS), matches.value_of(options::FIELDS), ) { - (Some(byte_ranges), None, None) => { - list_to_ranges(&byte_ranges[..], complement).map(|ranges| { - Mode::Bytes( - ranges, - Options { - out_delim: Some( - matches - .value_of(options::OUTPUT_DELIMITER) - .unwrap_or_default() - .to_owned(), - ), - zero_terminated: matches.is_present(options::ZERO_TERMINATED), - }, - ) - }) - } - (None, Some(char_ranges), None) => { - list_to_ranges(&char_ranges[..], complement).map(|ranges| { - Mode::Characters( - ranges, - Options { - out_delim: Some( - matches - .value_of(options::OUTPUT_DELIMITER) - .unwrap_or_default() - .to_owned(), - ), - zero_terminated: matches.is_present(options::ZERO_TERMINATED), - }, - ) - }) - } + (Some(byte_ranges), None, None) => list_to_ranges(byte_ranges, complement).map(|ranges| { + Mode::Bytes( + ranges, + Options { + out_delim: Some( + matches + .value_of(options::OUTPUT_DELIMITER) + .unwrap_or_default() + .to_owned(), + ), + zero_terminated: matches.is_present(options::ZERO_TERMINATED), + }, + ) + }), + (None, Some(char_ranges), None) => list_to_ranges(char_ranges, complement).map(|ranges| { + Mode::Characters( + ranges, + Options { + out_delim: Some( + matches + .value_of(options::OUTPUT_DELIMITER) + .unwrap_or_default() + .to_owned(), + ), + zero_terminated: matches.is_present(options::ZERO_TERMINATED), + }, + ) + }), (None, None, Some(field_ranges)) => { - list_to_ranges(&field_ranges[..], complement).and_then(|ranges| { + list_to_ranges(field_ranges, complement).and_then(|ranges| { let out_delim = match matches.value_of(options::OUTPUT_DELIMITER) { Some(s) => { if s.is_empty() { diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 615b66a4e..07635881a 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -322,7 +322,7 @@ fn convert_size_human(size: u64, multiplier: u64, _block_size: u64) -> String { } } if size == 0 { - return format!("0"); + return "0".to_string(); } format!("{}B", size) } diff --git a/src/uu/expr/src/expr.rs b/src/uu/expr/src/expr.rs index fee85dfe1..4a13812d3 100644 --- a/src/uu/expr/src/expr.rs +++ b/src/uu/expr/src/expr.rs @@ -51,7 +51,7 @@ fn print_expr_error(expr_error: &str) -> ! { crash!(2, "{}", expr_error) } -fn evaluate_ast(maybe_ast: Result, String>) -> Result { +fn evaluate_ast(maybe_ast: Result, String>) -> Result { if maybe_ast.is_err() { Err(maybe_ast.err().unwrap()) } else { diff --git a/src/uu/expr/src/syntax_tree.rs b/src/uu/expr/src/syntax_tree.rs index 3381c29bd..c81adf0c8 100644 --- a/src/uu/expr/src/syntax_tree.rs +++ b/src/uu/expr/src/syntax_tree.rs @@ -17,10 +17,10 @@ use onig::{Regex, RegexOptions, Syntax}; use crate::tokens::Token; type TokenStack = Vec<(usize, Token)>; -pub type OperandsList = Vec>; +pub type OperandsList = Vec>; #[derive(Debug)] -pub enum ASTNode { +pub enum AstNode { Leaf { token_idx: usize, value: String, @@ -31,7 +31,7 @@ pub enum ASTNode { operands: OperandsList, }, } -impl ASTNode { +impl AstNode { fn debug_dump(&self) { self.debug_dump_impl(1); } @@ -40,7 +40,7 @@ impl ASTNode { print!("\t",); } match *self { - ASTNode::Leaf { + AstNode::Leaf { ref token_idx, ref value, } => println!( @@ -49,7 +49,7 @@ impl ASTNode { token_idx, self.evaluate() ), - ASTNode::Node { + AstNode::Node { ref token_idx, ref op_type, ref operands, @@ -67,23 +67,23 @@ impl ASTNode { } } - fn new_node(token_idx: usize, op_type: &str, operands: OperandsList) -> Box { - Box::new(ASTNode::Node { + fn new_node(token_idx: usize, op_type: &str, operands: OperandsList) -> Box { + Box::new(AstNode::Node { token_idx, op_type: op_type.into(), operands, }) } - fn new_leaf(token_idx: usize, value: &str) -> Box { - Box::new(ASTNode::Leaf { + fn new_leaf(token_idx: usize, value: &str) -> Box { + Box::new(AstNode::Leaf { token_idx, value: value.into(), }) } pub fn evaluate(&self) -> Result { match *self { - ASTNode::Leaf { ref value, .. } => Ok(value.clone()), - ASTNode::Node { ref op_type, .. } => match self.operand_values() { + AstNode::Leaf { ref value, .. } => Ok(value.clone()), + AstNode::Node { ref op_type, .. } => match self.operand_values() { Err(reason) => Err(reason), Ok(operand_values) => match op_type.as_ref() { "+" => infix_operator_two_ints( @@ -161,7 +161,7 @@ impl ASTNode { } } pub fn operand_values(&self) -> Result, String> { - if let ASTNode::Node { ref operands, .. } = *self { + if let AstNode::Node { ref operands, .. } = *self { let mut out = Vec::with_capacity(operands.len()); for operand in operands { match operand.evaluate() { @@ -178,7 +178,7 @@ impl ASTNode { pub fn tokens_to_ast( maybe_tokens: Result, String>, -) -> Result, String> { +) -> Result, String> { if maybe_tokens.is_err() { Err(maybe_tokens.err().unwrap()) } else { @@ -212,7 +212,7 @@ pub fn tokens_to_ast( } } -fn maybe_dump_ast(result: &Result, String>) { +fn maybe_dump_ast(result: &Result, String>) { use std::env; if let Ok(debug_var) = env::var("EXPR_DEBUG_AST") { if debug_var == "1" { @@ -238,11 +238,11 @@ fn maybe_dump_rpn(rpn: &TokenStack) { } } -fn ast_from_rpn(rpn: &mut TokenStack) -> Result, String> { +fn ast_from_rpn(rpn: &mut TokenStack) -> Result, String> { match rpn.pop() { None => Err("syntax error (premature end of expression)".to_owned()), - Some((token_idx, Token::Value { value })) => Ok(ASTNode::new_leaf(token_idx, &value)), + Some((token_idx, Token::Value { value })) => Ok(AstNode::new_leaf(token_idx, &value)), Some((token_idx, Token::InfixOp { value, .. })) => { maybe_ast_node(token_idx, &value, 2, rpn) @@ -262,7 +262,7 @@ fn maybe_ast_node( op_type: &str, arity: usize, rpn: &mut TokenStack, -) -> Result, String> { +) -> Result, String> { let mut operands = Vec::with_capacity(arity); for _ in 0..arity { match ast_from_rpn(rpn) { @@ -271,7 +271,7 @@ fn maybe_ast_node( } } operands.reverse(); - Ok(ASTNode::new_node(token_idx, op_type, operands)) + Ok(AstNode::new_node(token_idx, op_type, operands)) } fn move_rest_of_ops_to_out( diff --git a/src/uu/fmt/src/parasplit.rs b/src/uu/fmt/src/parasplit.rs index f74a25413..950b3f66d 100644 --- a/src/uu/fmt/src/parasplit.rs +++ b/src/uu/fmt/src/parasplit.rs @@ -267,7 +267,7 @@ impl<'a> ParagraphStream<'a> { #[allow(clippy::match_like_matches_macro)] // `matches!(...)` macro not stabilized until rust v1.42 l_slice[..colon_posn].chars().all(|x| match x as usize { - y if y < 33 || y > 126 => false, + y if !(33..=126).contains(&y) => false, _ => true, }) } diff --git a/src/uu/fold/src/fold.rs b/src/uu/fold/src/fold.rs index c35e996f2..fa703eade 100644 --- a/src/uu/fold/src/fold.rs +++ b/src/uu/fold/src/fold.rs @@ -66,7 +66,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .takes_value(true), ) .arg(Arg::with_name(options::FILE).hidden(true).multiple(true)) - .get_matches_from(args.clone()); + .get_matches_from(args); let bytes = matches.is_present(options::BYTES); let spaces = matches.is_present(options::SPACES); diff --git a/src/uu/head/src/head.rs b/src/uu/head/src/head.rs index 3500af544..807d04314 100644 --- a/src/uu/head/src/head.rs +++ b/src/uu/head/src/head.rs @@ -625,7 +625,7 @@ mod tests { assert_eq!(arg_outputs("head"), Ok("head".to_owned())); } #[test] - #[cfg(linux)] + #[cfg(target_os = "linux")] fn test_arg_iterate_bad_encoding() { let invalid = unsafe { std::str::from_utf8_unchecked(b"\x80\x81") }; // this arises from a conversion from OsString to &str diff --git a/src/uu/ln/src/ln.rs b/src/uu/ln/src/ln.rs index 96a0df813..04358a415 100644 --- a/src/uu/ln/src/ln.rs +++ b/src/uu/ln/src/ln.rs @@ -303,7 +303,7 @@ fn exec(files: &[PathBuf], settings: &Settings) -> i32 { } } -fn link_files_in_dir(files: &[PathBuf], target_dir: &PathBuf, settings: &Settings) -> i32 { +fn link_files_in_dir(files: &[PathBuf], target_dir: &Path, settings: &Settings) -> i32 { if !target_dir.is_dir() { show_error!("target '{}' is not a directory", target_dir.display()); return 1; @@ -329,7 +329,7 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &PathBuf, settings: &Setting }; } } - target_dir.clone() + target_dir.to_path_buf() } else { match srcpath.as_os_str().to_str() { Some(name) => { @@ -370,7 +370,7 @@ fn link_files_in_dir(files: &[PathBuf], target_dir: &PathBuf, settings: &Setting } } -fn relative_path<'a>(src: &PathBuf, dst: &PathBuf) -> Result> { +fn relative_path<'a>(src: &Path, dst: &Path) -> Result> { let abssrc = canonicalize(src, CanonicalizeMode::Normal)?; let absdst = canonicalize(dst, CanonicalizeMode::Normal)?; let suffix_pos = abssrc @@ -390,7 +390,7 @@ fn relative_path<'a>(src: &PathBuf, dst: &PathBuf) -> Result> { Ok(result.into()) } -fn link(src: &PathBuf, dst: &PathBuf, settings: &Settings) -> Result<()> { +fn link(src: &Path, dst: &Path, settings: &Settings) -> Result<()> { let mut backup_path = None; let source: Cow<'_, Path> = if settings.relative { relative_path(&src, dst)? @@ -453,13 +453,13 @@ fn read_yes() -> bool { } } -fn simple_backup_path(path: &PathBuf, suffix: &str) -> PathBuf { +fn simple_backup_path(path: &Path, suffix: &str) -> PathBuf { let mut p = path.as_os_str().to_str().unwrap().to_owned(); p.push_str(suffix); PathBuf::from(p) } -fn numbered_backup_path(path: &PathBuf) -> PathBuf { +fn numbered_backup_path(path: &Path) -> PathBuf { let mut i: u64 = 1; loop { let new_path = simple_backup_path(path, &format!(".~{}~", i)); @@ -470,7 +470,7 @@ fn numbered_backup_path(path: &PathBuf) -> PathBuf { } } -fn existing_backup_path(path: &PathBuf, suffix: &str) -> PathBuf { +fn existing_backup_path(path: &Path, suffix: &str) -> PathBuf { let test_path = simple_backup_path(path, &".~1~".to_owned()); if test_path.exists() { return numbered_backup_path(path); diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index fdc11144a..4024328b5 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1076,7 +1076,7 @@ fn should_display(entry: &DirEntry, config: &Config) -> bool { true } -fn enter_directory(dir: &PathBuf, config: &Config) { +fn enter_directory(dir: &Path, config: &Config) { let mut entries: Vec<_> = safe_unwrap!(fs::read_dir(dir).and_then(Iterator::collect)); entries.retain(|e| should_display(e, config)); @@ -1101,7 +1101,7 @@ fn enter_directory(dir: &PathBuf, config: &Config) { } } -fn get_metadata(entry: &PathBuf, config: &Config) -> std::io::Result { +fn get_metadata(entry: &Path, config: &Config) -> std::io::Result { if config.dereference { entry.metadata().or_else(|_| entry.symlink_metadata()) } else { @@ -1109,7 +1109,7 @@ fn get_metadata(entry: &PathBuf, config: &Config) -> std::io::Result { } } -fn display_dir_entry_size(entry: &PathBuf, config: &Config) -> (usize, usize) { +fn display_dir_entry_size(entry: &Path, config: &Config) -> (usize, usize) { if let Ok(md) = get_metadata(entry, config) { ( display_symlink_count(&md).len(), @@ -1204,7 +1204,7 @@ fn display_grid(names: impl Iterator, width: u16, direction: Direct use uucore::fs::display_permissions; fn display_item_long( - item: &PathBuf, + item: &Path, strip: Option<&Path>, max_links: usize, max_size: usize, diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index b481aeebc..f57178a09 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -335,7 +335,7 @@ fn exec(files: &[PathBuf], b: Behavior) -> i32 { 0 } -fn move_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behavior) -> i32 { +fn move_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i32 { if !target_dir.is_dir() { show_error!("target ‘{}’ is not a directory", target_dir.display()); return 1; @@ -373,7 +373,7 @@ fn move_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behavior) -> } } -fn rename(from: &PathBuf, to: &PathBuf, b: &Behavior) -> io::Result<()> { +fn rename(from: &Path, to: &Path, b: &Behavior) -> io::Result<()> { let mut backup_path = None; if to.exists() { @@ -429,7 +429,7 @@ fn rename(from: &PathBuf, to: &PathBuf, b: &Behavior) -> io::Result<()> { /// A wrapper around `fs::rename`, so that if it fails, we try falling back on /// copying and removing. -fn rename_with_fallback(from: &PathBuf, to: &PathBuf) -> io::Result<()> { +fn rename_with_fallback(from: &Path, to: &Path) -> io::Result<()> { if fs::rename(from, to).is_err() { // Get metadata without following symlinks let metadata = from.symlink_metadata()?; @@ -464,7 +464,7 @@ fn rename_with_fallback(from: &PathBuf, to: &PathBuf) -> io::Result<()> { /// Move the given symlink to the given destination. On Windows, dangling /// symlinks return an error. #[inline] -fn rename_symlink_fallback(from: &PathBuf, to: &PathBuf) -> io::Result<()> { +fn rename_symlink_fallback(from: &Path, to: &Path) -> io::Result<()> { let path_symlink_points_to = fs::read_link(from)?; #[cfg(unix)] { @@ -507,20 +507,20 @@ fn read_yes() -> bool { } } -fn simple_backup_path(path: &PathBuf, suffix: &str) -> PathBuf { +fn simple_backup_path(path: &Path, suffix: &str) -> PathBuf { let mut p = path.to_string_lossy().into_owned(); p.push_str(suffix); PathBuf::from(p) } -fn numbered_backup_path(path: &PathBuf) -> PathBuf { +fn numbered_backup_path(path: &Path) -> PathBuf { (1_u64..) .map(|i| path.with_extension(format!("~{}~", i))) .find(|p| !p.exists()) .expect("cannot create backup") } -fn existing_backup_path(path: &PathBuf, suffix: &str) -> PathBuf { +fn existing_backup_path(path: &Path, suffix: &str) -> PathBuf { let test_path = path.with_extension("~1~"); if test_path.exists() { numbered_backup_path(path) @@ -529,7 +529,7 @@ fn existing_backup_path(path: &PathBuf, suffix: &str) -> PathBuf { } } -fn is_empty_dir(path: &PathBuf) -> bool { +fn is_empty_dir(path: &Path) -> bool { match fs::read_dir(path) { Ok(contents) => contents.peekable().peek().is_none(), Err(_e) => false, diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index c3b39fca1..36eae66ab 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -118,7 +118,7 @@ struct OdOptions { } impl OdOptions { - fn new<'a>(matches: ArgMatches<'a>, args: Vec) -> Result { + fn new(matches: ArgMatches, args: Vec) -> Result { let byte_order = match matches.value_of(options::ENDIAN) { None => ByteOrder::Native, Some("little") => ByteOrder::Little, diff --git a/src/uu/od/src/parse_inputs.rs b/src/uu/od/src/parse_inputs.rs index 915aa1d92..533f4f106 100644 --- a/src/uu/od/src/parse_inputs.rs +++ b/src/uu/od/src/parse_inputs.rs @@ -63,7 +63,7 @@ pub fn parse_inputs(matches: &dyn CommandLineOpts) -> Result) -> Result Ok(CommandLineInputs::FileAndOffset(( - input_strings[0].clone().to_owned(), + input_strings[0].to_string(), m, None, ))), @@ -118,7 +118,7 @@ pub fn parse_inputs_traditional(input_strings: Vec<&str>) -> Result Ok(CommandLineInputs::FileAndOffset(( - input_strings[0].clone().to_owned(), + input_strings[0].to_string(), n, Some(m), ))), diff --git a/src/uu/printf/src/tokenize/num_format/formatters/base_conv/mod.rs b/src/uu/printf/src/tokenize/num_format/formatters/base_conv/mod.rs index 79af9abd5..04d33b52c 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/base_conv/mod.rs +++ b/src/uu/printf/src/tokenize/num_format/formatters/base_conv/mod.rs @@ -199,8 +199,7 @@ pub fn arrnum_int_add(arrnum: &[u8], basenum: u8, base_ten_int_term: u8) -> Vec< } pub fn base_conv_vec(src: &[u8], radix_src: u8, radix_dest: u8) -> Vec { - let mut result: Vec = Vec::new(); - result.push(0); + let mut result = vec![0]; for i in src { result = arrnum_int_mult(&result, radix_dest, radix_src); result = arrnum_int_add(&result, radix_dest, *i); @@ -226,8 +225,7 @@ pub fn base_conv_float(src: &[u8], radix_src: u8, radix_dest: u8) -> f64 { // to implement this for arbitrary string input. // until then, the below operates as an outline // of how it would work. - let mut result: Vec = Vec::new(); - result.push(0); + let result: Vec = vec![0]; let mut factor: f64 = 1_f64; let radix_src_float: f64 = f64::from(radix_src); let mut r: f64 = 0_f64; diff --git a/src/uu/readlink/src/readlink.rs b/src/uu/readlink/src/readlink.rs index 727c2cce5..43a4ca656 100644 --- a/src/uu/readlink/src/readlink.rs +++ b/src/uu/readlink/src/readlink.rs @@ -13,7 +13,7 @@ extern crate uucore; use clap::{App, Arg}; use std::fs; use std::io::{stdout, Write}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use uucore::fs::{canonicalize, CanonicalizeMode}; const NAME: &str = "readlink"; @@ -160,8 +160,8 @@ pub fn uumain(args: impl uucore::Args) -> i32 { 0 } -fn show(path: &PathBuf, no_newline: bool, use_zero: bool) { - let path = path.as_path().to_str().unwrap(); +fn show(path: &Path, no_newline: bool, use_zero: bool) { + let path = path.to_str().unwrap(); if use_zero { print!("{}\0", path); } else if no_newline { diff --git a/src/uu/realpath/src/realpath.rs b/src/uu/realpath/src/realpath.rs index 5cc8f3d9a..37ff70fb2 100644 --- a/src/uu/realpath/src/realpath.rs +++ b/src/uu/realpath/src/realpath.rs @@ -12,7 +12,7 @@ extern crate uucore; use clap::{App, Arg}; use std::fs; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use uucore::fs::{canonicalize, CanonicalizeMode}; static ABOUT: &str = "print the resolved path"; @@ -82,7 +82,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { retcode } -fn resolve_path(p: &PathBuf, strip: bool, zero: bool, quiet: bool) -> bool { +fn resolve_path(p: &Path, strip: bool, zero: bool, quiet: bool) -> bool { let abs = canonicalize(p, CanonicalizeMode::Normal).unwrap(); if strip { diff --git a/src/uu/rm/src/rm.rs b/src/uu/rm/src/rm.rs index 09671768b..94626b4e7 100644 --- a/src/uu/rm/src/rm.rs +++ b/src/uu/rm/src/rm.rs @@ -176,7 +176,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } else if matches.is_present(OPT_PROMPT_MORE) { InteractiveMode::Once } else if matches.is_present(OPT_INTERACTIVE) { - match &matches.value_of(OPT_INTERACTIVE).unwrap()[..] { + match matches.value_of(OPT_INTERACTIVE).unwrap() { "none" => InteractiveMode::None, "once" => InteractiveMode::Once, "always" => InteractiveMode::Always, diff --git a/src/uu/seq/src/seq.rs b/src/uu/seq/src/seq.rs index 671dd7e1c..c3bba1c78 100644 --- a/src/uu/seq/src/seq.rs +++ b/src/uu/seq/src/seq.rs @@ -102,7 +102,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let mut largest_dec = 0; let mut padding = 0; let first = if numbers.len() > 1 { - let slice = &numbers[0][..]; + let slice = numbers[0]; let len = slice.len(); let dec = slice.find('.').unwrap_or(len); largest_dec = len - dec; @@ -118,7 +118,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { 1.0 }; let increment = if numbers.len() > 2 { - let slice = &numbers[1][..]; + let slice = numbers[1]; let len = slice.len(); let dec = slice.find('.').unwrap_or(len); largest_dec = cmp::max(largest_dec, len - dec); @@ -134,11 +134,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 { 1.0 }; if increment == 0.0 { - show_error!("increment value: '{}'", &numbers[1][..]); + show_error!("increment value: '{}'", numbers[1]); return 1; } let last = { - let slice = &numbers[numbers.len() - 1][..]; + let slice = numbers[numbers.len() - 1]; padding = cmp::max(padding, slice.find('.').unwrap_or_else(|| slice.len())); match parse_float(slice) { Ok(n) => n, diff --git a/src/uu/shred/src/shred.rs b/src/uu/shred/src/shred.rs index 7e0e77184..c56f14280 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -439,6 +439,7 @@ fn pass_name(pass_type: PassType) -> String { } } +#[allow(clippy::too_many_arguments)] fn wipe_file( path_str: &str, n_passes: usize, @@ -472,12 +473,9 @@ fn wipe_file( let mut perms = metadata.permissions(); perms.set_readonly(false); - match fs::set_permissions(path, perms) { - Err(e) => { - show_error!("{}", e); - return; - } - _ => {} + if let Err(e) = fs::set_permissions(path, perms) { + show_error!("{}", e); + return; } } diff --git a/src/uu/sum/src/sum.rs b/src/uu/sum/src/sum.rs index ed5655a3d..d0fbc7c0d 100644 --- a/src/uu/sum/src/sum.rs +++ b/src/uu/sum/src/sum.rs @@ -75,7 +75,7 @@ fn open(name: &str) -> Result> { "Is a directory", )); }; - if !path.metadata().is_ok() { + if path.metadata().is_err() { return Err(std::io::Error::new( std::io::ErrorKind::NotFound, "No such file or directory", diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index 68dae94e2..666ba3384 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -90,7 +90,7 @@ fn tac(filenames: Vec, before: bool, _: bool, separator: &str) -> i32 { Box::new(stdin()) as Box } else { let path = Path::new(filename); - if path.is_dir() || !path.metadata().is_ok() { + if path.is_dir() || path.metadata().is_err() { show_error!( "failed to open '{}' for reading: No such file or directory", filename diff --git a/src/uu/test/src/test.rs b/src/uu/test/src/test.rs index 4394e4a8e..f882ff5ae 100644 --- a/src/uu/test/src/test.rs +++ b/src/uu/test/src/test.rs @@ -55,16 +55,16 @@ fn two(args: &[&[u8]], error: &mut bool) -> bool { b"-d" => path(args[1], PathCondition::Directory), b"-e" => path(args[1], PathCondition::Exists), b"-f" => path(args[1], PathCondition::Regular), - b"-g" => path(args[1], PathCondition::GroupIDFlag), + b"-g" => path(args[1], PathCondition::GroupIdFlag), b"-h" => path(args[1], PathCondition::SymLink), b"-L" => path(args[1], PathCondition::SymLink), b"-n" => one(&args[1..]), - b"-p" => path(args[1], PathCondition::FIFO), + b"-p" => path(args[1], PathCondition::Fifo), b"-r" => path(args[1], PathCondition::Readable), b"-S" => path(args[1], PathCondition::Socket), b"-s" => path(args[1], PathCondition::NonEmpty), b"-t" => isatty(args[1]), - b"-u" => path(args[1], PathCondition::UserIDFlag), + b"-u" => path(args[1], PathCondition::UserIdFlag), b"-w" => path(args[1], PathCondition::Writable), b"-x" => path(args[1], PathCondition::Executable), b"-z" => !one(&args[1..]), @@ -322,13 +322,13 @@ enum PathCondition { Directory, Exists, Regular, - GroupIDFlag, + GroupIdFlag, SymLink, - FIFO, + Fifo, Readable, Socket, NonEmpty, - UserIDFlag, + UserIdFlag, Writable, Executable, } @@ -390,13 +390,13 @@ fn path(path: &[u8], cond: PathCondition) -> bool { PathCondition::Directory => file_type.is_dir(), PathCondition::Exists => true, PathCondition::Regular => file_type.is_file(), - PathCondition::GroupIDFlag => metadata.mode() & S_ISGID != 0, + PathCondition::GroupIdFlag => metadata.mode() & S_ISGID != 0, PathCondition::SymLink => metadata.file_type().is_symlink(), - PathCondition::FIFO => file_type.is_fifo(), + PathCondition::Fifo => file_type.is_fifo(), PathCondition::Readable => perm(metadata, Permission::Read), PathCondition::Socket => file_type.is_socket(), PathCondition::NonEmpty => metadata.size() > 0, - PathCondition::UserIDFlag => metadata.mode() & S_ISUID != 0, + PathCondition::UserIdFlag => metadata.mode() & S_ISUID != 0, PathCondition::Writable => perm(metadata, Permission::Write), PathCondition::Executable => perm(metadata, Permission::Execute), } @@ -416,13 +416,13 @@ fn path(path: &[u8], cond: PathCondition) -> bool { PathCondition::Directory => stat.is_dir(), PathCondition::Exists => true, PathCondition::Regular => stat.is_file(), - PathCondition::GroupIDFlag => false, + PathCondition::GroupIdFlag => false, PathCondition::SymLink => false, - PathCondition::FIFO => false, + PathCondition::Fifo => false, PathCondition::Readable => false, // TODO PathCondition::Socket => false, PathCondition::NonEmpty => stat.len() > 0, - PathCondition::UserIDFlag => false, + PathCondition::UserIdFlag => false, PathCondition::Writable => false, // TODO PathCondition::Executable => false, // TODO } diff --git a/src/uu/tr/src/expand.rs b/src/uu/tr/src/expand.rs index e71cf262c..73612a065 100644 --- a/src/uu/tr/src/expand.rs +++ b/src/uu/tr/src/expand.rs @@ -24,7 +24,7 @@ use std::ops::RangeInclusive; fn parse_sequence(s: &str) -> (char, usize) { let c = s.chars().next().expect("invalid escape: empty string"); - if '0' <= c && c <= '7' { + if ('0'..='7').contains(&c) { let mut v = c.to_digit(8).unwrap(); let mut consumed = 1; let bits_per_digit = 3; diff --git a/src/uu/tsort/src/tsort.rs b/src/uu/tsort/src/tsort.rs index 3440972a2..967a9514a 100644 --- a/src/uu/tsort/src/tsort.rs +++ b/src/uu/tsort/src/tsort.rs @@ -16,8 +16,8 @@ use std::io::{stdin, BufRead, BufReader, Read}; use std::path::Path; static VERSION: &str = env!("CARGO_PKG_VERSION"); -static SUMMARY: &str = "Topological sort the strings in FILE. -Strings are defined as any sequence of tokens separated by whitespace (tab, space, or newline). +static SUMMARY: &str = "Topological sort the strings in FILE. +Strings are defined as any sequence of tokens separated by whitespace (tab, space, or newline). If FILE is not passed in, stdin is used instead."; static USAGE: &str = "tsort [OPTIONS] FILE"; @@ -32,13 +32,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .version(VERSION) .usage(USAGE) .about(SUMMARY) - .arg(Arg::with_name(options::FILE).hidden(true)) + .arg( + Arg::with_name(options::FILE) + .default_value("-") + .hidden(true), + ) .get_matches_from(args); - let input = match matches.value_of(options::FILE) { - Some(v) => v, - None => "-", - }; + let input = matches + .value_of(options::FILE) + .expect("Value is required by clap"); let mut stdin_buf; let mut file_buf; diff --git a/src/uu/wc/src/count_bytes.rs b/src/uu/wc/src/count_bytes.rs index 0c3b5edb7..7f06f8171 100644 --- a/src/uu/wc/src/count_bytes.rs +++ b/src/uu/wc/src/count_bytes.rs @@ -72,30 +72,27 @@ pub(crate) fn count_bytes_fast(handle: &mut T) -> WcResult { - // If the file is regular, then the `st_size` should hold - // the file's size in bytes. - if (stat.st_mode & S_IFREG) != 0 { - return Ok(stat.st_size as usize); - } - #[cfg(any(target_os = "linux", target_os = "android"))] - { - // Else, if we're on Linux and our file is a FIFO pipe - // (or stdin), we use splice to count the number of bytes. - if (stat.st_mode & S_IFIFO) != 0 { - if let Ok(n) = count_bytes_using_splice(fd) { - return Ok(n); - } + if let Ok(stat) = fstat(fd) { + // If the file is regular, then the `st_size` should hold + // the file's size in bytes. + if (stat.st_mode & S_IFREG) != 0 { + return Ok(stat.st_size as usize); + } + #[cfg(any(target_os = "linux", target_os = "android"))] + { + // Else, if we're on Linux and our file is a FIFO pipe + // (or stdin), we use splice to count the number of bytes. + if (stat.st_mode & S_IFIFO) != 0 { + if let Ok(n) = count_bytes_using_splice(fd) { + return Ok(n); } } } - _ => {} } } // Fall back on `read`, but without the overhead of counting words and lines. - let mut buf = [0 as u8; BUF_SIZE]; + let mut buf = [0_u8; BUF_SIZE]; let mut byte_count = 0; loop { match handle.read(&mut buf) { diff --git a/src/uu/wc/src/wc.rs b/src/uu/wc/src/wc.rs index 22463caa4..59ca10141 100644 --- a/src/uu/wc/src/wc.rs +++ b/src/uu/wc/src/wc.rs @@ -138,11 +138,8 @@ impl AddAssign for WordCount { } impl WordCount { - fn with_title<'a>(self, title: &'a str) -> TitledWordCount<'a> { - return TitledWordCount { - title: title, - count: self, - }; + fn with_title(self, title: &str) -> TitledWordCount { + TitledWordCount { title, count: self } } } @@ -251,7 +248,7 @@ fn is_word_separator(byte: u8) -> bool { fn word_count_from_reader( mut reader: T, settings: &Settings, - path: &String, + path: &str, ) -> WcResult { let only_count_bytes = settings.show_bytes && (!(settings.show_chars @@ -333,18 +330,18 @@ fn word_count_from_reader( }) } -fn word_count_from_path(path: &String, settings: &Settings) -> WcResult { +fn word_count_from_path(path: &str, settings: &Settings) -> WcResult { if path == "-" { let stdin = io::stdin(); let stdin_lock = stdin.lock(); - return Ok(word_count_from_reader(stdin_lock, settings, path)?); + word_count_from_reader(stdin_lock, settings, path) } else { let path_obj = Path::new(path); if path_obj.is_dir() { - return Err(WcError::IsDirectory(path.clone())); + Err(WcError::IsDirectory(path.to_owned())) } else { let file = File::open(path)?; - return Ok(word_count_from_reader(file, settings, path)?); + word_count_from_reader(file, settings, path) } } } @@ -425,7 +422,7 @@ fn print_stats( } if result.title == "-" { - writeln!(stdout_lock, "")?; + writeln!(stdout_lock)?; } else { writeln!(stdout_lock, " {}", result.title)?; } From 75a76613e43308c53477b41575a33a01f7fdaa05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reto=20Habl=C3=BCtzel?= Date: Sun, 11 Apr 2021 16:09:18 +0200 Subject: [PATCH 04/15] fix clippy in cp --- src/uu/cp/src/cp.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 60484a79a..4e245b298 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -132,7 +132,9 @@ macro_rules! prompt_yes( pub type CopyResult = Result; pub type Source = PathBuf; +pub type SourceSlice = Path; pub type Target = PathBuf; +pub type TargetSlice = Path; /// Specifies whether when overwrite files #[derive(Clone, Eq, PartialEq)] @@ -664,7 +666,7 @@ impl TargetType { /// /// Treat target as a dir if we have multiple sources or the target /// exists and already is a directory - fn determine(sources: &[Source], target: &Target) -> TargetType { + fn determine(sources: &[Source], target: &TargetSlice) -> TargetType { if sources.len() > 1 || target.is_dir() { TargetType::Directory } else { @@ -787,7 +789,7 @@ fn preserve_hardlinks( /// Behavior depends on `options`, see [`Options`] for details. /// /// [`Options`]: ./struct.Options.html -fn copy(sources: &[Source], target: &Target, options: &Options) -> CopyResult<()> { +fn copy(sources: &[Source], target: &TargetSlice, options: &Options) -> CopyResult<()> { let target_type = TargetType::determine(sources, target); verify_target_type(target, &target_type)?; @@ -839,7 +841,7 @@ fn copy(sources: &[Source], target: &Target, options: &Options) -> CopyResult<() fn construct_dest_path( source_path: &Path, - target: &Target, + target: &TargetSlice, target_type: &TargetType, options: &Options, ) -> CopyResult { @@ -869,8 +871,8 @@ fn construct_dest_path( } fn copy_source( - source: &Source, - target: &Target, + source: &SourceSlice, + target: &TargetSlice, target_type: &TargetType, options: &Options, ) -> CopyResult<()> { @@ -911,7 +913,7 @@ fn adjust_canonicalization(p: &Path) -> Cow { /// /// Any errors encountered copying files in the tree will be logged but /// will not cause a short-circuit. -fn copy_directory(root: &Path, target: &Target, options: &Options) -> CopyResult<()> { +fn copy_directory(root: &Path, target: &TargetSlice, options: &Options) -> CopyResult<()> { if !options.recursive { return Err(format!("omitting directory '{}'", root.display()).into()); } From b465c34eeffc99ba6d5b390ed92209c08baa11e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reto=20Habl=C3=BCtzel?= Date: Sun, 11 Apr 2021 16:16:38 +0200 Subject: [PATCH 05/15] fix ls --- src/uu/ls/src/ls.rs | 2 +- src/uu/ls/src/version_cmp.rs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 4024328b5..d198f1588 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1042,7 +1042,7 @@ fn sort_entries(entries: &mut Vec, config: &Config) { .sort_by_key(|k| Reverse(get_metadata(k, config).map(|md| md.len()).unwrap_or(0))), // The default sort in GNU ls is case insensitive Sort::Name => entries.sort_by_key(|k| k.to_string_lossy().to_lowercase()), - Sort::Version => entries.sort_by(version_cmp::version_cmp), + Sort::Version => entries.sort_by(|a, b| version_cmp::version_cmp(a, b)), Sort::None => {} } diff --git a/src/uu/ls/src/version_cmp.rs b/src/uu/ls/src/version_cmp.rs index 3cd5989f1..4cd39f916 100644 --- a/src/uu/ls/src/version_cmp.rs +++ b/src/uu/ls/src/version_cmp.rs @@ -1,8 +1,9 @@ -use std::{cmp::Ordering, path::PathBuf}; +use std::cmp::Ordering; +use std::path::Path; -/// Compare pathbufs in a way that matches the GNU version sort, meaning that +/// Compare paths in a way that matches the GNU version sort, meaning that /// numbers get sorted in a natural way. -pub(crate) fn version_cmp(a: &PathBuf, b: &PathBuf) -> Ordering { +pub(crate) fn version_cmp(a: &Path, b: &Path) -> Ordering { let a_string = a.to_string_lossy(); let b_string = b.to_string_lossy(); let mut a = a_string.chars().peekable(); From d67560c37ac6e2a4443920ced20b788249fba45e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reto=20Habl=C3=BCtzel?= Date: Sun, 11 Apr 2021 16:34:19 +0200 Subject: [PATCH 06/15] fix clippy for unix --- src/uu/chgrp/src/chgrp.rs | 2 +- src/uu/chmod/src/chmod.rs | 8 ++++---- src/uu/chown/src/chown.rs | 4 ++-- src/uu/chroot/src/chroot.rs | 14 +++++++------- src/uu/install/src/install.rs | 18 +++++++++--------- src/uu/ls/src/ls.rs | 1 + src/uu/pinky/src/pinky.rs | 12 +++--------- src/uu/stdbuf/src/libstdbuf/src/libstdbuf.rs | 4 ++-- src/uu/stdbuf/src/stdbuf.rs | 6 ++---- src/uu/touch/src/touch.rs | 2 +- src/uu/tty/src/tty.rs | 4 ++-- src/uucore/src/lib/features/perms.rs | 4 ++-- 12 files changed, 36 insertions(+), 43 deletions(-) diff --git a/src/uu/chgrp/src/chgrp.rs b/src/uu/chgrp/src/chgrp.rs index b4c3360c5..592a0a905 100644 --- a/src/uu/chgrp/src/chgrp.rs +++ b/src/uu/chgrp/src/chgrp.rs @@ -286,7 +286,7 @@ impl Chgrper { ret = match wrap_chgrp(path, &meta, self.dest_gid, follow, self.verbosity.clone()) { Ok(n) => { - if n != "" { + if !n.is_empty() { show_info!("{}", n); } 0 diff --git a/src/uu/chmod/src/chmod.rs b/src/uu/chmod/src/chmod.rs index d9d8c8cf2..dc11be7b8 100644 --- a/src/uu/chmod/src/chmod.rs +++ b/src/uu/chmod/src/chmod.rs @@ -171,13 +171,13 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // of a prefix '-' if it's associated with MODE // e.g. "chmod -v -xw -R FILE" -> "chmod -v xw -R FILE" pub fn strip_minus_from_mode(args: &mut Vec) -> bool { - for i in 0..args.len() { - if args[i].starts_with("-") { - if let Some(second) = args[i].chars().nth(1) { + for arg in args { + if arg.starts_with('-') { + if let Some(second) = arg.chars().nth(1) { match second { 'r' | 'w' | 'x' | 'X' | 's' | 't' | 'u' | 'g' | 'o' | '0'..='7' => { // TODO: use strip_prefix() once minimum rust version reaches 1.45.0 - args[i] = args[i][1..args[i].len()].to_string(); + *arg = arg[1..arg.len()].to_string(); return true; } _ => {} diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 42010de03..0e3273b3b 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -391,7 +391,7 @@ impl Chowner { self.verbosity.clone(), ) { Ok(n) => { - if n != "" { + if !n.is_empty() { show_info!("{}", n); } 0 @@ -446,7 +446,7 @@ impl Chowner { self.verbosity.clone(), ) { Ok(n) => { - if n != "" { + if !n.is_empty() { show_info!("{}", n); } 0 diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs index 44c5dfa02..7e672da1e 100644 --- a/src/uu/chroot/src/chroot.rs +++ b/src/uu/chroot/src/chroot.rs @@ -104,7 +104,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { _ => { let mut vector: Vec<&str> = Vec::new(); for (&k, v) in matches.args.iter() { - vector.push(k.clone()); + vector.push(k); vector.push(&v.vals[0].to_str().unwrap()); } vector @@ -133,7 +133,7 @@ fn set_context(root: &Path, options: &clap::ArgMatches) { let userspec = match userspec_str { Some(ref u) => { let s: Vec<&str> = u.split(':').collect(); - if s.len() != 2 || s.iter().any(|&spec| spec == "") { + if s.len() != 2 || s.iter().any(|&spec| spec.is_empty()) { crash!(1, "invalid userspec: `{}`", u) }; s @@ -142,16 +142,16 @@ fn set_context(root: &Path, options: &clap::ArgMatches) { }; let (user, group) = if userspec.is_empty() { - (&user_str[..], &group_str[..]) + (user_str, group_str) } else { - (&userspec[0][..], &userspec[1][..]) + (userspec[0], userspec[1]) }; enter_chroot(root); - set_groups_from_str(&groups_str[..]); - set_main_group(&group[..]); - set_user(&user[..]); + set_groups_from_str(groups_str); + set_main_group(group); + set_user(user); } fn enter_chroot(root: &Path) { diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index e902862a8..a75ce45be 100644 --- a/src/uu/install/src/install.rs +++ b/src/uu/install/src/install.rs @@ -302,7 +302,7 @@ fn behavior(matches: &ArgMatches) -> Result { let specified_mode: Option = if matches.is_present(OPT_MODE) { match matches.value_of(OPT_MODE) { - Some(x) => match mode::parse(&x[..], considering_dir) { + Some(x) => match mode::parse(x, considering_dir) { Ok(y) => Some(y), Err(err) => { show_error!("Invalid mode string: {}", err); @@ -429,7 +429,7 @@ fn standard(paths: Vec, b: Behavior) -> i32 { /// _files_ must all exist as non-directories. /// _target_dir_ must be a directory. /// -fn copy_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behavior) -> i32 { +fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i32 { if !target_dir.is_dir() { show_error!("target '{}' is not a directory", target_dir.display()); return 1; @@ -453,7 +453,7 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behavior) -> continue; } - let mut targetpath = target_dir.clone().to_path_buf(); + let mut targetpath = target_dir.to_path_buf(); let filename = sourcepath.components().last().unwrap(); targetpath.push(filename); @@ -478,7 +478,7 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &PathBuf, b: &Behavior) -> /// _file_ must exist as a non-directory. /// _target_ must be a non-directory /// -fn copy_file_to_file(file: &PathBuf, target: &PathBuf, b: &Behavior) -> i32 { +fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> i32 { if copy(file, &target, b).is_err() { 1 } else { @@ -497,7 +497,7 @@ fn copy_file_to_file(file: &PathBuf, target: &PathBuf, b: &Behavior) -> i32 { /// /// If the copy system call fails, we print a verbose error and return an empty error value. /// -fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> { +fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> { if b.compare && !need_copy(from, to, b) { return Ok(()); } @@ -556,7 +556,7 @@ fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> { }; let gid = meta.gid(); match wrap_chown( - to.as_path(), + to, &meta, Some(owner_id), Some(gid), @@ -582,7 +582,7 @@ fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> { Ok(g) => g, _ => crash!(1, "no such group: {}", b.group), }; - match wrap_chgrp(to.as_path(), &meta, group_id, false, Verbosity::Normal) { + match wrap_chgrp(to, &meta, group_id, false, Verbosity::Normal) { Ok(n) => { if !n.is_empty() { show_info!("{}", n); @@ -601,7 +601,7 @@ fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> { let modified_time = FileTime::from_last_modification_time(&meta); let accessed_time = FileTime::from_last_access_time(&meta); - match set_file_times(to.as_path(), accessed_time, modified_time) { + match set_file_times(to, accessed_time, modified_time) { Ok(_) => {} Err(e) => show_info!("{}", e), } @@ -630,7 +630,7 @@ fn copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> Result<(), ()> { /// /// Crashes the program if a nonexistent owner or group is specified in _b_. /// -fn need_copy(from: &PathBuf, to: &PathBuf, b: &Behavior) -> bool { +fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool { let from_meta = match fs::metadata(from) { Ok(meta) => meta, Err(_) => return true, diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index d198f1588..aebaa6b44 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -370,6 +370,7 @@ impl Config { }) .or_else(|| termsize::get().map(|s| s.cols)); + #[allow(clippy::needless_bool)] let show_control = if options.is_present(options::HIDE_CONTROL_CHARS) { false } else if options.is_present(options::SHOW_CONTROL_CHARS) { diff --git a/src/uu/pinky/src/pinky.rs b/src/uu/pinky/src/pinky.rs index 772e311d6..851a3cd42 100644 --- a/src/uu/pinky/src/pinky.rs +++ b/src/uu/pinky/src/pinky.rs @@ -15,7 +15,6 @@ use uucore::utmpx::{self, time, Utmpx}; use std::io::prelude::*; use std::io::BufReader; -use std::io::Result as IOResult; use std::fs::File; use std::os::unix::fs::MetadataExt; @@ -136,12 +135,8 @@ The utmp file will be {}", }; if do_short_format { - if let Err(e) = pk.short_pinky() { - show_usage_error!("{}", e); - 1 - } else { - 0 - } + pk.short_pinky(); + 0 } else { pk.long_pinky() } @@ -282,7 +277,7 @@ impl Pinky { println!(); } - fn short_pinky(&self) -> IOResult<()> { + fn short_pinky(&self) { if self.include_heading { self.print_heading(); } @@ -295,7 +290,6 @@ impl Pinky { } } } - Ok(()) } fn long_pinky(&self) -> i32 { diff --git a/src/uu/stdbuf/src/libstdbuf/src/libstdbuf.rs b/src/uu/stdbuf/src/libstdbuf/src/libstdbuf.rs index fa36d4ab5..d08427d98 100644 --- a/src/uu/stdbuf/src/libstdbuf/src/libstdbuf.rs +++ b/src/uu/stdbuf/src/libstdbuf/src/libstdbuf.rs @@ -35,8 +35,8 @@ extern "C" { fn set_buffer(stream: *mut FILE, value: &str) { let (mode, size): (c_int, size_t) = match value { - "0" => (_IONBF, 0 as size_t), - "L" => (_IOLBF, 0 as size_t), + "0" => (_IONBF, 0_usize), + "L" => (_IOLBF, 0_usize), input => { let buff_size: usize = match input.parse() { Ok(num) => num, diff --git a/src/uu/stdbuf/src/stdbuf.rs b/src/uu/stdbuf/src/stdbuf.rs index a6c9f9dc5..8c65a5c7e 100644 --- a/src/uu/stdbuf/src/stdbuf.rs +++ b/src/uu/stdbuf/src/stdbuf.rs @@ -141,12 +141,10 @@ fn parse_size(size: &str) -> Option { fn check_option(matches: &ArgMatches, name: &str) -> Result { match matches.value_of(name) { - Some(value) => match &value[..] { + Some(value) => match value { "L" => { if name == options::INPUT { - Err(ProgramOptionsError(format!( - "line buffering stdin is meaningless" - ))) + Err(ProgramOptionsError("line buffering stdin is meaningless".to_string())) } else { Ok(BufferType::Line) } diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index 39405900e..f0c3c12d2 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -137,7 +137,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let (mut atime, mut mtime) = if matches.is_present(options::sources::REFERENCE) { stat( - &matches.value_of(options::sources::REFERENCE).unwrap()[..], + matches.value_of(options::sources::REFERENCE).unwrap(), !matches.is_present(options::NO_DEREF), ) } else if matches.is_present(options::sources::DATE) diff --git a/src/uu/tty/src/tty.rs b/src/uu/tty/src/tty.rs index 18d69db46..815c6f96b 100644 --- a/src/uu/tty/src/tty.rs +++ b/src/uu/tty/src/tty.rs @@ -65,9 +65,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { } } - return if is_stdin_interactive() { + if is_stdin_interactive() { libc::EXIT_SUCCESS } else { libc::EXIT_FAILURE - }; + } } diff --git a/src/uucore/src/lib/features/perms.rs b/src/uucore/src/lib/features/perms.rs index 66db15451..36f56206d 100644 --- a/src/uucore/src/lib/features/perms.rs +++ b/src/uucore/src/lib/features/perms.rs @@ -31,9 +31,9 @@ fn chgrp>(path: P, dgid: gid_t, follow: bool) -> IOResult<()> { let s = CString::new(path.as_os_str().as_bytes()).unwrap(); let ret = unsafe { if follow { - libc::chown(s.as_ptr(), (0 as gid_t).wrapping_sub(1), dgid) + libc::chown(s.as_ptr(), 0_u32.wrapping_sub(1), dgid) } else { - lchown(s.as_ptr(), (0 as gid_t).wrapping_sub(1), dgid) + lchown(s.as_ptr(), 0_u32.wrapping_sub(1), dgid) } }; if ret == 0 { From d219b6e705b8f48a73f19481aad4fdd9b25beca4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reto=20Habl=C3=BCtzel?= Date: Mon, 12 Apr 2021 19:50:23 +0200 Subject: [PATCH 07/15] strip_suffix is not avaialble with rust 1.40 --- src/uu/basename/src/basename.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/uu/basename/src/basename.rs b/src/uu/basename/src/basename.rs index b7f99af27..84521bdd1 100644 --- a/src/uu/basename/src/basename.rs +++ b/src/uu/basename/src/basename.rs @@ -113,8 +113,12 @@ fn basename(fullname: &str, suffix: &str) -> String { fn strip_suffix(name: &str, suffix: &str) -> String { if name == suffix { - name.to_owned() - } else { - name.strip_suffix(suffix).unwrap_or(name).to_owned() + return name.to_owned(); } + + if name.ends_with(suffix) { + return name[..name.len() - suffix.len()].to_owned(); + } + + name.to_owned() } From 07e9c5896c9dc72fa2c1ec5f20db2b7dc1e39327 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reto=20Habl=C3=BCtzel?= Date: Mon, 12 Apr 2021 19:53:47 +0200 Subject: [PATCH 08/15] ignore strip_suffix until minimum rust version is 1.45 --- src/uu/basename/src/basename.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/uu/basename/src/basename.rs b/src/uu/basename/src/basename.rs index 84521bdd1..7b02a7a83 100644 --- a/src/uu/basename/src/basename.rs +++ b/src/uu/basename/src/basename.rs @@ -111,6 +111,7 @@ fn basename(fullname: &str, suffix: &str) -> String { } } +#[allow(clippy::manual_strip)] // can be replaced with strip_suffix once the minimum rust version is 1.45 fn strip_suffix(name: &str, suffix: &str) -> String { if name == suffix { return name.to_owned(); From a4253d125497a98e15f4da27191053302a1870e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Reto=20Habl=C3=BCtzel?= Date: Mon, 12 Apr 2021 20:07:10 +0200 Subject: [PATCH 09/15] apply more clippy suggestions from nightly --- src/uu/df/src/df.rs | 5 ----- src/uu/hashsum/src/hashsum.rs | 14 +++++++------- .../printf/src/tokenize/num_format/num_format.rs | 6 +----- src/uu/ptx/src/ptx.rs | 4 ++-- src/uu/shred/src/shred.rs | 5 +---- src/uu/unexpand/src/unexpand.rs | 6 ++---- src/uu/who/src/who.rs | 5 +---- 7 files changed, 14 insertions(+), 31 deletions(-) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 57caf7970..e898b187c 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -116,7 +116,6 @@ struct Options { show_listed_fs: bool, show_fs_type: bool, show_inode_instead: bool, - print_grand_total: bool, // block_size: usize, human_readable_base: i64, fs_selector: FsSelector, @@ -286,7 +285,6 @@ impl Options { show_listed_fs: false, show_fs_type: false, show_inode_instead: false, - print_grand_total: false, // block_size: match env::var("BLOCKSIZE") { // Ok(size) => size.parse().unwrap(), // Err(_) => 512, @@ -871,9 +869,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { if matches.is_present(OPT_ALL) { opt.show_all_fs = true; } - if matches.is_present(OPT_TOTAL) { - opt.print_grand_total = true; - } if matches.is_present(OPT_INODES) { opt.show_inode_instead = true; } diff --git a/src/uu/hashsum/src/hashsum.rs b/src/uu/hashsum/src/hashsum.rs index ee7d2a0f7..2e31ddd25 100644 --- a/src/uu/hashsum/src/hashsum.rs +++ b/src/uu/hashsum/src/hashsum.rs @@ -78,7 +78,7 @@ fn detect_algo<'a>( "sha512sum" => ("SHA512", Box::new(Sha512::new()) as Box, 512), "b2sum" => ("BLAKE2", Box::new(Blake2b::new(64)) as Box, 512), "sha3sum" => match matches.value_of("bits") { - Some(bits_str) => match usize::from_str_radix(&bits_str, 10) { + Some(bits_str) => match (&bits_str).parse::() { Ok(224) => ( "SHA3-224", Box::new(Sha3_224::new()) as Box, @@ -128,7 +128,7 @@ fn detect_algo<'a>( 512, ), "shake128sum" => match matches.value_of("bits") { - Some(bits_str) => match usize::from_str_radix(&bits_str, 10) { + Some(bits_str) => match (&bits_str).parse::() { Ok(bits) => ( "SHAKE128", Box::new(Shake128::new()) as Box, @@ -139,7 +139,7 @@ fn detect_algo<'a>( None => crash!(1, "--bits required for SHAKE-128"), }, "shake256sum" => match matches.value_of("bits") { - Some(bits_str) => match usize::from_str_radix(&bits_str, 10) { + Some(bits_str) => match (&bits_str).parse::() { Ok(bits) => ( "SHAKE256", Box::new(Shake256::new()) as Box, @@ -182,7 +182,7 @@ fn detect_algo<'a>( } if matches.is_present("sha3") { match matches.value_of("bits") { - Some(bits_str) => match usize::from_str_radix(&bits_str, 10) { + Some(bits_str) => match (&bits_str).parse::() { Ok(224) => set_or_crash( "SHA3-224", Box::new(Sha3_224::new()) as Box, @@ -226,7 +226,7 @@ fn detect_algo<'a>( } if matches.is_present("shake128") { match matches.value_of("bits") { - Some(bits_str) => match usize::from_str_radix(&bits_str, 10) { + Some(bits_str) => match (&bits_str).parse::() { Ok(bits) => set_or_crash("SHAKE128", Box::new(Shake128::new()), bits), Err(err) => crash!(1, "{}", err), }, @@ -235,7 +235,7 @@ fn detect_algo<'a>( } if matches.is_present("shake256") { match matches.value_of("bits") { - Some(bits_str) => match usize::from_str_radix(&bits_str, 10) { + Some(bits_str) => match (&bits_str).parse::() { Ok(bits) => set_or_crash("SHAKE256", Box::new(Shake256::new()), bits), Err(err) => crash!(1, "{}", err), }, @@ -253,7 +253,7 @@ fn detect_algo<'a>( // TODO: return custom error type fn parse_bit_num(arg: &str) -> Result { - usize::from_str_radix(arg, 10) + arg.parse() } fn is_valid_bit_num(arg: String) -> Result<(), String> { diff --git a/src/uu/printf/src/tokenize/num_format/num_format.rs b/src/uu/printf/src/tokenize/num_format/num_format.rs index 9a519e95e..812f51b5a 100644 --- a/src/uu/printf/src/tokenize/num_format/num_format.rs +++ b/src/uu/printf/src/tokenize/num_format/num_format.rs @@ -263,9 +263,5 @@ pub fn num_format(field: &FormatField, in_str_opt: Option<&String>) -> Option Config { } if matches.is_present(options::WIDTH) { let width_str = matches.value_of(options::WIDTH).expect(err_msg).to_string(); - config.line_width = crash_if_err!(1, usize::from_str_radix(&width_str, 10)); + config.line_width = crash_if_err!(1, (&width_str).parse::()); } if matches.is_present(options::GAP_SIZE) { let gap_str = matches .value_of(options::GAP_SIZE) .expect(err_msg) .to_string(); - config.gap_size = crash_if_err!(1, usize::from_str_radix(&gap_str, 10)); + config.gap_size = crash_if_err!(1, (&gap_str).parse::()); } if matches.is_present(options::FORMAT_ROFF) { config.format = OutFormat::Roff; diff --git a/src/uu/shred/src/shred.rs b/src/uu/shred/src/shred.rs index c56f14280..b89d48a10 100644 --- a/src/uu/shred/src/shred.rs +++ b/src/uu/shred/src/shred.rs @@ -363,10 +363,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let force = matches.is_present(options::FORCE); let remove = matches.is_present(options::REMOVE); - let size_arg = match matches.value_of(options::SIZE) { - Some(s) => Some(s.to_string()), - None => None, - }; + let size_arg = matches.value_of(options::SIZE).map(|s| s.to_string()); let size = get_size(size_arg); let exact = matches.is_present(options::EXACT) && size.is_none(); // if -s is given, ignore -x let zero = matches.is_present(options::ZERO); diff --git a/src/uu/unexpand/src/unexpand.rs b/src/uu/unexpand/src/unexpand.rs index 5b08c33cf..3d80bd6e9 100644 --- a/src/uu/unexpand/src/unexpand.rs +++ b/src/uu/unexpand/src/unexpand.rs @@ -149,10 +149,8 @@ fn next_tabstop(tabstops: &[usize], col: usize) -> Option { Some(tabstops[0] - col % tabstops[0]) } else { // find next larger tab - match tabstops.iter().find(|&&t| t > col) { - Some(t) => Some(t - col), - None => None, // if there isn't one in the list, tab becomes a single space - } + // if there isn't one in the list, tab becomes a single space + tabstops.iter().find(|&&t| t > col).map(|t| t-col) } } diff --git a/src/uu/who/src/who.rs b/src/uu/who/src/who.rs index 8c7ff3211..9444985dc 100644 --- a/src/uu/who/src/who.rs +++ b/src/uu/who/src/who.rs @@ -222,7 +222,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { need_runlevel, need_users, my_line_only, - has_records: false, args: matches.free, }; @@ -247,7 +246,6 @@ struct Who { need_runlevel: bool, need_users: bool, my_line_only: bool, - has_records: bool, args: Vec, } @@ -321,8 +319,7 @@ impl Who { println!("{}", users.join(" ")); println!("# users={}", users.len()); } else { - let mut records = Utmpx::iter_all_records().read_from(f).peekable(); - self.has_records = records.peek().is_some(); + let records = Utmpx::iter_all_records().read_from(f).peekable(); if self.include_heading { self.print_heading() From 3aff898acd9722f325c761d9faf6675a077853a7 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 16 Apr 2021 18:58:46 +0200 Subject: [PATCH 10/15] Disable test_no_options_big_input on Windows --- tests/by-util/test_cat.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index 7b4c9924e..f83d51328 100644 --- a/tests/by-util/test_cat.rs +++ b/tests/by-util/test_cat.rs @@ -26,6 +26,7 @@ fn test_no_options() { } #[test] +#[cfg(unix)] fn test_no_options_big_input() { for &n in &[ 0, From 58a2821dce70ed0d60704cbabb2924080f39f5f6 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 16 Apr 2021 19:44:40 +0200 Subject: [PATCH 11/15] Also disable on test_three_directories_and_file_and_stdin --- tests/by-util/test_cat.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index f83d51328..eb6cc9148 100644 --- a/tests/by-util/test_cat.rs +++ b/tests/by-util/test_cat.rs @@ -109,6 +109,7 @@ fn test_directory_and_file() { } #[test] +#[cfg(unix)] fn test_three_directories_and_file_and_stdin() { let s = TestScenario::new(util_name!()); s.fixtures.mkdir("test_directory3"); From a76d452f75e1a503e1e1369143555d9153004edb Mon Sep 17 00:00:00 2001 From: electricboogie <32370782+electricboogie@users.noreply.github.com> Date: Sat, 17 Apr 2021 03:06:19 -0500 Subject: [PATCH 12/15] Sort: More small fixes (#2065) * Various fixes and performance improvements * fix a typo Co-authored-by: Michael Debertol * Fix month parse for months with leading whitespace * Implement test for months whitespace fix * Confirm human numeric works as expected with whitespace with a test * Correct arg help value name for --parallel * Fix SemVer non version lines/empty line sorting with a test Co-authored-by: Sylvestre Ledru Co-authored-by: Michael Debertol --- Cargo.lock | 2 -- src/uu/sort/src/sort.rs | 19 +++++++++++++++---- tests/by-util/test_sort.rs | 19 +++++++++++++++++++ .../sort/human-numeric-whitespace.expected | 11 +++++++++++ .../sort/human-numeric-whitespace.txt | 11 +++++++++++ .../fixtures/sort/months-whitespace.expected | 8 ++++++++ tests/fixtures/sort/months-whitespace.txt | 8 ++++++++ .../sort/version-empty-lines.expected | 11 +++++++++++ tests/fixtures/sort/version-empty-lines.txt | 11 +++++++++++ 9 files changed, 94 insertions(+), 6 deletions(-) create mode 100644 tests/fixtures/sort/human-numeric-whitespace.expected create mode 100644 tests/fixtures/sort/human-numeric-whitespace.txt create mode 100644 tests/fixtures/sort/months-whitespace.expected create mode 100644 tests/fixtures/sort/months-whitespace.txt create mode 100644 tests/fixtures/sort/version-empty-lines.expected create mode 100644 tests/fixtures/sort/version-empty-lines.txt diff --git a/Cargo.lock b/Cargo.lock index 430abf921..461716b1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,7 +1,5 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 - [[package]] name = "advapi32-sys" version = "0.2.0" diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 35ab71ba2..8bf6eb1e8 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -677,7 +677,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .arg( Arg::with_name(OPT_PARALLEL) .long(OPT_PARALLEL) - .help("change the number of threads running concurrently to N") + .help("change the number of threads running concurrently to NUM_THREADS") .takes_value(true) .value_name("NUM_THREADS"), ) @@ -1226,7 +1226,7 @@ fn month_parse(line: &str) -> Month { // GNU splits at any 3 letter match "JUNNNN" is JUN let pattern = if line.trim().len().ge(&3) { // Split a 3 and get first element of tuple ".0" - line.split_at(3).0 + line.trim().split_at(3).0 } else { "" }; @@ -1262,10 +1262,21 @@ fn month_compare(a: &str, b: &str) -> Ordering { } } +fn version_parse(a: &str) -> Version { + let result = Version::parse(a); + + match result { + Ok(vers_a) => vers_a, + // Non-version lines parse to 0.0.0 + Err(_e) => Version::parse("0.0.0").unwrap(), + } +} + fn version_compare(a: &str, b: &str) -> Ordering { #![allow(clippy::comparison_chain)] - let ver_a = Version::parse(a); - let ver_b = Version::parse(b); + let ver_a = version_parse(a); + let ver_b = version_parse(b); + // Version::cmp is not implemented; implement comparison directly if ver_a > ver_b { Ordering::Greater diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 866beefff..0f8020688 100644 --- a/tests/by-util/test_sort.rs +++ b/tests/by-util/test_sort.rs @@ -8,6 +8,25 @@ fn test_helper(file_name: &str, args: &str) { .stdout_is_fixture(format!("{}.expected", file_name)); } +#[test] +fn test_months_whitespace() { + test_helper("months-whitespace", "-M"); +} + +#[test] +fn test_version_empty_lines() { + new_ucmd!() + .arg("-V") + .arg("version-empty-lines.txt") + .succeeds() + .stdout_is("\n\n\n\n\n\n\n1.2.3-alpha\n1.2.3-alpha2\n\t\t\t1.12.4\n11.2.3\n"); +} + +#[test] +fn test_human_numeric_whitespace() { + test_helper("human-numeric-whitespace", "-h"); +} + #[test] fn test_multiple_decimals_general() { new_ucmd!() diff --git a/tests/fixtures/sort/human-numeric-whitespace.expected b/tests/fixtures/sort/human-numeric-whitespace.expected new file mode 100644 index 000000000..6fb9291ff --- /dev/null +++ b/tests/fixtures/sort/human-numeric-whitespace.expected @@ -0,0 +1,11 @@ + + + + + + + +456K +4568K + 456M + 6.2G diff --git a/tests/fixtures/sort/human-numeric-whitespace.txt b/tests/fixtures/sort/human-numeric-whitespace.txt new file mode 100644 index 000000000..19db648b1 --- /dev/null +++ b/tests/fixtures/sort/human-numeric-whitespace.txt @@ -0,0 +1,11 @@ + + +456K + + 456M + + +4568K + + 6.2G + diff --git a/tests/fixtures/sort/months-whitespace.expected b/tests/fixtures/sort/months-whitespace.expected new file mode 100644 index 000000000..84a44d564 --- /dev/null +++ b/tests/fixtures/sort/months-whitespace.expected @@ -0,0 +1,8 @@ + + +JAN + FEb + apr + apr + JUNNNN +AUG diff --git a/tests/fixtures/sort/months-whitespace.txt b/tests/fixtures/sort/months-whitespace.txt new file mode 100644 index 000000000..45c477477 --- /dev/null +++ b/tests/fixtures/sort/months-whitespace.txt @@ -0,0 +1,8 @@ +JAN + JUNNNN +AUG + + apr + apr + + FEb diff --git a/tests/fixtures/sort/version-empty-lines.expected b/tests/fixtures/sort/version-empty-lines.expected new file mode 100644 index 000000000..c496c0ff5 --- /dev/null +++ b/tests/fixtures/sort/version-empty-lines.expected @@ -0,0 +1,11 @@ + + + + + + + +1.2.3-alpha +1.2.3-alpha2 +11.2.3 + 1.12.4 diff --git a/tests/fixtures/sort/version-empty-lines.txt b/tests/fixtures/sort/version-empty-lines.txt new file mode 100644 index 000000000..9b6b89788 --- /dev/null +++ b/tests/fixtures/sort/version-empty-lines.txt @@ -0,0 +1,11 @@ +11.2.3 + + + +1.2.3-alpha2 + + +1.2.3-alpha + + + 1.12.4 From f33320e58133c1e4182ddb1eac72ee0743e77ba0 Mon Sep 17 00:00:00 2001 From: joppich Date: Sat, 17 Apr 2021 10:07:45 +0200 Subject: [PATCH 13/15] do not pipe data into failure tests (#2072) Co-authored-by: joppich --- tests/by-util/test_stdbuf.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/tests/by-util/test_stdbuf.rs b/tests/by-util/test_stdbuf.rs index 61fa36977..808b7382a 100644 --- a/tests/by-util/test_stdbuf.rs +++ b/tests/by-util/test_stdbuf.rs @@ -25,19 +25,15 @@ fn test_stdbuf_line_buffered_stdout() { #[cfg(not(target_os = "windows"))] #[test] fn test_stdbuf_no_buffer_option_fails() { - new_ucmd!() - .args(&["head"]) - .pipe_in("The quick brown fox jumps over the lazy dog.") - .fails() - .stderr_is( - "error: The following required arguments were not provided:\n \ + new_ucmd!().args(&["head"]).fails().stderr_is( + "error: The following required arguments were not provided:\n \ --error \n \ --input \n \ --output \n\n\ USAGE:\n \ stdbuf OPTION... COMMAND\n\n\ For more information try --help", - ); + ); } #[cfg(not(target_os = "windows"))] @@ -55,7 +51,6 @@ fn test_stdbuf_trailing_var_arg() { fn test_stdbuf_line_buffering_stdin_fails() { new_ucmd!() .args(&["-i", "L", "head"]) - .pipe_in("The quick brown fox jumps over the lazy dog.") .fails() .stderr_is("stdbuf: error: line buffering stdin is meaningless\nTry 'stdbuf --help' for more information."); } @@ -65,7 +60,6 @@ fn test_stdbuf_line_buffering_stdin_fails() { fn test_stdbuf_invalid_mode_fails() { new_ucmd!() .args(&["-i", "1024R", "head"]) - .pipe_in("The quick brown fox jumps over the lazy dog.") .fails() .stderr_is("stdbuf: error: invalid mode 1024R\nTry 'stdbuf --help' for more information."); } From fe207640e24b25ec3cade4384da8b156265418b2 Mon Sep 17 00:00:00 2001 From: Aleksandar Janicijevic Date: Sat, 17 Apr 2021 04:08:10 -0400 Subject: [PATCH 14/15] touch: dealing with DST in touch -m -t (#2073) --- src/uu/touch/src/touch.rs | 23 ++++++++++++++++++++++- tests/by-util/test_touch.rs | 19 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/uu/touch/src/touch.rs b/src/uu/touch/src/touch.rs index f0c3c12d2..b158fdc0e 100644 --- a/src/uu/touch/src/touch.rs +++ b/src/uu/touch/src/touch.rs @@ -18,6 +18,7 @@ use filetime::*; use std::fs::{self, File}; use std::io::Error; use std::path::Path; +use std::process; static VERSION: &str = env!("CARGO_PKG_VERSION"); static ABOUT: &str = "Update the access and modification times of each FILE to the current time."; @@ -261,7 +262,27 @@ fn parse_timestamp(s: &str) -> FileTime { }; match time::strptime(&ts, format) { - Ok(tm) => local_tm_to_filetime(to_local(tm)), + Ok(tm) => { + let mut local = to_local(tm); + local.tm_isdst = -1; + let ft = local_tm_to_filetime(local); + + // We have to check that ft is valid time. Due to daylight saving + // time switch, local time can jump from 1:59 AM to 3:00 AM, + // in which case any time between 2:00 AM and 2:59 AM is not valid. + // Convert back to local time and see if we got the same value back. + let ts = time::Timespec { + sec: ft.unix_seconds(), + nsec: 0, + }; + let tm2 = time::at(ts); + if tm.tm_hour != tm2.tm_hour { + show_error!("invalid date format {}", s); + process::exit(1); + } + + ft + } Err(e) => panic!("Unable to parse timestamp\n{}", e), } } diff --git a/tests/by-util/test_touch.rs b/tests/by-util/test_touch.rs index 9921c16b5..9f2c079b0 100644 --- a/tests/by-util/test_touch.rs +++ b/tests/by-util/test_touch.rs @@ -29,6 +29,7 @@ fn set_file_times(at: &AtPath, path: &str, atime: FileTime, mtime: FileTime) { fn str_to_filetime(format: &str, s: &str) -> FileTime { let mut tm = time::strptime(s, format).unwrap(); tm.tm_utcoff = time::now().tm_utcoff; + tm.tm_isdst = -1; // Unknown flag DST let ts = tm.to_timespec(); FileTime::from_unix_time(ts.sec as i64, ts.nsec as u32) } @@ -352,3 +353,21 @@ fn test_touch_set_date() { assert_eq!(atime, start_of_year); assert_eq!(mtime, start_of_year); } + +#[test] +fn test_touch_mtime_dst_succeeds() { + let (at, mut ucmd) = at_and_ucmd!(); + let file = "test_touch_set_mtime_dst_succeeds"; + + ucmd.args(&["-m", "-t", "202103140300", file]) + .succeeds() + .no_stderr(); + + assert!(at.file_exists(file)); + + let target_time = str_to_filetime("%Y%m%d%H%M", "202103140300"); + let (_, mtime) = get_file_times(&at, file); + eprintln!("target_time: {:?}", target_time); + eprintln!("mtime: {:?}", mtime); + assert!(target_time == mtime); +} From d0c7e8c09e6101a10975640762006d6228ca0ed0 Mon Sep 17 00:00:00 2001 From: Andrew Rowson Date: Sat, 17 Apr 2021 09:26:52 +0100 Subject: [PATCH 15/15] du error output should match GNU (#1776) * du error output should match GNU * Created a new error macro which allows the customization of the "error:" string part * Match the du output based on the type of error encountered. Can extend to handling other errors I guess. * Rustfmt updates * Added non-windows test for du no permission output --- src/uu/du/src/du.rs | 18 ++++++++++++++++-- src/uucore/src/lib/macros.rs | 8 ++++++++ tests/by-util/test_du.rs | 30 ++++++++++++++++++++++++++++++ 3 files changed, 54 insertions(+), 2 deletions(-) diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 07635881a..e01af5195 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -15,7 +15,7 @@ use chrono::Local; use std::collections::HashSet; use std::env; use std::fs; -use std::io::{stderr, Result, Write}; +use std::io::{stderr, ErrorKind, Result, Write}; use std::iter; #[cfg(not(windows))] use std::os::unix::fs::MetadataExt; @@ -296,7 +296,21 @@ fn du( } } } - Err(error) => show_error!("{}", error), + Err(error) => match error.kind() { + ErrorKind::PermissionDenied => { + let description = format!( + "cannot access '{}'", + entry + .path() + .as_os_str() + .to_str() + .unwrap_or("") + ); + let error_message = "Permission denied"; + show_error_custom_description!(description, "{}", error_message) + } + _ => show_error!("{}", error), + }, }, Err(error) => show_error!("{}", error), } diff --git a/src/uucore/src/lib/macros.rs b/src/uucore/src/lib/macros.rs index 24b392ebd..637e91f8f 100644 --- a/src/uucore/src/lib/macros.rs +++ b/src/uucore/src/lib/macros.rs @@ -31,6 +31,14 @@ macro_rules! show_error( ); /// Show a warning to stderr in a silimar style to GNU coreutils. +#[macro_export] +macro_rules! show_error_custom_description ( + ($err:expr,$($args:tt)+) => ({ + eprint!("{}: {}: ", executable!(), $err); + eprintln!($($args)+); + }) +); + #[macro_export] macro_rules! show_warning( ($($args:tt)+) => ({ diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 8f2cff65d..f668edeef 100644 --- a/tests/by-util/test_du.rs +++ b/tests/by-util/test_du.rs @@ -190,3 +190,33 @@ fn test_du_time() { assert_eq!(result.stderr, ""); assert_eq!(result.stdout, "0\t2015-05-15 00:00\tdate_test\n"); } + +#[cfg(not(target_os = "windows"))] +#[cfg(feature = "chmod")] +#[test] +fn test_du_no_permission() { + let ts = TestScenario::new("du"); + + let chmod = ts.ccmd("chmod").arg("-r").arg(SUB_DIR_LINKS).run(); + println!("chmod output: {:?}", chmod); + assert!(chmod.success); + let result = ts.ucmd().arg(SUB_DIR_LINKS).run(); + + ts.ccmd("chmod").arg("+r").arg(SUB_DIR_LINKS).run(); + + assert!(result.success); + assert_eq!( + result.stderr, + "du: cannot read directory ‘subdir/links‘: Permission denied (os error 13)\n" + ); + _du_no_permission(result.stdout); +} + +#[cfg(target_vendor = "apple")] +fn _du_no_permission(s: String) { + assert_eq!(s, "0\tsubdir/links\n"); +} +#[cfg(all(not(target_vendor = "apple"), not(target_os = "windows")))] +fn _du_no_permission(s: String) { + assert_eq!(s, "4\tsubdir/links\n"); +}