Merge branch 'master' into split-wsl-detection

.github/workflows/GNU.yml

@@ -80,6 +80,9 @@ jobs:
                -e '/tests\/misc\/help-version-getopt.sh/ D' \
                Makefile
 
+        # printf doesn't limit the values used in its arg, so this produced ~2GB of output
+        sed -i '/INT_OFLOW/ D' tests/misc/printf.sh
+
         # Use the system coreutils where the test fails due to error in a util that is not the one being tested
         sed -i 's|stat|/usr/bin/stat|' tests/chgrp/basic.sh tests/cp/existing-perm-dir.sh tests/touch/60-seconds.sh tests/misc/sort-compress-proc.sh
         sed -i 's|ls -|/usr/bin/ls -|' tests/chgrp/posix-H.sh tests/chown/deref.sh tests/cp/same-file.sh tests/misc/mknod.sh tests/mv/part-symlink.sh tests/du/8gb.sh

Cargo.lock

@@ -28,6 +28,15 @@ dependencies = [
  "winapi 0.3.9",
 ]
 
+[[package]]
+name = "ansi_term"
+version = "0.12.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d52a9bb7ec0cf484c551830a7ce27bd20d67eac647e1befb56b0be4ee39a55d2"
+dependencies = [
+ "winapi 0.3.9",
+]
+
 [[package]]
 name = "arrayvec"
 version = "0.4.12"
@@ -127,9 +136,9 @@ checksum = "14c189c53d098945499cdfa7ecc63567cf3886b3332b312a5b4585d8d3a6a610"
 
 [[package]]
 name = "cast"
-version = "0.2.3"
+version = "0.2.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "4b9434b9a5aa1450faa3f9cb14ea0e8c53bb5d2b3c1bfd1ab4fc03e9f33fbfb0"
+checksum = "cc38c385bfd7e444464011bb24820f40dd1c76bcdfa1b78611cb7c2e5cafab75"
 dependencies = [
  "rustc_version",
 ]
@@ -169,7 +178,7 @@ version = "2.33.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "37e58ac78573c40708d45522f0d80fa2f01cc4f9b4e2bf749807255454312002"
 dependencies = [
- "ansi_term",
+ "ansi_term 0.11.0",
  "atty",
  "bitflags",
  "strsim",
@@ -452,9 +461,9 @@ dependencies = [
 
 [[package]]
 name = "crossbeam-channel"
-version = "0.5.0"
+version = "0.5.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "dca26ee1f8d361640700bde38b2c37d8c22b3ce2d360e1fc1c74ea4b0aa7d775"
+checksum = "06ed27e177f16d65f0f0c22a213e17c696ace5dd64b14258b52f9417ccb52db4"
 dependencies = [
  "cfg-if 1.0.0",
  "crossbeam-utils",
@@ -580,7 +589,7 @@ checksum = "1d34cfa13a63ae058bfa601fe9e313bbdb3746427c1459185464ce0fcf62e1e8"
 dependencies = [
  "cfg-if 1.0.0",
  "libc",
- "redox_syscall 0.2.5",
+ "redox_syscall 0.2.6",
  "winapi 0.3.9",
 ]
 
@@ -783,6 +792,15 @@ dependencies = [
  "cfg-if 1.0.0",
 ]
 
+[[package]]
+name = "lscolors"
+version = "0.7.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d24b894c45c9da468621cdd615a5a79ee5e5523dd4f75c76ebc03d458940c16e"
+dependencies = [
+ "ansi_term 0.12.1",
+]
+
 [[package]]
 name = "match_cfg"
 version = "0.1.0"
@@ -1176,9 +1194,9 @@ checksum = "41cc0f7e4d5d4544e8861606a285bb08d3e70712ccc7d2b84d7c0ccfaf4b05ce"
 
 [[package]]
 name = "redox_syscall"
-version = "0.2.5"
+version = "0.2.6"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "94341e4e44e24f6b591b59e47a8a027df12e008d73fd5672dbea9cc22f4507d9"
+checksum = "8270314b5ccceb518e7e578952f0b72b88222d02e8f77f5ecf7abbb673539041"
 dependencies = [
  "bitflags",
 ]
@@ -1189,7 +1207,7 @@ version = "0.1.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "8440d8acb4fd3d277125b4bd01a6f38aee8d814b3b5fc09b3f2b825d37d3fe8f"
 dependencies = [
- "redox_syscall 0.2.5",
+ "redox_syscall 0.2.6",
 ]
 
 [[package]]
@@ -1394,9 +1412,9 @@ dependencies = [
 
 [[package]]
 name = "syn"
-version = "1.0.69"
+version = "1.0.70"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "48fe99c6bd8b1cc636890bcc071842de909d902c81ac7dab53ba33c421ab8ffb"
+checksum = "b9505f307c872bab8eb46f77ae357c8eba1fdacead58ee5a850116b1d7f82883"
 dependencies = [
  "proc-macro2",
  "quote 1.0.9",
@@ -1454,7 +1472,7 @@ checksum = "077185e2eac69c3f8379a4298e1e07cd36beb962290d4a51199acf0fdc10607e"
 dependencies = [
  "libc",
  "numtoa",
- "redox_syscall 0.2.5",
+ "redox_syscall 0.2.6",
  "redox_termios",
 ]
 
@@ -1990,11 +2008,11 @@ dependencies = [
  "clap",
  "globset",
  "lazy_static",
+ "lscolors",
  "number_prefix",
  "term_grid",
  "termsize",
  "time",
- "unicode-width",
  "uucore",
  "uucore_procs",
 ]

README.md

@@ -93,7 +93,7 @@ $ cargo build --features "base32 cat echo rm" --no-default-features
 
 If you don't want to build the multicall binary and would prefer to build
 the utilities as individual binaries, that is also possible. Each utility
-is contained in it's own package within the main repository, named
+is contained in its own package within the main repository, named
 "uu_UTILNAME". To build individual utilities, use cargo to build just the
 specific packages (using the `--package` [aka `-p`] option). For example:
 

src/uu/cat/src/cat.rs

@@ -22,6 +22,12 @@ use std::io::{self, Read, Write};
 use thiserror::Error;
 use uucore::fs::is_stdin_interactive;
 
+/// Linux splice support
+#[cfg(any(target_os = "linux", target_os = "android"))]
+mod splice;
+#[cfg(any(target_os = "linux", target_os = "android"))]
+use std::os::unix::io::{AsRawFd, RawFd};
+
 /// Unix domain socket support
 #[cfg(unix)]
 use std::net::Shutdown;
@@ -30,14 +36,6 @@ use std::os::unix::fs::FileTypeExt;
 #[cfg(unix)]
 use unix_socket::UnixStream;
 
-/// Linux splice support
-#[cfg(any(target_os = "linux", target_os = "android"))]
-use nix::fcntl::{splice, SpliceFFlags};
-#[cfg(any(target_os = "linux", target_os = "android"))]
-use nix::unistd::pipe;
-#[cfg(any(target_os = "linux", target_os = "android"))]
-use std::os::unix::io::{AsRawFd, RawFd};
-
 static NAME: &str = "cat";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
 static SYNTAX: &str = "[OPTION]... [FILE]...";
@@ -395,7 +393,7 @@ fn write_fast<R: Read>(handle: &mut InputHandle<R>) -> CatResult<()> {
     {
         // If we're on Linux or Android, try to use the splice() system call
         // for faster writing. If it works, we're done.
-        if !write_fast_using_splice(handle, stdout_lock.as_raw_fd())? {
+        if !splice::write_fast_using_splice(handle, stdout_lock.as_raw_fd())? {
             return Ok(());
         }
     }
@@ -411,75 +409,6 @@ fn write_fast<R: Read>(handle: &mut InputHandle<R>) -> CatResult<()> {
     Ok(())
 }
 
-/// This function is called from `write_fast()` on Linux and Android. The
-/// function `splice()` is used to move data between two file descriptors
-/// without copying between kernel- and userspace. This results in a large
-/// speedup.
-///
-/// The `bool` in the result value indicates if we need to fall back to normal
-/// copying or not. False means we don't have to.
-#[cfg(any(target_os = "linux", target_os = "android"))]
-#[inline]
-fn write_fast_using_splice<R: Read>(handle: &mut InputHandle<R>, writer: RawFd) -> CatResult<bool> {
-    const BUF_SIZE: usize = 1024 * 16;
-
-    let (pipe_rd, pipe_wr) = pipe()?;
-
-    // We only fall back if splice fails on the first call.
-    match splice(
-        handle.file_descriptor,
-        None,
-        pipe_wr,
-        None,
-        BUF_SIZE,
-        SpliceFFlags::empty(),
-    ) {
-        Ok(n) => {
-            if n == 0 {
-                return Ok(false);
-            }
-            splice_exact(pipe_rd, writer, n)?;
-        }
-        Err(_) => {
-            return Ok(true);
-        }
-    }
-
-    loop {
-        let n = splice(
-            handle.file_descriptor,
-            None,
-            pipe_wr,
-            None,
-            BUF_SIZE,
-            SpliceFFlags::empty(),
-        )?;
-        if n == 0 {
-            // We read 0 bytes from the input,
-            // which means we're done copying.
-            break;
-        }
-        splice_exact(pipe_rd, writer, n)?;
-    }
-
-    Ok(false)
-}
-
-/// Splice wrapper which handles short writes
-#[cfg(any(target_os = "linux", target_os = "android"))]
-#[inline]
-fn splice_exact(read_fd: RawFd, write_fd: RawFd, num_bytes: usize) -> nix::Result<()> {
-    let mut left = num_bytes;
-    loop {
-        let written = splice(read_fd, None, write_fd, None, left, SpliceFFlags::empty())?;
-        left -= written;
-        if left == 0 {
-            break;
-        }
-    }
-    Ok(())
-}
-
 /// Outputs file contents to stdout in a line-by-line fashion,
 /// propagating any errors that might occur.
 fn write_lines<R: Read>(

src/uu/cat/src/splice.rs

@@ -0,0 +1,91 @@
+use super::{CatResult, InputHandle};
+
+use nix::fcntl::{splice, SpliceFFlags};
+use nix::unistd::{self, pipe};
+use std::io::Read;
+use std::os::unix::io::RawFd;
+
+const BUF_SIZE: usize = 1024 * 16;
+
+/// This function is called from `write_fast()` on Linux and Android. The
+/// function `splice()` is used to move data between two file descriptors
+/// without copying between kernel- and userspace. This results in a large
+/// speedup.
+///
+/// The `bool` in the result value indicates if we need to fall back to normal
+/// copying or not. False means we don't have to.
+#[inline]
+pub(super) fn write_fast_using_splice<R: Read>(
+    handle: &mut InputHandle<R>,
+    write_fd: RawFd,
+) -> CatResult<bool> {
+    let (pipe_rd, pipe_wr) = match pipe() {
+        Ok(r) => r,
+        Err(_) => {
+            // It is very rare that creating a pipe fails, but it can happen.
+            return Ok(true);
+        }
+    };
+
+    loop {
+        match splice(
+            handle.file_descriptor,
+            None,
+            pipe_wr,
+            None,
+            BUF_SIZE,
+            SpliceFFlags::empty(),
+        ) {
+            Ok(n) => {
+                if n == 0 {
+                    return Ok(false);
+                }
+                if splice_exact(pipe_rd, write_fd, n).is_err() {
+                    // If the first splice manages to copy to the intermediate
+                    // pipe, but the second splice to stdout fails for some reason
+                    // we can recover by copying the data that we have from the
+                    // intermediate pipe to stdout using normal read/write. Then
+                    // we tell the caller to fall back.
+                    copy_exact(pipe_rd, write_fd, n)?;
+                    return Ok(true);
+                }
+            }
+            Err(_) => {
+                return Ok(true);
+            }
+        }
+    }
+}
+
+/// Splice wrapper which handles short writes.
+#[inline]
+fn splice_exact(read_fd: RawFd, write_fd: RawFd, num_bytes: usize) -> nix::Result<()> {
+    let mut left = num_bytes;
+    loop {
+        let written = splice(read_fd, None, write_fd, None, left, SpliceFFlags::empty())?;
+        left -= written;
+        if left == 0 {
+            break;
+        }
+    }
+    Ok(())
+}
+
+/// Caller must ensure that `num_bytes <= BUF_SIZE`, otherwise this function
+/// will panic. The way we use this function in `write_fast_using_splice`
+/// above is safe because `splice` is set to write at most `BUF_SIZE` to the
+/// pipe.
+#[inline]
+fn copy_exact(read_fd: RawFd, write_fd: RawFd, num_bytes: usize) -> nix::Result<()> {
+    let mut left = num_bytes;
+    let mut buf = [0; BUF_SIZE];
+    loop {
+        let read = unistd::read(read_fd, &mut buf[..left])?;
+        let written = unistd::write(write_fd, &mut buf[..read])?;
+        left -= written;
+        if left == 0 {
+            break;
+        }
+    }
+    Ok(())
+}

src/uu/chown/src/chown.rs

@@ -272,16 +272,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 }
 
 fn parse_spec(spec: &str) -> Result<(Option<u32>, Option<u32>), String> {
-    let args = spec.split(':').collect::<Vec<_>>();
+    let args = spec.split_terminator(':').collect::<Vec<_>>();
-    let usr_only = args.len() == 1;
+    let usr_only = args.len() == 1 && !args[0].is_empty();
-    let grp_only = args.len() == 2 && args[0].is_empty() && !args[1].is_empty();
+    let grp_only = args.len() == 2 && args[0].is_empty();
     let usr_grp = args.len() == 2 && !args[0].is_empty() && !args[1].is_empty();
 
     if usr_only {
         Ok((
             Some(match Passwd::locate(args[0]) {
                 Ok(v) => v.uid(),
-                _ => return Err(format!("invalid user: '{}'", spec)),
+                _ => return Err(format!("invalid user: ‘{}’", spec)),
             }),
             None,
         ))
@@ -290,18 +290,18 @@ fn parse_spec(spec: &str) -> Result<(Option<u32>, Option<u32>), String> {
             None,
             Some(match Group::locate(args[1]) {
                 Ok(v) => v.gid(),
-                _ => return Err(format!("invalid group: '{}'", spec)),
+                _ => return Err(format!("invalid group: ‘{}’", spec)),
             }),
         ))
     } else if usr_grp {
         Ok((
             Some(match Passwd::locate(args[0]) {
                 Ok(v) => v.uid(),
-                _ => return Err(format!("invalid user: '{}'", spec)),
+                _ => return Err(format!("invalid user: ‘{}’", spec)),
             }),
             Some(match Group::locate(args[1]) {
                 Ok(v) => v.gid(),
-                _ => return Err(format!("invalid group: '{}'", spec)),
+                _ => return Err(format!("invalid group: ‘{}’", spec)),
             }),
         ))
     } else {

src/uu/cp/src/cp.rs

@@ -210,7 +210,6 @@ pub struct Options {
     overwrite: OverwriteMode,
     parents: bool,
     strip_trailing_slashes: bool,
-    reflink: bool,
     reflink_mode: ReflinkMode,
     preserve_attributes: Vec<Attribute>,
     recursive: bool,
@@ -633,12 +632,12 @@ impl Options {
             update: matches.is_present(OPT_UPDATE),
             verbose: matches.is_present(OPT_VERBOSE),
             strip_trailing_slashes: matches.is_present(OPT_STRIP_TRAILING_SLASHES),
-            reflink: matches.is_present(OPT_REFLINK),
             reflink_mode: {
                 if let Some(reflink) = matches.value_of(OPT_REFLINK) {
                     match reflink {
                         "always" => ReflinkMode::Always,
                         "auto" => ReflinkMode::Auto,
+                        "never" => ReflinkMode::Never,
                         value => {
                             return Err(Error::InvalidArgument(format!(
                                 "invalid argument '{}' for \'reflink\'",
@@ -1196,7 +1195,7 @@ fn copy_file(source: &Path, dest: &Path, options: &Options) -> CopyResult<()> {
 ///Copy the file from `source` to `dest` either using the normal `fs::copy` or the
 ///`FICLONE` ioctl if --reflink is specified and the filesystem supports it.
 fn copy_helper(source: &Path, dest: &Path, options: &Options) -> CopyResult<()> {
-    if options.reflink {
+    if options.reflink_mode != ReflinkMode::Never {
         #[cfg(not(target_os = "linux"))]
         return Err("--reflink is only supported on linux".to_string().into());
 

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("<Un-printable path>")
+                            );
+                            let error_message = "Permission denied";
+                            show_error_custom_description!(description, "{}", error_message)
+                        }
+                        _ => show_error!("{}", error),
+                    },
                 },
                 Err(error) => show_error!("{}", error),
             }
@@ -486,7 +500,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     };
 
     let strs = if matches.free.is_empty() {
-        vec!["./".to_owned()]
+        vec!["./".to_owned()] // TODO: gnu `du` doesn't use trailing "/" here
     } else {
         matches.free.clone()
     };

src/uu/install/src/install.rs

@@ -41,6 +41,7 @@ pub struct Behavior {
     compare: bool,
     strip: bool,
     strip_program: String,
+    create_leading: bool,
 }
 
 #[derive(Clone, Eq, PartialEq)]
@@ -70,7 +71,7 @@ static OPT_BACKUP: &str = "backup";
 static OPT_BACKUP_2: &str = "backup2";
 static OPT_DIRECTORY: &str = "directory";
 static OPT_IGNORED: &str = "ignored";
-static OPT_CREATED: &str = "created";
+static OPT_CREATE_LEADING: &str = "create-leading";
 static OPT_GROUP: &str = "group";
 static OPT_MODE: &str = "mode";
 static OPT_OWNER: &str = "owner";
@@ -133,9 +134,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
         .arg(
             // TODO implement flag
-            Arg::with_name(OPT_CREATED)
+            Arg::with_name(OPT_CREATE_LEADING)
                 .short("D")
-                .help("(unimplemented) create all leading components of DEST except the last, then copy SOURCE to DEST")
+                .help("create all leading components of DEST except the last, then copy SOURCE to DEST")
         )
         .arg(
             Arg::with_name(OPT_GROUP)
@@ -266,8 +267,6 @@ fn check_unimplemented<'a>(matches: &ArgMatches) -> Result<(), &'a str> {
         Err("--backup")
     } else if matches.is_present(OPT_BACKUP_2) {
         Err("-b")
-    } else if matches.is_present(OPT_CREATED) {
-        Err("-D")
     } else if matches.is_present(OPT_SUFFIX) {
         Err("--suffix, -S")
     } else if matches.is_present(OPT_TARGET_DIRECTORY) {
@@ -343,6 +342,7 @@ fn behavior(matches: &ArgMatches) -> Result<Behavior, i32> {
                 .value_of(OPT_STRIP_PROGRAM)
                 .unwrap_or(DEFAULT_STRIP_PROGRAM),
         ),
+        create_leading: matches.is_present(OPT_CREATE_LEADING),
     })
 }
 
@@ -410,12 +410,35 @@ fn standard(paths: Vec<String>, b: Behavior) -> i32 {
         .iter()
         .map(PathBuf::from)
         .collect::<Vec<_>>();
+
     let target = Path::new(paths.last().unwrap());
 
-    if (target.is_file() || is_new_file_path(target)) && sources.len() == 1 {
+    if sources.len() > 1 || (target.exists() && target.is_dir()) {
-        copy_file_to_file(&sources[0], &target.to_path_buf(), &b)
-    } else {
         copy_files_into_dir(sources, &target.to_path_buf(), &b)
+    } else {
+        if let Some(parent) = target.parent() {
+            if !parent.exists() && b.create_leading {
+                if let Err(e) = fs::create_dir_all(parent) {
+                    show_error!("failed to create {}: {}", parent.display(), e);
+                    return 1;
+                }
+
+                if mode::chmod(&parent, b.mode()).is_err() {
+                    show_error!("failed to chmod {}", parent.display());
+                    return 1;
+                }
+            }
+        }
+
+        if target.is_file() || is_new_file_path(target) {
+            copy_file_to_file(&sources[0], &target.to_path_buf(), &b)
+        } else {
+            show_error!(
+                "invalid target {}: No such file or directory",
+                target.display()
+            );
+            1
+        }
     }
 }
 

src/uu/ls/BENCHMARKING.md

@@ -0,0 +1,34 @@
+# Benchmarking ls
+
+ls majorly involves fetching a lot of details (depending upon what details are requested, eg. time/date, inode details, etc) for each path using system calls. Ideally, any system call should be done only once for each of the paths - not adhering to this principle leads to a lot of system call overhead multiplying and bubbling up, especially for recursive ls, therefore it is important to always benchmark multiple scenarios.
+This is an overwiew over what was benchmarked, and if you make changes to `ls`, you are encouraged to check
+how performance was affected for the workloads listed below. Feel free to add other workloads to the
+list that we should improve / make sure not to regress.
+
+Run `cargo build --release` before benchmarking after you make a change!
+
+## Simple recursive ls
+
+-   Get a large tree, for example linux kernel source tree.
+-   Benchmark simple recursive ls with hyperfine: `hyperfine --warmup 2 "target/release/coreutils ls -R tree > /dev/null"`.
+
+## Recursive ls with all and long options
+
+-   Same tree as above
+-   Benchmark recursive ls with -al -R options with hyperfine: `hyperfine --warmup 2 "target/release/coreutils ls -al -R tree > /dev/null"`.
+
+## Comparing with GNU ls
+
+Hyperfine accepts multiple commands to run and will compare them. To compare performance with GNU ls
+duplicate the string you passed to hyperfine but remove the `target/release/coreutils` bit from it.
+
+Example: `hyperfine --warmup 2 "target/release/coreutils ls -al -R tree > /dev/null"` becomes
+`hyperfine --warmup 2 "target/release/coreutils ls -al -R tree > /dev/null" "ls -al -R tree > /dev/null"`
+(This assumes GNU ls is installed as `ls`)
+
+This can also be used to compare with version of ls built before your changes to ensure your change does not regress this
+
+## Checking system call count
+
+- Another thing to look at would be system calls count using strace (on linux) or equivalent on other operating systems.
+- Example: `strace -c target/release/coreutils ls -al -R tree`

src/uu/ls/Cargo.toml

@@ -16,18 +16,18 @@ path = "src/ls.rs"
 
 [dependencies]
 clap = "2.33"
-lazy_static = "1.0.1"
 number_prefix = "0.4"
 term_grid = "0.1.5"
 termsize = "0.1.6"
 time = "0.1.40"
-unicode-width = "0.1.5"
 globset = "0.4.6"
+lscolors = { version="0.7.1", features=["ansi_term"] }
 uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["entries", "fs"] }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
+atty = "0.2"
 
 [target.'cfg(unix)'.dependencies]
-atty = "0.2"
+lazy_static = "1.4.0"
 
 [[bin]]
 name = "ls"

src/uu/ls/src/ls.rs

@@ -7,27 +7,25 @@
 
 // spell-checker:ignore (ToDO) cpio svgz webm somegroup nlink rmvb xspf
 
+#[macro_use]
+extern crate uucore;
 #[cfg(unix)]
 #[macro_use]
 extern crate lazy_static;
-#[macro_use]
-extern crate uucore;
 
 mod quoting_style;
 mod version_cmp;
 
 use clap::{App, Arg};
 use globset::{self, Glob, GlobSet, GlobSetBuilder};
+use lscolors::LsColors;
 use number_prefix::NumberPrefix;
 use quoting_style::{escape_name, QuotingStyle};
 #[cfg(unix)]
 use std::collections::HashMap;
-use std::fs;
+use std::fs::{self, DirEntry, FileType, Metadata};
-use std::fs::{DirEntry, FileType, Metadata};
-#[cfg(unix)]
-use std::os::unix::fs::FileTypeExt;
 #[cfg(any(unix, target_os = "redox"))]
-use std::os::unix::fs::MetadataExt;
+use std::os::unix::fs::{FileTypeExt, MetadataExt};
 #[cfg(windows)]
 use std::os::windows::fs::MetadataExt;
 use std::path::{Path, PathBuf};
@@ -39,9 +37,7 @@ use std::{cmp::Reverse, process::exit};
 use term_grid::{Cell, Direction, Filling, Grid, GridOptions};
 use time::{strftime, Timespec};
 #[cfg(unix)]
-use unicode_width::UnicodeWidthStr;
+use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR};
-#[cfg(unix)]
-use uucore::libc::{mode_t, S_ISGID, S_ISUID, S_ISVTX, S_IWOTH, S_IXGRP, S_IXOTH, S_IXUSR};
 
 static VERSION: &str = env!("CARGO_PKG_VERSION");
 static ABOUT: &str = "
@@ -54,30 +50,6 @@ fn get_usage() -> String {
     format!("{0} [OPTION]... [FILE]...", executable!())
 }
 
-#[cfg(unix)]
-static DEFAULT_COLORS: &str = "rs=0:di=01;34:ln=01;36:mh=00:pi=40;33:so=01;35:do=01;35:bd=40;33;01:cd=40;33;01:or=40;31;01:mi=00:su=37;41:sg=30;43:ca=30;41:tw=30;42:ow=34;42:st=37;44:ex=01;32:*.tar=01;31:*.tgz=01;31:*.arc=01;31:*.arj=01;31:*.taz=01;31:*.lha=01;31:*.lz4=01;31:*.lzh=01;31:*.lzma=01;31:*.tlz=01;31:*.txz=01;31:*.tzo=01;31:*.t7z=01;31:*.zip=01;31:*.z=01;31:*.Z=01;31:*.dz=01;31:*.gz=01;31:*.lrz=01;31:*.lz=01;31:*.lzo=01;31:*.xz=01;31:*.bz2=01;31:*.bz=01;31:*.tbz=01;31:*.tbz2=01;31:*.tz=01;31:*.deb=01;31:*.rpm=01;31:*.jar=01;31:*.war=01;31:*.ear=01;31:*.sar=01;31:*.rar=01;31:*.alz=01;31:*.ace=01;31:*.zoo=01;31:*.cpio=01;31:*.7z=01;31:*.rz=01;31:*.cab=01;31:*.jpg=01;35:*.jpeg=01;35:*.gif=01;35:*.bmp=01;35:*.pbm=01;35:*.pgm=01;35:*.ppm=01;35:*.tga=01;35:*.xbm=01;35:*.xpm=01;35:*.tif=01;35:*.tiff=01;35:*.png=01;35:*.svg=01;35:*.svgz=01;35:*.mng=01;35:*.pcx=01;35:*.mov=01;35:*.mpg=01;35:*.mpeg=01;35:*.m2v=01;35:*.mkv=01;35:*.webm=01;35:*.ogm=01;35:*.mp4=01;35:*.m4v=01;35:*.mp4v=01;35:*.vob=01;35:*.qt=01;35:*.nuv=01;35:*.wmv=01;35:*.asf=01;35:*.rm=01;35:*.rmvb=01;35:*.flc=01;35:*.avi=01;35:*.fli=01;35:*.flv=01;35:*.gl=01;35:*.dl=01;35:*.xcf=01;35:*.xwd=01;35:*.yuv=01;35:*.cgm=01;35:*.emf=01;35:*.ogv=01;35:*.ogx=01;35:*.aac=00;36:*.au=00;36:*.flac=00;36:*.m4a=00;36:*.mid=00;36:*.midi=00;36:*.mka=00;36:*.mp3=00;36:*.mpc=00;36:*.ogg=00;36:*.ra=00;36:*.wav=00;36:*.oga=00;36:*.opus=00;36:*.spx=00;36:*.xspf=00;36:";
-
-#[cfg(unix)]
-lazy_static! {
-    static ref LS_COLORS: String =
-        std::env::var("LS_COLORS").unwrap_or_else(|_| DEFAULT_COLORS.to_string());
-    static ref COLOR_MAP: HashMap<&'static str, &'static str> = {
-        let codes = LS_COLORS.split(':');
-        let mut map = HashMap::new();
-        for c in codes {
-            let p: Vec<_> = c.splitn(2, '=').collect();
-            if p.len() == 2 {
-                map.insert(p[0], p[1]);
-            }
-        }
-        map
-    };
-    static ref RESET_CODE: &'static str = COLOR_MAP.get("rs").unwrap_or(&"0");
-    static ref LEFT_CODE: &'static str = COLOR_MAP.get("lc").unwrap_or(&"\x1b[");
-    static ref RIGHT_CODE: &'static str = COLOR_MAP.get("rc").unwrap_or(&"m");
-    static ref END_CODE: &'static str = COLOR_MAP.get("ec").unwrap_or(&"");
-}
-
 pub mod options {
     pub mod format {
         pub static ONELINE: &str = "1";
@@ -120,6 +92,11 @@ pub mod options {
         pub static FILE_TYPE: &str = "file-type";
         pub static CLASSIFY: &str = "classify";
     }
+    pub mod dereference {
+        pub static ALL: &str = "dereference";
+        pub static ARGS: &str = "dereference-command-line";
+        pub static DIR_ARGS: &str = "dereference-command-line-symlink-to-dir";
+    }
     pub static HIDE_CONTROL_CHARS: &str = "hide-control-chars";
     pub static SHOW_CONTROL_CHARS: &str = "show-control-chars";
     pub static WIDTH: &str = "width";
@@ -134,7 +111,6 @@ pub mod options {
     pub static FILE_TYPE: &str = "file-type";
     pub static SLASH: &str = "p";
     pub static INODE: &str = "inode";
-    pub static DEREFERENCE: &str = "dereference";
     pub static REVERSE: &str = "reverse";
     pub static RECURSIVE: &str = "recursive";
     pub static COLOR: &str = "color";
@@ -180,6 +156,13 @@ enum Time {
     Change,
 }
 
+enum Dereference {
+    None,
+    DirArgs,
+    Args,
+    All,
+}
+
 #[derive(PartialEq, Eq)]
 enum IndicatorStyle {
     None,
@@ -194,15 +177,14 @@ struct Config {
     sort: Sort,
     recursive: bool,
     reverse: bool,
-    dereference: bool,
+    dereference: Dereference,
     ignore_patterns: GlobSet,
     size_format: SizeFormat,
     directory: bool,
     time: Time,
     #[cfg(unix)]
     inode: bool,
-    #[cfg(unix)]
+    color: Option<LsColors>,
-    color: bool,
     long: LongFormat,
     width: Option<u16>,
     quoting_style: QuotingStyle,
@@ -326,8 +308,7 @@ impl Config {
             Time::Modification
         };
 
-        #[cfg(unix)]
+        let needs_color = match options.value_of(options::COLOR) {
-        let color = match options.value_of(options::COLOR) {
             None => options.is_present(options::COLOR),
             Some(val) => match val {
                 "" | "always" | "yes" | "force" => true,
@@ -336,6 +317,12 @@ impl Config {
             },
         };
 
+        let color = if needs_color {
+            Some(LsColors::from_env().unwrap_or_default())
+        } else {
+            None
+        };
+
         let size_format = if options.is_present(options::size::HUMAN_READABLE) {
             SizeFormat::Binary
         } else if options.is_present(options::size::SI) {
@@ -483,18 +470,32 @@ impl Config {
 
         let ignore_patterns = ignore_patterns.build().unwrap();
 
+        let dereference = if options.is_present(options::dereference::ALL) {
+            Dereference::All
+        } else if options.is_present(options::dereference::ARGS) {
+            Dereference::Args
+        } else if options.is_present(options::dereference::DIR_ARGS) {
+            Dereference::DirArgs
+        } else if options.is_present(options::DIRECTORY)
+            || indicator_style == IndicatorStyle::Classify
+            || format == Format::Long
+        {
+            Dereference::None
+        } else {
+            Dereference::DirArgs
+        };
+
         Config {
             format,
             files,
             sort,
             recursive: options.is_present(options::RECURSIVE),
             reverse: options.is_present(options::REVERSE),
-            dereference: options.is_present(options::DEREFERENCE),
+            dereference,
             ignore_patterns,
             size_format,
             directory: options.is_present(options::DIRECTORY),
             time,
-            #[cfg(unix)]
             color,
             #[cfg(unix)]
             inode: options.is_present(options::INODE),
@@ -820,6 +821,48 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 ])
         )
 
+        // Dereferencing
+        .arg(
+            Arg::with_name(options::dereference::ALL)
+                .short("L")
+                .long(options::dereference::ALL)
+                .help(
+                    "When showing file information for a symbolic link, show information for the \
+                file the link references rather than the link itself.",
+                )
+                .overrides_with_all(&[
+                    options::dereference::ALL,
+                    options::dereference::DIR_ARGS,
+                    options::dereference::ARGS,
+                ])
+        )
+        .arg(
+            Arg::with_name(options::dereference::DIR_ARGS)
+                .long(options::dereference::DIR_ARGS)
+                .help(
+                    "Do not dereference symlinks except when they link to directories and are \
+                    given as command line arguments.",
+                )
+                .overrides_with_all(&[
+                    options::dereference::ALL,
+                    options::dereference::DIR_ARGS,
+                    options::dereference::ARGS,
+                ])
+        )
+        .arg(
+            Arg::with_name(options::dereference::ARGS)
+                .short("H")
+                .long(options::dereference::ARGS)
+                .help(
+                    "Do not dereference symlinks except when given as command line arguments.",
+                )
+                .overrides_with_all(&[
+                    options::dereference::ALL,
+                    options::dereference::DIR_ARGS,
+                    options::dereference::ARGS,
+                ])
+        )
+
         // Long format options
         .arg(
             Arg::with_name(options::NO_GROUP)
@@ -878,15 +921,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 .long(options::INODE)
                 .help("print the index number of each file"),
         )
-        .arg(
-            Arg::with_name(options::DEREFERENCE)
-                .short("L")
-                .long(options::DEREFERENCE)
-                .help(
-                    "When showing file information for a symbolic link, show information for the \
-                file the link references rather than the link itself.",
-                ),
-        )
         .arg(
             Arg::with_name(options::REVERSE)
                 .short("r")
@@ -979,12 +1013,43 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     list(locs, Config::from(matches))
 }
 
+/// Represents a Path along with it's associated data
+/// Any data that will be reused several times makes sense to be added to this structure
+/// Caching data here helps eliminate redundant syscalls to fetch same information
+struct PathData {
+    // Result<MetaData> got from symlink_metadata() or metadata() based on config
+    md: std::io::Result<Metadata>,
+    // String formed from get_lossy_string() for the path
+    lossy_string: String,
+    // Name of the file - will be empty for . or ..
+    file_name: String,
+    // PathBuf that all above data corresponds to
+    p_buf: PathBuf,
+}
+
+impl PathData {
+    fn new(p_buf: PathBuf, config: &Config, command_line: bool) -> Self {
+        let md = get_metadata(&p_buf, config, command_line);
+        let lossy_string = p_buf.to_string_lossy().into_owned();
+        let name = p_buf
+            .file_name()
+            .map_or(String::new(), |s| s.to_string_lossy().into_owned());
+        Self {
+            md,
+            lossy_string,
+            file_name: name,
+            p_buf,
+        }
+    }
+}
+
 fn list(locs: Vec<String>, config: Config) -> i32 {
     let number_of_locs = locs.len();
 
-    let mut files = Vec::<PathBuf>::new();
+    let mut files = Vec::<PathData>::new();
-    let mut dirs = Vec::<PathBuf>::new();
+    let mut dirs = Vec::<PathData>::new();
     let mut has_failed = false;
+
     for loc in locs {
         let p = PathBuf::from(&loc);
         if !p.exists() {
@@ -994,22 +1059,20 @@ fn list(locs: Vec<String>, config: Config) -> i32 {
             has_failed = true;
             continue;
         }
-        let mut dir = false;
 
-        if p.is_dir() && !config.directory {
+        let path_data = PathData::new(p, &config, true);
-            dir = true;
+
-            if config.format == Format::Long && !config.dereference {
+        let show_dir_contents = if let Ok(md) = path_data.md.as_ref() {
-                if let Ok(md) = p.symlink_metadata() {
+            !config.directory && md.is_dir()
-                    if md.file_type().is_symlink() && !p.ends_with("/") {
-                        dir = false;
-                    }
-                }
-            }
-        }
-        if dir {
-            dirs.push(p);
         } else {
-            files.push(p);
+            has_failed = true;
+            false
+        };
+
+        if show_dir_contents {
+            dirs.push(path_data);
+        } else {
+            files.push(path_data);
         }
     }
     sort_entries(&mut files, &config);
@@ -1018,7 +1081,7 @@ fn list(locs: Vec<String>, config: Config) -> i32 {
     sort_entries(&mut dirs, &config);
     for dir in dirs {
         if number_of_locs > 1 {
-            println!("\n{}:", dir.to_string_lossy());
+            println!("\n{}:", dir.lossy_string);
         }
         enter_directory(&dir, &config);
     }
@@ -1029,21 +1092,22 @@ fn list(locs: Vec<String>, config: Config) -> i32 {
     }
 }
 
-fn sort_entries(entries: &mut Vec<PathBuf>, config: &Config) {
+fn sort_entries(entries: &mut Vec<PathData>, config: &Config) {
     match config.sort {
         Sort::Time => entries.sort_by_key(|k| {
             Reverse(
-                get_metadata(k, config)
+                k.md.as_ref()
                     .ok()
                     .and_then(|md| get_system_time(&md, config))
                     .unwrap_or(UNIX_EPOCH),
             )
         }),
-        Sort::Size => entries
+        Sort::Size => {
-            .sort_by_key(|k| Reverse(get_metadata(k, config).map(|md| md.len()).unwrap_or(0))),
+            entries.sort_by_key(|k| Reverse(k.md.as_ref().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::Name => entries.sort_by_key(|k| k.file_name.to_lowercase()),
-        Sort::Version => entries.sort_by(|a, b| version_cmp::version_cmp(a, b)),
+        Sort::Version => entries.sort_by(|k, j| version_cmp::version_cmp(&k.p_buf, &j.p_buf)),
         Sort::None => {}
     }
 
@@ -1077,41 +1141,66 @@ fn should_display(entry: &DirEntry, config: &Config) -> bool {
     true
 }
 
-fn enter_directory(dir: &Path, config: &Config) {
+fn enter_directory(dir: &PathData, config: &Config) {
-    let mut entries: Vec<_> = safe_unwrap!(fs::read_dir(dir).and_then(Iterator::collect));
+    let mut entries: Vec<_> = if config.files == Files::All {
-
+        vec![
-    entries.retain(|e| should_display(e, config));
+            PathData::new(dir.p_buf.join("."), config, false),
-
+            PathData::new(dir.p_buf.join(".."), config, false),
-    let mut entries: Vec<_> = entries.iter().map(DirEntry::path).collect();
+        ]
-    sort_entries(&mut entries, config);
-
-    if config.files == Files::All {
-        let mut display_entries = entries.clone();
-        display_entries.insert(0, dir.join(".."));
-        display_entries.insert(0, dir.join("."));
-        display_items(&display_entries, Some(dir), config);
     } else {
-        display_items(&entries, Some(dir), config);
+        vec![]
-    }
+    };
+
+    let mut temp: Vec<_> = safe_unwrap!(fs::read_dir(&dir.p_buf))
+        .map(|res| safe_unwrap!(res))
+        .filter(|e| should_display(e, config))
+        .map(|e| PathData::new(DirEntry::path(&e), config, false))
+        .collect();
+
+    sort_entries(&mut temp, config);
+
+    entries.append(&mut temp);
+
+    display_items(&entries, Some(&dir.p_buf), config);
 
     if config.recursive {
-        for e in entries.iter().filter(|p| p.is_dir()) {
+        for e in entries
-            println!("\n{}:", e.to_string_lossy());
+            .iter()
+            .skip(if config.files == Files::All { 2 } else { 0 })
+            .filter(|p| p.md.as_ref().map(|md| md.is_dir()).unwrap_or(false))
+        {
+            println!("\n{}:", e.lossy_string);
             enter_directory(&e, config);
         }
     }
 }
 
-fn get_metadata(entry: &Path, config: &Config) -> std::io::Result<Metadata> {
+fn get_metadata(entry: &Path, config: &Config, command_line: bool) -> std::io::Result<Metadata> {
-    if config.dereference {
+    let dereference = match &config.dereference {
+        Dereference::All => true,
+        Dereference::Args => command_line,
+        Dereference::DirArgs => {
+            if command_line {
+                if let Ok(md) = entry.metadata() {
+                    md.is_dir()
+                } else {
+                    false
+                }
+            } else {
+                false
+            }
+        }
+        Dereference::None => false,
+    };
+    if dereference {
         entry.metadata().or_else(|_| entry.symlink_metadata())
     } else {
         entry.symlink_metadata()
     }
 }
 
-fn display_dir_entry_size(entry: &Path, config: &Config) -> (usize, usize) {
+fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize) {
-    if let Ok(md) = get_metadata(entry, config) {
+    if let Ok(md) = entry.md.as_ref() {
         (
             display_symlink_count(&md).len(),
             display_file_size(&md, config).len(),
@@ -1125,7 +1214,7 @@ fn pad_left(string: String, count: usize) -> String {
     format!("{:>width$}", string, width = count)
 }
 
-fn display_items(items: &[PathBuf], strip: Option<&Path>, config: &Config) {
+fn display_items(items: &[PathData], strip: Option<&Path>, config: &Config) {
     if config.format == Format::Long {
         let (mut max_links, mut max_size) = (1, 1);
         for item in items {
@@ -1138,14 +1227,14 @@ fn display_items(items: &[PathBuf], strip: Option<&Path>, config: &Config) {
         }
     } else {
         let names = items.iter().filter_map(|i| {
-            let md = get_metadata(i, config);
+            let md = i.md.as_ref();
             match md {
                 Err(e) => {
-                    let filename = get_file_name(i, strip);
+                    let filename = get_file_name(&i.p_buf, strip);
                     show_error!("'{}': {}", filename, e);
                     None
                 }
-                Ok(md) => Some(display_file_name(&i, strip, &md, config)),
+                Ok(md) => Some(display_file_name(&i.p_buf, strip, &md, config)),
             }
         });
 
@@ -1205,15 +1294,15 @@ fn display_grid(names: impl Iterator<Item = Cell>, width: u16, direction: Direct
 use uucore::fs::display_permissions;
 
 fn display_item_long(
-    item: &Path,
+    item: &PathData,
     strip: Option<&Path>,
     max_links: usize,
     max_size: usize,
     config: &Config,
 ) {
-    let md = match get_metadata(item, config) {
+    let md = match &item.md {
         Err(e) => {
-            let filename = get_file_name(&item, strip);
+            let filename = get_file_name(&item.p_buf, strip);
             show_error!("{}: {}", filename, e);
             return;
         }
@@ -1252,7 +1341,7 @@ fn display_item_long(
         " {} {} {}",
         pad_left(display_file_size(&md, config), max_size),
         display_date(&md, config),
-        display_file_name(&item, strip, &md, config).contents,
+        display_file_name(&item.p_buf, strip, &md, config).contents,
     );
 }
 
@@ -1264,14 +1353,50 @@ fn get_inode(metadata: &Metadata) -> String {
 // Currently getpwuid is `linux` target only. If it's broken out into
 // a posix-compliant attribute this can be updated...
 #[cfg(unix)]
+use std::sync::Mutex;
+#[cfg(unix)]
 use uucore::entries;
 
+#[cfg(unix)]
+fn cached_uid2usr(uid: u32) -> String {
+    lazy_static! {
+        static ref UID_CACHE: Mutex<HashMap<u32, String>> = Mutex::new(HashMap::new());
+    }
+
+    let mut uid_cache = UID_CACHE.lock().unwrap();
+    match uid_cache.get(&uid) {
+        Some(usr) => usr.clone(),
+        None => {
+            let usr = entries::uid2usr(uid).unwrap_or_else(|_| uid.to_string());
+            uid_cache.insert(uid, usr.clone());
+            usr
+        }
+    }
+}
+
 #[cfg(unix)]
 fn display_uname(metadata: &Metadata, config: &Config) -> String {
     if config.long.numeric_uid_gid {
         metadata.uid().to_string()
     } else {
-        entries::uid2usr(metadata.uid()).unwrap_or_else(|_| metadata.uid().to_string())
+        cached_uid2usr(metadata.uid())
+    }
+}
+
+#[cfg(unix)]
+fn cached_gid2grp(gid: u32) -> String {
+    lazy_static! {
+        static ref GID_CACHE: Mutex<HashMap<u32, String>> = Mutex::new(HashMap::new());
+    }
+
+    let mut gid_cache = GID_CACHE.lock().unwrap();
+    match gid_cache.get(&gid) {
+        Some(grp) => grp.clone(),
+        None => {
+            let grp = entries::gid2grp(gid).unwrap_or_else(|_| gid.to_string());
+            gid_cache.insert(gid, grp.clone());
+            grp
+        }
     }
 }
 
@@ -1280,7 +1405,7 @@ fn display_group(metadata: &Metadata, config: &Config) -> String {
     if config.long.numeric_uid_gid {
         metadata.gid().to_string()
     } else {
-        entries::gid2grp(metadata.gid()).unwrap_or_else(|_| metadata.gid().to_string())
+        cached_gid2grp(metadata.gid())
     }
 }
 
@@ -1386,140 +1511,63 @@ fn get_file_name(name: &Path, strip: Option<&Path>) -> String {
     name.to_string_lossy().into_owned()
 }
 
-#[cfg(not(unix))]
+#[cfg(unix)]
-fn display_file_name(
+fn file_is_executable(md: &Metadata) -> bool {
-    path: &Path,
+    // Mode always returns u32, but the flags might not be, based on the platform
-    strip: Option<&Path>,
+    // e.g. linux has u32, mac has u16.
-    metadata: &Metadata,
+    // S_IXUSR -> user has execute permission
-    config: &Config,
+    // S_IXGRP -> group has execute persmission
-) -> Cell {
+    // S_IXOTH -> other users have execute permission
-    let mut name = escape_name(get_file_name(path, strip), &config.quoting_style);
+    md.mode() & ((S_IXUSR | S_IXGRP | S_IXOTH) as u32) != 0
-    let file_type = metadata.file_type();
-
-    match config.indicator_style {
-        IndicatorStyle::Classify | IndicatorStyle::FileType => {
-            if file_type.is_dir() {
-                name.push('/');
-            }
-            if file_type.is_symlink() {
-                name.push('@');
-            }
-        }
-        IndicatorStyle::Slash => {
-            if file_type.is_dir() {
-                name.push('/');
-            }
-        }
-        _ => (),
-    };
-
-    if config.format == Format::Long && metadata.file_type().is_symlink() {
-        if let Ok(target) = path.read_link() {
-            // We don't bother updating width here because it's not used for long listings
-            let target_name = target.to_string_lossy().to_string();
-            name.push_str(" -> ");
-            name.push_str(&target_name);
-        }
-    }
-
-    name.into()
 }
 
-#[cfg(unix)]
+#[allow(clippy::clippy::collapsible_else_if)]
-fn color_name(name: String, typ: &str) -> String {
+fn classify_file(md: &Metadata) -> Option<char> {
-    let mut typ = typ;
+    let file_type = md.file_type();
-    if !COLOR_MAP.contains_key(typ) {
+
-        if typ == "or" {
+    if file_type.is_dir() {
-            typ = "ln";
+        Some('/')
-        } else if typ == "mi" {
+    } else if file_type.is_symlink() {
-            typ = "fi";
+        Some('@')
-        }
-    };
-    if let Some(code) = COLOR_MAP.get(typ) {
-        format!(
-            "{}{}{}{}{}{}{}{}",
-            *LEFT_CODE, code, *RIGHT_CODE, name, *END_CODE, *LEFT_CODE, *RESET_CODE, *RIGHT_CODE,
-        )
     } else {
-        name
+        #[cfg(unix)]
-    }
+        {
-}
+            if file_type.is_socket() {
-
+                Some('=')
-#[cfg(unix)]
+            } else if file_type.is_fifo() {
-macro_rules! has {
+                Some('|')
-    ($mode:expr, $perm:expr) => {
+            } else if file_type.is_file() && file_is_executable(&md) {
-        $mode & ($perm as mode_t) != 0
-    };
-}
-
-#[cfg(unix)]
-#[allow(clippy::cognitive_complexity)]
-fn display_file_name(
-    path: &Path,
-    strip: Option<&Path>,
-    metadata: &Metadata,
-    config: &Config,
-) -> Cell {
-    let mut name = escape_name(get_file_name(path, strip), &config.quoting_style);
-    if config.format != Format::Long && config.inode {
-        name = get_inode(metadata) + " " + &name;
-    }
-    let mut width = UnicodeWidthStr::width(&*name);
-
-    let ext;
-    if config.color || config.indicator_style != IndicatorStyle::None {
-        let file_type = metadata.file_type();
-
-        let (code, sym) = if file_type.is_dir() {
-            ("di", Some('/'))
-        } else if file_type.is_symlink() {
-            if path.exists() {
-                ("ln", Some('@'))
-            } else {
-                ("or", Some('@'))
-            }
-        } else if file_type.is_socket() {
-            ("so", Some('='))
-        } else if file_type.is_fifo() {
-            ("pi", Some('|'))
-        } else if file_type.is_block_device() {
-            ("bd", None)
-        } else if file_type.is_char_device() {
-            ("cd", None)
-        } else if file_type.is_file() {
-            let mode = metadata.mode() as mode_t;
-            let sym = if has!(mode, S_IXUSR | S_IXGRP | S_IXOTH) {
                 Some('*')
             } else {
                 None
-            };
-            if has!(mode, S_ISUID) {
-                ("su", sym)
-            } else if has!(mode, S_ISGID) {
-                ("sg", sym)
-            } else if has!(mode, S_ISVTX) && has!(mode, S_IWOTH) {
-                ("tw", sym)
-            } else if has!(mode, S_ISVTX) {
-                ("st", sym)
-            } else if has!(mode, S_IWOTH) {
-                ("ow", sym)
-            } else if has!(mode, S_IXUSR | S_IXGRP | S_IXOTH) {
-                ("ex", sym)
-            } else if metadata.nlink() > 1 {
-                ("mh", sym)
-            } else if let Some(e) = path.extension() {
-                ext = format!("*.{}", e.to_string_lossy());
-                (ext.as_str(), None)
-            } else {
-                ("fi", None)
             }
-        } else {
-            ("", None)
-        };
-
-        if config.color {
-            name = color_name(name, code);
         }
+        #[cfg(not(unix))]
+        None
+    }
+}
+
+fn display_file_name(
+    path: &Path,
+    strip: Option<&Path>,
+    metadata: &Metadata,
+    config: &Config,
+) -> Cell {
+    let mut name = escape_name(get_file_name(path, strip), &config.quoting_style);
+
+    #[cfg(unix)]
+    {
+        if config.format != Format::Long && config.inode {
+            name = get_inode(metadata) + " " + &name;
+        }
+    }
+
+    if let Some(ls_colors) = &config.color {
+        name = color_name(&ls_colors, path, name, metadata);
+    }
+
+    if config.indicator_style != IndicatorStyle::None {
+        let sym = classify_file(metadata);
 
         let char_opt = match config.indicator_style {
             IndicatorStyle::Classify => sym,
@@ -1542,23 +1590,24 @@ fn display_file_name(
 
         if let Some(c) = char_opt {
             name.push(c);
-            width += 1;
         }
     }
 
     if config.format == Format::Long && metadata.file_type().is_symlink() {
         if let Ok(target) = path.read_link() {
-            // We don't bother updating width here because it's not used for long listings
+            // We don't bother updating width here because it's not used for long
-            let code = if target.exists() { "fi" } else { "mi" };
-            let target_name = color_name(target.to_string_lossy().to_string(), code);
             name.push_str(" -> ");
-            name.push_str(&target_name);
+            name.push_str(&target.to_string_lossy());
         }
     }
 
-    Cell {
+    name.into()
-        contents: name,
+}
-        width,
+
+fn color_name(ls_colors: &LsColors, path: &Path, name: String, md: &Metadata) -> String {
+    match ls_colors.style_for_path_with_metadata(path, Some(&md)) {
+        Some(style) => style.to_ansi_term_style().paint(name).to_string(),
+        None => name,
     }
 }
 

src/uu/ls/src/quoting_style.rs

@@ -1,6 +1,6 @@
 use std::char::from_digit;
 
-const SPECIAL_SHELL_CHARS: &str = "~`#$&*()\\|[]{};'\"<>?! ";
+const SPECIAL_SHELL_CHARS: &str = "~`#$&*()|[]{};\\'\"<>?! ";
 
 pub(super) enum QuotingStyle {
     Shell {
@@ -27,12 +27,10 @@ pub(super) enum Quotes {
 // This implementation is heavily inspired by the std::char::EscapeDefault implementation
 // in the Rust standard library. This custom implementation is needed because the
 // characters \a, \b, \e, \f & \v are not recognized by Rust.
-#[derive(Clone, Debug)]
 struct EscapedChar {
     state: EscapeState,
 }
 
-#[derive(Clone, Debug)]
 enum EscapeState {
     Done,
     Char(char),
@@ -41,14 +39,12 @@ enum EscapeState {
     Octal(EscapeOctal),
 }
 
-#[derive(Clone, Debug)]
 struct EscapeOctal {
     c: char,
     state: EscapeOctalState,
     idx: usize,
 }
 
-#[derive(Clone, Debug)]
 enum EscapeOctalState {
     Done,
     Backslash,
@@ -135,7 +131,6 @@ impl EscapedChar {
             '\x0B' => Backslash('v'),
             '\x0C' => Backslash('f'),
             '\r' => Backslash('r'),
-            '\\' => Backslash('\\'),
             '\x00'..='\x1F' | '\x7F' => Octal(EscapeOctal::from(c)),
             '\'' => match quotes {
                 Quotes::Single => Backslash('\''),
@@ -511,6 +506,23 @@ mod tests {
             ],
         );
 
+        // A control character followed by a special shell character
+        check_names(
+            "one\n&two",
+            vec![
+                ("one?&two", "literal"),
+                ("one\n&two", "literal-show"),
+                ("one\\n&two", "escape"),
+                ("\"one\\n&two\"", "c"),
+                ("'one?&two'", "shell"),
+                ("'one\n&two'", "shell-show"),
+                ("'one?&two'", "shell-always"),
+                ("'one\n&two'", "shell-always-show"),
+                ("'one'$'\\n''&two'", "shell-escape"),
+                ("'one'$'\\n''&two'", "shell-escape-always"),
+            ],
+        );
+
         // The first 16 control characters. NUL is also included, even though it is of
         // no importance for file names.
         check_names(
@@ -627,4 +639,22 @@ mod tests {
             ],
         );
     }
+
+    #[test]
+    fn test_backslash() {
+        // Escaped in C-style, but not in Shell-style escaping
+        check_names(
+            "one\\two",
+            vec![
+                ("one\\two", "literal"),
+                ("one\\two", "literal-show"),
+                ("one\\\\two", "escape"),
+                ("\"one\\\\two\"", "c"),
+                ("'one\\two'", "shell"),
+                ("\'one\\two\'", "shell-always"),
+                ("'one\\two'", "shell-escape"),
+                ("'one\\two'", "shell-escape-always"),
+            ],
+        );
+    }
 }

src/uu/printf/src/cli.rs

@@ -18,18 +18,15 @@ pub fn err_msg(msg: &str) {
 
 // by default stdout only flushes
 // to console when a newline is passed.
-#[allow(unused_must_use)]
 pub fn flush_char(c: char) {
     print!("{}", c);
-    stdout().flush();
+    let _ = stdout().flush();
 }
-#[allow(unused_must_use)]
 pub fn flush_str(s: &str) {
     print!("{}", s);
-    stdout().flush();
+    let _ = stdout().flush();
 }
-#[allow(unused_must_use)]
 pub fn flush_bytes(bslice: &[u8]) {
-    stdout().write(bslice);
+    let _ = stdout().write(bslice);
-    stdout().flush();
+    let _ = stdout().flush();
 }

src/uu/printf/src/printf.rs

@@ -1,5 +1,4 @@
 #![allow(dead_code)]
-
 // spell-checker:ignore (change!) each's
 // spell-checker:ignore (ToDO) LONGHELP FORMATSTRING templating parameterizing formatstr
 
@@ -9,7 +8,6 @@ mod tokenize;
 
 static NAME: &str = "printf";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
-static SHORT_USAGE: &str = "printf: usage: printf [-v var] format [arguments]";
 static LONGHELP_LEAD: &str = "printf
 
  USAGE: printf FORMATSTRING [ARGUMENT]...

src/uu/printf/src/tokenize/num_format/formatters/base_conv/mod.rs

@@ -28,8 +28,7 @@ pub fn arrnum_int_mult(arr_num: &[u8], basenum: u8, base_ten_int_fact: u8) -> Ve
             }
         }
     }
-    #[allow(clippy::map_clone)]
+    let ret: Vec<u8> = ret_rev.into_iter().rev().collect();
-    let ret: Vec<u8> = ret_rev.iter().rev().map(|x| *x).collect();
     ret
 }
 
@@ -102,70 +101,6 @@ pub fn arrnum_int_div_step(
         remainder: rem_out,
     }
 }
-// pub struct ArrFloat {
-// pub leading_zeros: u8,
-// pub values: Vec<u8>,
-// pub basenum: u8
-// }
-//
-// pub struct ArrFloatDivOut {
-// pub quotient: u8,
-// pub remainder: ArrFloat
-// }
-//
-// pub fn arrfloat_int_div(
-// arrfloat_in : &ArrFloat,
-// base_ten_int_divisor : u8,
-// precision : u16
-// ) -> DivOut  {
-//
-// let mut remainder = ArrFloat {
-// basenum: arrfloat_in.basenum,
-// leading_zeros: arrfloat_in.leading_zeroes,
-// values: Vec<u8>::new()
-// }
-// let mut quotient = 0;
-//
-// let mut bufferval : u16 = 0;
-// let base : u16 = arrfloat_in.basenum as u16;
-// let divisor : u16 = base_ten_int_divisor as u16;
-//
-// let mut it_f = arrfloat_in.values.iter();
-// let mut position = 0 + arrfloat_in.leading_zeroes as u16;
-// let mut at_end = false;
-// while position< precision {
-// let next_digit = match it_f.next() {
-// Some(c) => {}
-// None => { 0 }
-// }
-// match u_cur {
-// Some(u) => {
-// bufferval += u.clone() as u16;
-// if bufferval > divisor {
-// while bufferval >= divisor {
-// quotient+=1;
-// bufferval -= divisor;
-// }
-// if bufferval == 0 {
-// rem_out.position +=1;
-// } else {
-// rem_out.replace = Some(bufferval as u8);
-// }
-// break;
-// } else {
-// bufferval *= base;
-// }
-// },
-// None => {
-// break;
-// }
-// }
-// u_cur = it_f.next().clone();
-// rem_out.position+=1;
-// }
-// ArrFloatDivOut { quotient: quotient, remainder: remainder }
-// }
-//
 pub fn arrnum_int_add(arrnum: &[u8], basenum: u8, base_ten_int_term: u8) -> Vec<u8> {
     let mut carry: u16 = u16::from(base_ten_int_term);
     let mut rem: u16;
@@ -193,8 +128,7 @@ pub fn arrnum_int_add(arrnum: &[u8], basenum: u8, base_ten_int_term: u8) -> Vec<
             }
         }
     }
-    #[allow(clippy::map_clone)]
+    let ret: Vec<u8> = ret_rev.into_iter().rev().collect();
-    let ret: Vec<u8> = ret_rev.iter().rev().map(|x| *x).collect();
     ret
 }
 
@@ -219,8 +153,7 @@ pub fn unsigned_to_arrnum(src: u16) -> Vec<u8> {
 }
 
 // temporary needs-improvement-function
-#[allow(unused_variables)]
+pub fn base_conv_float(src: &[u8], radix_src: u8, _radix_dest: u8) -> f64 {
-pub fn base_conv_float(src: &[u8], radix_src: u8, radix_dest: u8) -> f64 {
     // it would require a lot of addl code
     // to implement this for arbitrary string input.
     // until then, the below operates as an outline
@@ -267,7 +200,6 @@ pub fn arrnum_to_str(src: &[u8], radix_def_dest: &dyn RadixDef) -> String {
     str_out
 }
 
-#[allow(unused_variables)]
 pub fn base_conv_str(
     src: &str,
     radix_def_src: &dyn RadixDef,

src/uu/printf/src/tokenize/num_format/formatters/cninetyninehexfloatf.rs

@@ -43,45 +43,15 @@ impl Formatter for CninetyNineHexFloatf {
 // c99 hex has unique requirements of all floating point subs in pretty much every part of building a primitive, from prefix and suffix to need for base conversion (in all other cases if you don't have decimal you must have decimal, here it's the other way around)
 
 // on the todo list is to have a trait for get_primitive that is implemented by each float formatter and can override a default. when that happens we can take the parts of get_primitive_dec specific to dec and spin them out to their own functions that can be overridden.
-#[allow(unused_variables)]
-#[allow(unused_assignments)]
 fn get_primitive_hex(
     inprefix: &InPrefix,
-    str_in: &str,
+    _str_in: &str,
-    analysis: &FloatAnalysis,
+    _analysis: &FloatAnalysis,
-    last_dec_place: usize,
+    _last_dec_place: usize,
     capitalized: bool,
 ) -> FormatPrimitive {
     let prefix = Some(String::from(if inprefix.sign == -1 { "-0x" } else { "0x" }));
 
-    // assign the digits before and after the decimal points
-    // to separate slices. If no digits after decimal point,
-    // assign 0
-    let (mut first_segment_raw, second_segment_raw) = match analysis.decimal_pos {
-        Some(pos) => (&str_in[..pos], &str_in[pos + 1..]),
-        None => (str_in, "0"),
-    };
-    if first_segment_raw.is_empty() {
-        first_segment_raw = "0";
-    }
-    // convert to string, hexifying if input is in dec.
-    // let (first_segment, second_segment) =
-    // match inprefix.radix_in {
-    // Base::Ten => {
-    // (to_hex(first_segment_raw, true),
-    // to_hex(second_segment_raw, false))
-    // }
-    // _ => {
-    // (String::from(first_segment_raw),
-    // String::from(second_segment_raw))
-    // }
-    // };
-    //
-    //
-    // f.pre_decimal = Some(first_segment);
-    // f.post_decimal = Some(second_segment);
-    //
-
     // TODO actual conversion, make sure to get back mantissa.
     // for hex to hex, it's really just a matter of moving the
     // decimal point and calculating the mantissa by its initial

src/uu/printf/src/tokenize/num_format/formatters/decf.rs

@@ -22,12 +22,11 @@ fn get_len_fprim(fprim: &FormatPrimitive) -> usize {
     len
 }
 
-pub struct Decf {
+pub struct Decf;
-    as_num: f64,
+
-}
 impl Decf {
     pub fn new() -> Decf {
-        Decf { as_num: 0.0 }
+        Decf
     }
 }
 impl Formatter for Decf {

src/uu/printf/src/tokenize/num_format/formatters/floatf.rs

@@ -5,12 +5,10 @@ use super::super::format_field::FormatField;
 use super::super::formatter::{FormatPrimitive, Formatter, InPrefix};
 use super::float_common::{get_primitive_dec, primitive_to_str_common, FloatAnalysis};
 
-pub struct Floatf {
+pub struct Floatf;
-    as_num: f64,
-}
 impl Floatf {
     pub fn new() -> Floatf {
-        Floatf { as_num: 0.0 }
+        Floatf
     }
 }
 impl Formatter for Floatf {

src/uu/printf/src/tokenize/num_format/formatters/intf.rs

@@ -11,7 +11,7 @@ use std::i64;
 use std::u64;
 
 pub struct Intf {
-    a: u32,
+    _a: u32,
 }
 
 // see the Intf::analyze() function below
@@ -24,7 +24,7 @@ struct IntAnalysis {
 
 impl Intf {
     pub fn new() -> Intf {
-        Intf { a: 0 }
+        Intf { _a: 0 }
     }
     // take a ref to argument string, and basic information
     // about prefix (offset, radix, sign), and analyze string

src/uu/printf/src/tokenize/num_format/formatters/scif.rs

@@ -5,12 +5,11 @@ use super::super::format_field::FormatField;
 use super::super::formatter::{FormatPrimitive, Formatter, InPrefix};
 use super::float_common::{get_primitive_dec, primitive_to_str_common, FloatAnalysis};
 
-pub struct Scif {
+pub struct Scif;
-    as_num: f64,
+
-}
 impl Scif {
     pub fn new() -> Scif {
-        Scif { as_num: 0.0 }
+        Scif
     }
 }
 impl Formatter for Scif {

src/uu/printf/src/tokenize/unescaped_text.rs

@@ -242,18 +242,16 @@ impl UnescapedText {
         }
     }
 }
-#[allow(unused_variables)]
 impl token::Tokenizer for UnescapedText {
     fn from_it(
         it: &mut PutBackN<Chars>,
-        args: &mut Peekable<Iter<String>>,
+        _: &mut Peekable<Iter<String>>,
     ) -> Option<Box<dyn token::Token>> {
         UnescapedText::from_it_core(it, false)
     }
 }
-#[allow(unused_variables)]
 impl token::Token for UnescapedText {
-    fn print(&self, pf_args_it: &mut Peekable<Iter<String>>) {
+    fn print(&self, _: &mut Peekable<Iter<String>>) {
         cli::flush_bytes(&self.0[..]);
     }
 }

src/uu/sort/BENCHMARKING.md

@@ -9,25 +9,125 @@ list that we should improve / make sure not to regress.
 Run `cargo build --release` before benchmarking after you make a change!
 
 ## Sorting a wordlist
-- Get a wordlist, for example with [words](https://en.wikipedia.org/wiki/Words_(Unix)) on Linux. The exact wordlist
+
-  doesn't matter for performance comparisons. In this example I'm using `/usr/share/dict/american-english` as the wordlist.
+-   Get a wordlist, for example with [words](<https://en.wikipedia.org/wiki/Words_(Unix)>) on Linux. The exact wordlist
-- Shuffle the wordlist by running `sort -R /usr/share/dict/american-english > shuffled_wordlist.txt`.
+    doesn't matter for performance comparisons. In this example I'm using `/usr/share/dict/american-english` as the wordlist.
-- Benchmark sorting the wordlist with hyperfine: `hyperfine "target/release/coreutils sort shuffled_wordlist.txt -o output.txt"`.
+-   Shuffle the wordlist by running `sort -R /usr/share/dict/american-english > shuffled_wordlist.txt`.
+-   Benchmark sorting the wordlist with hyperfine: `hyperfine "target/release/coreutils sort shuffled_wordlist.txt -o output.txt"`.
 
 ## Sorting a wordlist with ignore_case
-- Same wordlist as above
+
-- Benchmark sorting the wordlist ignoring the case with hyperfine: `hyperfine "target/release/coreutils sort shuffled_wordlist.txt -f -o output.txt"`.
+-   Same wordlist as above
+-   Benchmark sorting the wordlist ignoring the case with hyperfine: `hyperfine "target/release/coreutils sort shuffled_wordlist.txt -f -o output.txt"`.
 
 ## Sorting numbers
-- Generate a list of numbers: `seq 0 100000 | sort -R > shuffled_numbers.txt`.
+
-- Benchmark numeric sorting with hyperfine: `hyperfine "target/release/coreutils sort shuffled_numbers.txt -n -o output.txt"`.
+-   Generate a list of numbers: `seq 0 100000 | sort -R > shuffled_numbers.txt`.
+-   Benchmark numeric sorting with hyperfine: `hyperfine "target/release/coreutils sort shuffled_numbers.txt -n -o output.txt"`.
+
+## Sorting numbers with -g
+
+-   Same list of numbers as above.
+-   Benchmark numeric sorting with hyperfine: `hyperfine "target/release/coreutils sort shuffled_numbers.txt -g -o output.txt"`.
+
+## Sorting numbers with SI prefixes
+
+-   Generate a list of numbers:
+    <details>
+      <summary>Rust script</summary>
+
+    ## Cargo.toml
+
+    ```toml
+    [dependencies]
+    rand = "0.8.3"
+    ```
+
+    ## main.rs
+
+    ```rust
+    use rand::prelude::*;
+    fn main() {
+      let suffixes = ['k', 'K', 'M', 'G', 'T', 'P', 'E', 'Z', 'Y'];
+      let mut rng = thread_rng();
+      for _ in 0..100000 {
+          println!(
+              "{}{}",
+              rng.gen_range(0..1000000),
+              suffixes.choose(&mut rng).unwrap()
+          )
+      }
+    }
+
+    ```
+
+    ## running
+
+    `cargo run > shuffled_numbers_si.txt`
+
+    </details>
+
+-   Benchmark numeric sorting with hyperfine: `hyperfine "target/release/coreutils sort shuffled_numbers_si.txt -h -o output.txt"`.
 
 ## Stdout and stdin performance
+
 Try to run the above benchmarks by piping the input through stdin (standard input) and redirect the
 output through stdout (standard output):
-- Remove the input file from the arguments and add `cat [inputfile] | ` at the beginning.
+
-- Remove `-o output.txt` and add `> output.txt` at the end.
+-   Remove the input file from the arguments and add `cat [inputfile] | ` at the beginning.
+-   Remove `-o output.txt` and add `> output.txt` at the end.
 
 Example: `hyperfine "target/release/coreutils sort shuffled_numbers.txt -n -o output.txt"` becomes
 `hyperfine "cat shuffled_numbers.txt | target/release/coreutils sort -n > output.txt`
-- Check that performance is similar to the original benchmark.
+
+-   Check that performance is similar to the original benchmark.
+
+## Comparing with GNU sort
+
+Hyperfine accepts multiple commands to run and will compare them. To compare performance with GNU sort
+duplicate the string you passed to hyperfine but remove the `target/release/coreutils` bit from it.
+
+Example: `hyperfine "target/release/coreutils sort shuffled_numbers_si.txt -h -o output.txt"` becomes
+`hyperfine "target/release/coreutils sort shuffled_numbers_si.txt -h -o output.txt" "sort shuffled_numbers_si.txt -h -o output.txt"`
+(This assumes GNU sort is installed as `sort`)
+
+## Memory and CPU usage
+
+The above benchmarks use hyperfine to measure the speed of sorting. There are however other useful metrics to determine overall
+resource usage. One way to measure them is the `time` command. This is not to be confused with the `time` that is built in to the bash shell.
+You may have to install `time` first, then you have to run it with `/bin/time -v` to give it precedence over the built in `time`.
+
+<details>
+      <summary>Example output</summary>
+
+      Command being timed: "target/release/coreutils sort shuffled_numbers.txt"
+      User time (seconds): 0.10
+      System time (seconds): 0.00
+      Percent of CPU this job got: 365%
+      Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.02
+      Average shared text size (kbytes): 0
+      Average unshared data size (kbytes): 0
+      Average stack size (kbytes): 0
+      Average total size (kbytes): 0
+      Maximum resident set size (kbytes): 25360
+      Average resident set size (kbytes): 0
+      Major (requiring I/O) page faults: 0
+      Minor (reclaiming a frame) page faults: 5802
+      Voluntary context switches: 462
+      Involuntary context switches: 73
+      Swaps: 0
+      File system inputs: 1184
+      File system outputs: 0
+      Socket messages sent: 0
+      Socket messages received: 0
+      Signals delivered: 0
+      Page size (bytes): 4096
+      Exit status: 0
+
+</details>
+
+Useful metrics to look at could be:
+
+-   User time
+-   Percent of CPU this job got
+-   Maximum resident set size

src/uu/sort/src/numeric_str_cmp.rs

@@ -0,0 +1,455 @@
+//  * This file is part of the uutils coreutils package.
+//  *
+//  * (c) Michael Debertol <michael.debertol..AT..gmail.com>
+//  *
+//  * For the full copyright and license information, please view the LICENSE
+//  * file that was distributed with this source code.
+
+//! Fast comparison for strings representing a base 10 number without precision loss.
+//!
+//! To be able to short-circuit when comparing, [NumInfo] must be passed along with each number
+//! to [numeric_str_cmp]. [NumInfo] is generally obtained by calling [NumInfo::parse] and should be cached.
+//! It is allowed to arbitrarily modify the exponent afterwards, which is equivalent to shifting the decimal point.
+//!
+//! More specifically, exponent can be understood so that the original number is in (1..10)*10^exponent.
+//! From that follows the constraints of this algorithm: It is able to compare numbers in ±(1*10^[i64::MIN]..10*10^[i64::MAX]).
+
+use std::{cmp::Ordering, ops::Range};
+
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord)]
+enum Sign {
+    Negative,
+    Positive,
+}
+
+#[derive(Debug, PartialEq)]
+pub struct NumInfo {
+    exponent: i64,
+    sign: Sign,
+}
+
+pub struct NumInfoParseSettings {
+    pub accept_si_units: bool,
+    pub thousands_separator: Option<char>,
+    pub decimal_pt: Option<char>,
+}
+
+impl Default for NumInfoParseSettings {
+    fn default() -> Self {
+        Self {
+            accept_si_units: false,
+            thousands_separator: None,
+            decimal_pt: Some('.'),
+        }
+    }
+}
+
+impl NumInfo {
+    /// Parse NumInfo for this number.
+    /// Also returns the range of num that should be passed to numeric_str_cmp later
+    pub fn parse(num: &str, parse_settings: NumInfoParseSettings) -> (Self, Range<usize>) {
+        let mut exponent = -1;
+        let mut had_decimal_pt = false;
+        let mut had_digit = false;
+        let mut start = None;
+        let mut sign = Sign::Positive;
+
+        let mut first_char = true;
+
+        for (idx, char) in num.char_indices() {
+            if first_char && char.is_whitespace() {
+                continue;
+            }
+
+            if first_char && char == '-' {
+                sign = Sign::Negative;
+                first_char = false;
+                continue;
+            }
+            first_char = false;
+
+            if parse_settings
+                .thousands_separator
+                .map_or(false, |c| c == char)
+            {
+                continue;
+            }
+
+            if Self::is_invalid_char(char, &mut had_decimal_pt, &parse_settings) {
+                let si_unit = if parse_settings.accept_si_units {
+                    match char {
+                        'K' | 'k' => 3,
+                        'M' => 6,
+                        'G' => 9,
+                        'T' => 12,
+                        'P' => 15,
+                        'E' => 18,
+                        'Z' => 21,
+                        'Y' => 24,
+                        _ => 0,
+                    }
+                } else {
+                    0
+                };
+                return if let Some(start) = start {
+                    (
+                        NumInfo {
+                            exponent: exponent + si_unit,
+                            sign,
+                        },
+                        start..idx,
+                    )
+                } else {
+                    (
+                        NumInfo {
+                            sign: if had_digit { sign } else { Sign::Positive },
+                            exponent: 0,
+                        },
+                        0..0,
+                    )
+                };
+            }
+            if Some(char) == parse_settings.decimal_pt {
+                continue;
+            }
+            had_digit = true;
+            if start.is_none() && char == '0' {
+                if had_decimal_pt {
+                    // We're parsing a number whose first nonzero digit is after the decimal point.
+                    exponent -= 1;
+                } else {
+                    // Skip leading zeroes
+                    continue;
+                }
+            }
+            if !had_decimal_pt {
+                exponent += 1;
+            }
+            if start.is_none() && char != '0' {
+                start = Some(idx);
+            }
+        }
+        if let Some(start) = start {
+            (NumInfo { exponent, sign }, start..num.len())
+        } else {
+            (
+                NumInfo {
+                    sign: if had_digit { sign } else { Sign::Positive },
+                    exponent: 0,
+                },
+                0..0,
+            )
+        }
+    }
+
+    fn is_invalid_char(
+        c: char,
+        had_decimal_pt: &mut bool,
+        parse_settings: &NumInfoParseSettings,
+    ) -> bool {
+        if Some(c) == parse_settings.decimal_pt {
+            if *had_decimal_pt {
+                // this is a decimal pt but we already had one, so it is invalid
+                true
+            } else {
+                *had_decimal_pt = true;
+                false
+            }
+        } else {
+            !c.is_ascii_digit()
+        }
+    }
+}
+
+/// compare two numbers as strings without parsing them as a number first. This should be more performant and can handle numbers more precisely.
+/// NumInfo is needed to provide a fast path for most numbers.
+pub fn numeric_str_cmp((a, a_info): (&str, &NumInfo), (b, b_info): (&str, &NumInfo)) -> Ordering {
+    // check for a difference in the sign
+    if a_info.sign != b_info.sign {
+        return a_info.sign.cmp(&b_info.sign);
+    }
+
+    // check for a difference in the exponent
+    let ordering = if a_info.exponent != b_info.exponent && !a.is_empty() && !b.is_empty() {
+        a_info.exponent.cmp(&b_info.exponent)
+    } else {
+        // walk the characters from the front until we find a difference
+        let mut a_chars = a.chars().filter(|c| c.is_ascii_digit());
+        let mut b_chars = b.chars().filter(|c| c.is_ascii_digit());
+        loop {
+            let a_next = a_chars.next();
+            let b_next = b_chars.next();
+            match (a_next, b_next) {
+                (None, None) => break Ordering::Equal,
+                (Some(c), None) => {
+                    break if c == '0' && a_chars.all(|c| c == '0') {
+                        Ordering::Equal
+                    } else {
+                        Ordering::Greater
+                    }
+                }
+                (None, Some(c)) => {
+                    break if c == '0' && b_chars.all(|c| c == '0') {
+                        Ordering::Equal
+                    } else {
+                        Ordering::Less
+                    }
+                }
+                (Some(a_char), Some(b_char)) => {
+                    let ord = a_char.cmp(&b_char);
+                    if ord != Ordering::Equal {
+                        break ord;
+                    }
+                }
+            }
+        }
+    };
+
+    if a_info.sign == Sign::Negative {
+        ordering.reverse()
+    } else {
+        ordering
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn parses_exp() {
+        let n = "1";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Positive
+                },
+                0..1
+            )
+        );
+        let n = "100";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 2,
+                    sign: Sign::Positive
+                },
+                0..3
+            )
+        );
+        let n = "1,000";
+        assert_eq!(
+            NumInfo::parse(
+                n,
+                NumInfoParseSettings {
+                    thousands_separator: Some(','),
+                    ..Default::default()
+                }
+            ),
+            (
+                NumInfo {
+                    exponent: 3,
+                    sign: Sign::Positive
+                },
+                0..5
+            )
+        );
+        let n = "1,000";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Positive
+                },
+                0..1
+            )
+        );
+        let n = "1000.00";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 3,
+                    sign: Sign::Positive
+                },
+                0..7
+            )
+        );
+    }
+    #[test]
+    fn parses_negative_exp() {
+        let n = "0.00005";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: -5,
+                    sign: Sign::Positive
+                },
+                6..7
+            )
+        );
+        let n = "00000.00005";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: -5,
+                    sign: Sign::Positive
+                },
+                10..11
+            )
+        );
+    }
+
+    #[test]
+    fn parses_sign() {
+        let n = "5";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Positive
+                },
+                0..1
+            )
+        );
+        let n = "-5";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Negative
+                },
+                1..2
+            )
+        );
+        let n = "    -5";
+        assert_eq!(
+            NumInfo::parse(n, Default::default()),
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Negative
+                },
+                5..6
+            )
+        );
+    }
+
+    fn test_helper(a: &str, b: &str, expected: Ordering) {
+        let (a_info, a_range) = NumInfo::parse(a, Default::default());
+        let (b_info, b_range) = NumInfo::parse(b, Default::default());
+        let ordering = numeric_str_cmp(
+            (&a[a_range.to_owned()], &a_info),
+            (&b[b_range.to_owned()], &b_info),
+        );
+        assert_eq!(ordering, expected);
+        let ordering = numeric_str_cmp((&b[b_range], &b_info), (&a[a_range], &a_info));
+        assert_eq!(ordering, expected.reverse());
+    }
+    #[test]
+    fn test_single_digit() {
+        test_helper("1", "2", Ordering::Less);
+        test_helper("0", "0", Ordering::Equal);
+    }
+    #[test]
+    fn test_minus() {
+        test_helper("-1", "-2", Ordering::Greater);
+        test_helper("-0", "-0", Ordering::Equal);
+    }
+    #[test]
+    fn test_different_len() {
+        test_helper("-20", "-100", Ordering::Greater);
+        test_helper("10.0", "2.000000", Ordering::Greater);
+    }
+    #[test]
+    fn test_decimal_digits() {
+        test_helper("20.1", "20.2", Ordering::Less);
+        test_helper("20.1", "20.15", Ordering::Less);
+        test_helper("-20.1", "+20.15", Ordering::Less);
+        test_helper("-20.1", "-20", Ordering::Less);
+    }
+    #[test]
+    fn test_trailing_zeroes() {
+        test_helper("20.00000", "20.1", Ordering::Less);
+        test_helper("20.00000", "20.0", Ordering::Equal);
+    }
+    #[test]
+    fn test_invalid_digits() {
+        test_helper("foo", "bar", Ordering::Equal);
+        test_helper("20.1", "a", Ordering::Greater);
+        test_helper("-20.1", "a", Ordering::Less);
+        test_helper("a", "0.15", Ordering::Less);
+    }
+    #[test]
+    fn test_multiple_decimal_pts() {
+        test_helper("10.0.0", "50.0.0", Ordering::Less);
+        test_helper("0.1.", "0.2.0", Ordering::Less);
+        test_helper("1.1.", "0", Ordering::Greater);
+        test_helper("1.1.", "-0", Ordering::Greater);
+    }
+    #[test]
+    fn test_leading_decimal_pts() {
+        test_helper(".0", ".0", Ordering::Equal);
+        test_helper(".1", ".0", Ordering::Greater);
+        test_helper(".02", "0", Ordering::Greater);
+    }
+    #[test]
+    fn test_leading_zeroes() {
+        test_helper("000000.0", ".0", Ordering::Equal);
+        test_helper("0.1", "0000000000000.0", Ordering::Greater);
+        test_helper("-01", "-2", Ordering::Greater);
+    }
+
+    #[test]
+    fn minus_zero() {
+        // This matches GNU sort behavior.
+        test_helper("-0", "0", Ordering::Less);
+        test_helper("-0x", "0", Ordering::Less);
+    }
+    #[test]
+    fn double_minus() {
+        test_helper("--1", "0", Ordering::Equal);
+    }
+    #[test]
+    fn single_minus() {
+        let info = NumInfo::parse("-", Default::default());
+        assert_eq!(
+            info,
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Positive
+                },
+                0..0
+            )
+        );
+    }
+    #[test]
+    fn invalid_with_unit() {
+        let info = NumInfo::parse(
+            "-K",
+            NumInfoParseSettings {
+                accept_si_units: true,
+                ..Default::default()
+            },
+        );
+        assert_eq!(
+            info,
+            (
+                NumInfo {
+                    exponent: 0,
+                    sign: Sign::Positive
+                },
+                0..0
+            )
+        );
+    }
+}

src/uu/sort/src/sort.rs

@@ -15,9 +15,12 @@
 #[macro_use]
 extern crate uucore;
 
+mod numeric_str_cmp;
+
 use clap::{App, Arg};
 use fnv::FnvHasher;
 use itertools::Itertools;
+use numeric_str_cmp::{numeric_str_cmp, NumInfo, NumInfoParseSettings};
 use rand::distributions::Alphanumeric;
 use rand::{thread_rng, Rng};
 use rayon::prelude::*;
@@ -162,27 +165,71 @@ impl From<&GlobalSettings> for KeySettings {
 }
 
 /// Represents the string selected by a FieldSelector.
-#[derive(Debug)]
+enum SelectionRange {
-enum Selection {
     /// If we had to transform this selection, we have to store a new string.
     String(String),
     /// If there was no transformation, we can store an index into the line.
     ByIndex(Range<usize>),
 }
 
+impl SelectionRange {
+    /// Gets the actual string slice represented by this Selection.
+    fn get_str<'a>(&'a self, line: &'a str) -> &'a str {
+        match self {
+            SelectionRange::String(string) => string.as_str(),
+            SelectionRange::ByIndex(range) => &line[range.to_owned()],
+        }
+    }
+
+    fn shorten(&mut self, new_range: Range<usize>) {
+        match self {
+            SelectionRange::String(string) => {
+                string.drain(new_range.end..);
+                string.drain(..new_range.start);
+            }
+            SelectionRange::ByIndex(range) => {
+                range.end = range.start + new_range.end;
+                range.start += new_range.start;
+            }
+        }
+    }
+}
+
+enum NumCache {
+    AsF64(f64),
+    WithInfo(NumInfo),
+    None,
+}
+
+impl NumCache {
+    fn as_f64(&self) -> f64 {
+        match self {
+            NumCache::AsF64(n) => *n,
+            _ => unreachable!(),
+        }
+    }
+    fn as_num_info(&self) -> &NumInfo {
+        match self {
+            NumCache::WithInfo(n) => n,
+            _ => unreachable!(),
+        }
+    }
+}
+
+struct Selection {
+    range: SelectionRange,
+    num_cache: NumCache,
+}
+
 impl Selection {
     /// Gets the actual string slice represented by this Selection.
     fn get_str<'a>(&'a self, line: &'a Line) -> &'a str {
-        match self {
+        self.range.get_str(&line.line)
-            Selection::String(string) => string.as_str(),
-            Selection::ByIndex(range) => &line.line[range.to_owned()],
-        }
     }
 }
 
 type Field = Range<usize>;
 
-#[derive(Debug)]
 struct Line {
     line: String,
     // The common case is not to specify fields. Let's make this fast.
@@ -206,18 +253,38 @@ impl Line {
             .selectors
             .iter()
             .map(|selector| {
-                if let Some(range) = selector.get_selection(&line, fields.as_deref()) {
+                let mut range =
-                    if let Some(transformed) =
+                    if let Some(range) = selector.get_selection(&line, fields.as_deref()) {
-                        transform(&line[range.to_owned()], &selector.settings)
+                        if let Some(transformed) =
-                    {
+                            transform(&line[range.to_owned()], &selector.settings)
-                        Selection::String(transformed)
+                        {
+                            SelectionRange::String(transformed)
+                        } else {
+                            SelectionRange::ByIndex(range.start().to_owned()..range.end() + 1)
+                        }
                     } else {
-                        Selection::ByIndex(range.start().to_owned()..range.end() + 1)
+                        // If there is no match, match the empty string.
-                    }
+                        SelectionRange::ByIndex(0..0)
+                    };
+                let num_cache = if selector.settings.mode == SortMode::Numeric
+                    || selector.settings.mode == SortMode::HumanNumeric
+                {
+                    let (info, num_range) = NumInfo::parse(
+                        range.get_str(&line),
+                        NumInfoParseSettings {
+                            accept_si_units: selector.settings.mode == SortMode::HumanNumeric,
+                            thousands_separator: Some(THOUSANDS_SEP),
+                            decimal_pt: Some(DECIMAL_PT),
+                        },
+                    );
+                    range.shorten(num_range);
+                    NumCache::WithInfo(info)
+                } else if selector.settings.mode == SortMode::GeneralNumeric {
+                    NumCache::AsF64(permissive_f64_parse(get_leading_gen(range.get_str(&line))))
                 } else {
-                    // If there is no match, match the empty string.
+                    NumCache::None
-                    Selection::ByIndex(0..0)
+                };
-                }
+                Selection { range, num_cache }
             })
             .collect();
         Self { line, selections }
@@ -284,20 +351,18 @@ fn tokenize_default(line: &str) -> Vec<Field> {
 
 /// Split between separators. These separators are not included in fields.
 fn tokenize_with_separator(line: &str, separator: char) -> Vec<Field> {
-    let mut tokens = vec![0..0];
+    let mut tokens = vec![];
-    let mut previous_was_separator = false;
+    let separator_indices =
-    for (idx, char) in line.char_indices() {
+        line.char_indices()
-        if previous_was_separator {
+            .filter_map(|(i, c)| if c == separator { Some(i) } else { None });
-            tokens.push(idx..0);
+    let mut start = 0;
-        }
+    for sep_idx in separator_indices {
-        if char == separator {
+        tokens.push(start..sep_idx);
-            tokens.last_mut().unwrap().end = idx;
+        start = sep_idx + 1;
-            previous_was_separator = true;
+    }
-        } else {
+    if start < line.len() {
-            previous_was_separator = false;
+        tokens.push(start..line.len());
-        }
     }
-    tokens.last_mut().unwrap().end = line.len();
     tokens
 }
 
@@ -918,26 +983,37 @@ fn exec_check_file(unwrapped_lines: &[Line], settings: &GlobalSettings) -> i32 {
 }
 
 fn sort_by(lines: &mut Vec<Line>, settings: &GlobalSettings) {
-    lines.par_sort_by(|a, b| compare_by(a, b, &settings))
+    if settings.stable || settings.unique {
+        lines.par_sort_by(|a, b| compare_by(a, b, &settings))
+    } else {
+        lines.par_sort_unstable_by(|a, b| compare_by(a, b, &settings))
+    }
 }
 
 fn compare_by(a: &Line, b: &Line, global_settings: &GlobalSettings) -> Ordering {
     for (idx, selector) in global_settings.selectors.iter().enumerate() {
-        let a = a.selections[idx].get_str(a);
+        let a_selection = &a.selections[idx];
-        let b = b.selections[idx].get_str(b);
+        let b_selection = &b.selections[idx];
+        let a_str = a_selection.get_str(a);
+        let b_str = b_selection.get_str(b);
         let settings = &selector.settings;
 
         let cmp: Ordering = if settings.random {
-            random_shuffle(a, b, global_settings.salt.clone())
+            random_shuffle(a_str, b_str, global_settings.salt.clone())
         } else {
-            (match settings.mode {
+            match settings.mode {
-                SortMode::Numeric => numeric_compare,
+                SortMode::Numeric | SortMode::HumanNumeric => numeric_str_cmp(
-                SortMode::GeneralNumeric => general_numeric_compare,
+                    (a_str, a_selection.num_cache.as_num_info()),
-                SortMode::HumanNumeric => human_numeric_size_compare,
+                    (b_str, b_selection.num_cache.as_num_info()),
-                SortMode::Month => month_compare,
+                ),
-                SortMode::Version => version_compare,
+                SortMode::GeneralNumeric => general_numeric_compare(
-                SortMode::Default => default_compare,
+                    a_selection.num_cache.as_f64(),
-            })(a, b)
+                    b_selection.num_cache.as_f64(),
+                ),
+                SortMode::Month => month_compare(a_str, b_str),
+                SortMode::Version => version_compare(a_str, b_str),
+                SortMode::Default => default_compare(a_str, b_str),
+            }
         };
         if cmp != Ordering::Equal {
             return if settings.reverse { cmp.reverse() } else { cmp };
@@ -945,7 +1021,6 @@ fn compare_by(a: &Line, b: &Line, global_settings: &GlobalSettings) -> Ordering
     }
 
     // Call "last resort compare" if all selectors returned Equal
-
     let cmp = if global_settings.random || global_settings.stable || global_settings.unique {
         Ordering::Equal
     } else {
@@ -997,34 +1072,6 @@ fn leading_num_common(a: &str) -> &str {
     s
 }
 
-// This function cleans up the initial comparison done by leading_num_common for a numeric compare.
-// GNU sort does its numeric comparison through strnumcmp.  However, we don't have or
-// may not want to use libc.  Instead we emulate the GNU sort numeric compare by ignoring
-// those leading number lines GNU sort would not recognize.  GNU numeric compare would
-// not recognize a positive sign or scientific/E notation so we strip those elements here.
-fn get_leading_num(a: &str) -> &str {
-    let mut s = "";
-
-    let a = leading_num_common(a);
-
-    // GNU numeric sort doesn't recognize '+' or 'e' notation so we strip
-    for (idx, c) in a.char_indices() {
-        if c.eq(&'e') || c.eq(&'E') || a.chars().next().unwrap_or('\0').eq(&POSITIVE) {
-            s = &a[..idx];
-            break;
-        }
-        // If no further processing needed to be done, return the line as-is to be sorted
-        s = &a;
-    }
-
-    // And empty number or non-number lines are to be treated as ‘0’ but only for numeric sort
-    // All '0'-ed lines will be sorted later, but only amongst themselves, during the so-called 'last resort comparison.'
-    if s.is_empty() {
-        s = "0";
-    };
-    s
-}
-
 // This function cleans up the initial comparison done by leading_num_common for a general numeric compare.
 // In contrast to numeric compare, GNU general numeric/FP sort *should* recognize positive signs and
 // scientific notation, so we strip those lines only after the end of the following numeric string.
@@ -1054,17 +1101,6 @@ fn get_leading_gen(a: &str) -> &str {
     result
 }
 
-#[inline(always)]
-fn remove_thousands_sep<'a, S: Into<Cow<'a, str>>>(input: S) -> Cow<'a, str> {
-    let input = input.into();
-    if input.contains(THOUSANDS_SEP) {
-        let output = input.replace(THOUSANDS_SEP, "");
-        Cow::Owned(output)
-    } else {
-        input
-    }
-}
-
 #[inline(always)]
 fn remove_trailing_dec<'a, S: Into<Cow<'a, str>>>(input: S) -> Cow<'a, str> {
     let input = input.into();
@@ -1093,87 +1129,15 @@ fn permissive_f64_parse(a: &str) -> f64 {
     }
 }
 
-fn numeric_compare(a: &str, b: &str) -> Ordering {
-    #![allow(clippy::comparison_chain)]
-
-    let sa = get_leading_num(a);
-    let sb = get_leading_num(b);
-
-    // Avoids a string alloc for every line to remove thousands seperators here
-    // instead of inside the get_leading_num function, which is a HUGE performance benefit
-    let ta = remove_thousands_sep(sa);
-    let tb = remove_thousands_sep(sb);
-
-    let fa = permissive_f64_parse(&ta);
-    let fb = permissive_f64_parse(&tb);
-
-    // f64::cmp isn't implemented (due to NaN issues); implement directly instead
-    if fa > fb {
-        Ordering::Greater
-    } else if fa < fb {
-        Ordering::Less
-    } else {
-        Ordering::Equal
-    }
-}
-
 /// Compares two floats, with errors and non-numerics assumed to be -inf.
 /// Stops coercing at the first non-numeric char.
-fn general_numeric_compare(a: &str, b: &str) -> Ordering {
+/// We explicitly need to convert to f64 in this case.
+fn general_numeric_compare(a: f64, b: f64) -> Ordering {
     #![allow(clippy::comparison_chain)]
-
-    let sa = get_leading_gen(a);
-    let sb = get_leading_gen(b);
-
-    let fa = permissive_f64_parse(&sa);
-    let fb = permissive_f64_parse(&sb);
-
     // f64::cmp isn't implemented (due to NaN issues); implement directly instead
-    if fa > fb {
+    if a > b {
         Ordering::Greater
-    } else if fa < fb {
+    } else if a < b {
-        Ordering::Less
-    } else {
-        Ordering::Equal
-    }
-}
-
-// GNU/BSD does not handle converting numbers to an equal scale
-// properly.  GNU/BSD simply recognize that there is a human scale and sorts
-// those numbers ahead of other number inputs. There are perhaps limits
-// to the type of behavior we should emulate, and this might be such a limit.
-// Properly handling these units seems like a value add to me. And when sorting
-// these types of numbers, we rarely care about pure performance.
-fn human_numeric_convert(a: &str) -> f64 {
-    let num_str = get_leading_num(a);
-    let suffix = a.trim_start_matches(&num_str);
-    let num_part = permissive_f64_parse(&num_str);
-    let suffix: f64 = match suffix.parse().unwrap_or('\0') {
-        // SI Units
-        'K' => 1E3,
-        'M' => 1E6,
-        'G' => 1E9,
-        'T' => 1E12,
-        'P' => 1E15,
-        'E' => 1E18,
-        'Z' => 1E21,
-        'Y' => 1E24,
-        _ => 1f64,
-    };
-    num_part * suffix
-}
-
-/// Compare two strings as if they are human readable sizes.
-/// AKA 1M > 100k
-fn human_numeric_size_compare(a: &str, b: &str) -> Ordering {
-    #![allow(clippy::comparison_chain)]
-    let fa = human_numeric_convert(a);
-    let fb = human_numeric_convert(b);
-
-    // f64::cmp isn't implemented (due to NaN issues); implement directly instead
-    if fa > fb {
-        Ordering::Greater
-    } else if fa < fb {
         Ordering::Less
     } else {
         Ordering::Equal
@@ -1373,30 +1337,6 @@ mod tests {
         assert_eq!(Ordering::Less, default_compare(a, b));
     }
 
-    #[test]
-    fn test_numeric_compare1() {
-        let a = "149:7";
-        let b = "150:5";
-
-        assert_eq!(Ordering::Less, numeric_compare(a, b));
-    }
-
-    #[test]
-    fn test_numeric_compare2() {
-        let a = "-1.02";
-        let b = "1";
-
-        assert_eq!(Ordering::Less, numeric_compare(a, b));
-    }
-
-    #[test]
-    fn test_human_numeric_compare() {
-        let a = "300K";
-        let b = "1M";
-
-        assert_eq!(Ordering::Less, human_numeric_size_compare(a, b));
-    }
-
     #[test]
     fn test_month_compare() {
         let a = "JaN";
@@ -1441,4 +1381,14 @@ mod tests {
             vec![0..0, 1..1, 2..2, 3..9, 10..18,]
         );
     }
+
+    #[test]
+    fn test_tokenize_fields_trailing_custom_separator() {
+        let line = "a";
+        assert_eq!(tokenize(line, Some('a')), vec![0..0]);
+        let line = "aa";
+        assert_eq!(tokenize(line, Some('a')), vec![0..0, 1..1]);
+        let line = "..a..a";
+        assert_eq!(tokenize(line, Some('a')), vec![0..2, 3..5]);
+    }
 }

src/uu/stdbuf/src/stdbuf.rs

@@ -144,7 +144,9 @@ fn check_option(matches: &ArgMatches, name: &str) -> Result<BufferType, ProgramO
         Some(value) => match value {
             "L" => {
                 if name == options::INPUT {
-                    Err(ProgramOptionsError("line buffering stdin is meaningless".to_string()))
+                    Err(ProgramOptionsError(
+                        "line buffering stdin is meaningless".to_string(),
+                    ))
                 } else {
                     Ok(BufferType::Line)
                 }

src/uu/tac/src/tac.rs

@@ -91,10 +91,15 @@ fn tac(filenames: Vec<String>, before: bool, _: bool, separator: &str) -> i32 {
         } else {
             let path = Path::new(filename);
             if path.is_dir() || path.metadata().is_err() {
-                show_error!(
+                if path.is_dir() {
-                    "failed to open '{}' for reading: No such file or directory",
+                    show_error!("dir: read error: Invalid argument");
-                    filename
+                } else {
-                );
+                    show_error!(
+                        "failed to open '{}' for reading: No such file or directory",
+                        filename
+                    );
+                }
+                exit_code = 1;
                 continue;
             }
             match File::open(path) {

src/uu/tail/src/tail.rs

@@ -117,6 +117,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         .arg(
             Arg::with_name(options::SLEEP_INT)
                 .short("s")
+                .takes_value(true)
                 .long(options::SLEEP_INT)
                 .help("Number or seconds to sleep between polling the file when running with -f"),
         )

src/uu/truncate/src/truncate.rs

@@ -192,7 +192,8 @@ fn truncate(
 }
 
 fn parse_size(size: &str) -> (u64, TruncateMode) {
-    let mode = match size.chars().next().unwrap() {
+    let clean_size = size.replace(" ", "");
+    let mode = match clean_size.chars().next().unwrap() {
         '+' => TruncateMode::Extend,
         '-' => TruncateMode::Reduce,
         '<' => TruncateMode::AtMost,
@@ -203,9 +204,9 @@ fn parse_size(size: &str) -> (u64, TruncateMode) {
     };
     let bytes = {
         let mut slice = if mode == TruncateMode::Reference {
-            size
+            &clean_size
         } else {
-            &size[1..]
+            &clean_size[1..]
         };
         if slice.chars().last().unwrap().is_alphabetic() {
             slice = &slice[..slice.len() - 1];
@@ -220,11 +221,11 @@ fn parse_size(size: &str) -> (u64, TruncateMode) {
         Ok(num) => num,
         Err(e) => crash!(1, "'{}' is not a valid number: {}", size, e),
     };
-    if size.chars().last().unwrap().is_alphabetic() {
+    if clean_size.chars().last().unwrap().is_alphabetic() {
-        number *= match size.chars().last().unwrap().to_ascii_uppercase() {
+        number *= match clean_size.chars().last().unwrap().to_ascii_uppercase() {
-            'B' => match size
+            'B' => match clean_size
                 .chars()
-                .nth(size.len() - 2)
+                .nth(clean_size.len() - 2)
                 .unwrap()
                 .to_ascii_uppercase()
             {

src/uu/unexpand/src/unexpand.rs

@@ -150,7 +150,7 @@ fn next_tabstop(tabstops: &[usize], col: usize) -> Option<usize> {
     } else {
         // find next larger tab
         // if there isn't one in the list, tab becomes a single space
-        tabstops.iter().find(|&&t| t > col).map(|t| t-col)
+        tabstops.iter().find(|&&t| t > col).map(|t| t - col)
     }
 }
 

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)+) => ({

tests/by-util/test_cat.rs

@@ -1,4 +1,7 @@
 use crate::common::util::*;
+#[cfg(unix)]
+use std::fs::OpenOptions;
+#[cfg(unix)]
 use std::io::Read;
 
 #[test]
@@ -26,7 +29,7 @@ fn test_no_options() {
 }
 
 #[test]
-#[cfg(unix)]
+#[cfg(any(target_vendor = "apple", target_os = "linux", target_os = "android"))]
 fn test_no_options_big_input() {
     for &n in &[
         0,
@@ -54,7 +57,6 @@ fn test_no_options_big_input() {
 #[test]
 #[cfg(unix)]
 fn test_fifo_symlink() {
-    use std::fs::OpenOptions;
     use std::io::Write;
     use std::thread;
 
@@ -85,6 +87,74 @@ fn test_fifo_symlink() {
     thread.join().unwrap();
 }
 
+#[test]
+#[cfg(unix)]
+fn test_piped_to_regular_file() {
+    use std::fs::read_to_string;
+
+    for &append in &[true, false] {
+        let s = TestScenario::new(util_name!());
+        let file_path = s.fixtures.plus("file.txt");
+
+        {
+            let file = OpenOptions::new()
+                .create_new(true)
+                .write(true)
+                .append(append)
+                .open(&file_path)
+                .unwrap();
+
+            s.ucmd()
+                .set_stdout(file)
+                .pipe_in_fixture("alpha.txt")
+                .succeeds();
+        }
+        let contents = read_to_string(&file_path).unwrap();
+        assert_eq!(contents, "abcde\nfghij\nklmno\npqrst\nuvwxyz\n");
+    }
+}
+
+#[test]
+#[cfg(unix)]
+fn test_piped_to_dev_null() {
+    for &append in &[true, false] {
+        let s = TestScenario::new(util_name!());
+        {
+            let dev_null = OpenOptions::new()
+                .write(true)
+                .append(append)
+                .open("/dev/null")
+                .unwrap();
+
+            s.ucmd()
+                .set_stdout(dev_null)
+                .pipe_in_fixture("alpha.txt")
+                .succeeds();
+        }
+    }
+}
+
+#[test]
+#[cfg(any(target_os = "linux", target_os = "freebsd", target_os = "netbsd"))]
+fn test_piped_to_dev_full() {
+    for &append in &[true, false] {
+        let s = TestScenario::new(util_name!());
+        {
+            let dev_full = OpenOptions::new()
+                .write(true)
+                .append(append)
+                .open("/dev/full")
+                .unwrap();
+
+            s.ucmd()
+                .set_stdout(dev_full)
+                .pipe_in_fixture("alpha.txt")
+                .fails()
+                .stderr_contains(&"No space left on device".to_owned());
+        }
+    }
+}
+
 #[test]
 fn test_directory() {
     let s = TestScenario::new(util_name!());
@@ -330,22 +400,29 @@ fn test_domain_socket() {
     use std::thread;
     use tempdir::TempDir;
     use unix_socket::UnixListener;
+    use std::sync::{Barrier, Arc};
 
     let dir = TempDir::new("unix_socket").expect("failed to create dir");
     let socket_path = dir.path().join("sock");
     let listener = UnixListener::bind(&socket_path).expect("failed to create socket");
 
+    // use a barrier to ensure we don't run cat before the listener is setup
+    let barrier = Arc::new(Barrier::new(2));
+    let barrier2 = Arc::clone(&barrier);
+
     let thread = thread::spawn(move || {
         let mut stream = listener.accept().expect("failed to accept connection").0;
+        barrier2.wait();
         stream
             .write_all(b"a\tb")
             .expect("failed to write test data");
     });
 
-    new_ucmd!()
+    let child = new_ucmd!().args(&[socket_path]).run_no_wait();
-        .args(&[socket_path])
+    barrier.wait();
-        .succeeds()
+    let stdout = &child.wait_with_output().unwrap().stdout.clone();
-        .stdout_only("a\tb");
+    let output = String::from_utf8_lossy(&stdout);
+    assert_eq!("a\tb", output);
 
     thread.join().unwrap();
 }

tests/by-util/test_chmod.rs

@@ -47,7 +47,7 @@ fn run_single_test(test: &TestCase, at: AtPath, mut ucmd: UCommand) {
         ucmd.arg(arg);
     }
     let r = ucmd.run();
-    if !r.success {
+    if !r.succeeded() {
         println!("{}", r.stderr_str());
         panic!("{:?}: failed", ucmd.raw);
     }
@@ -357,7 +357,8 @@ fn test_chmod_symlink_non_existing_file() {
     at.symlink_file(non_existing, test_symlink);
 
     // this cannot succeed since the symbolic link dangles
-    scene.ucmd()
+    scene
+        .ucmd()
         .arg("755")
         .arg("-v")
         .arg(test_symlink)
@@ -367,7 +368,8 @@ fn test_chmod_symlink_non_existing_file() {
         .stderr_contains(expected_stderr);
 
     // this should be the same than with just '-v' but without stderr
-    scene.ucmd()
+    scene
+        .ucmd()
         .arg("755")
         .arg("-v")
         .arg("-f")
@@ -394,7 +396,8 @@ fn test_chmod_symlink_non_existing_file_recursive() {
     );
 
     // this should succeed
-    scene.ucmd()
+    scene
+        .ucmd()
         .arg("-R")
         .arg("755")
         .arg(test_directory)
@@ -408,7 +411,8 @@ fn test_chmod_symlink_non_existing_file_recursive() {
     );
 
     // '-v': this should succeed without stderr
-    scene.ucmd()
+    scene
+        .ucmd()
         .arg("-R")
         .arg("-v")
         .arg("755")
@@ -418,7 +422,8 @@ fn test_chmod_symlink_non_existing_file_recursive() {
         .no_stderr();
 
     // '-vf': this should be the same than with just '-v'
-    scene.ucmd()
+    scene
+        .ucmd()
         .arg("-R")
         .arg("-v")
         .arg("-f")

tests/by-util/test_chown.rs

@@ -4,6 +4,34 @@ use rust_users::get_effective_uid;
 
 extern crate chown;
 
+// Apparently some CI environments have configuration issues, e.g. with 'whoami' and 'id'.
+// If we are running inside the CI and "needle" is in "stderr" skipping this test is
+// considered okay. If we are not inside the CI this calls assert!(result.success).
+//
+// From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)"
+//
+// stderr: "whoami: cannot find name for user ID 1001"
+// TODO: Maybe `adduser --uid 1001 username` can put things right?
+//
+// stderr: "id: cannot find name for group ID 116"
+// stderr: "thread 'main' panicked at 'called `Result::unwrap()` on an `Err`
+//     value: Custom { kind: NotFound, error: "No such id: 1001" }',
+//     /project/src/uucore/src/lib/features/perms.rs:176:44"
+//
+fn skipping_test_is_okay(result: &CmdResult, needle: &str) -> bool {
+    if !result.succeeded() {
+        println!("result.stdout = {}", result.stdout_str());
+        println!("result.stderr = {}", result.stderr_str());
+        if is_ci() && result.stderr_str().contains(needle) {
+            println!("test skipped:");
+            return true;
+        } else {
+            result.success();
+        }
+    }
+    false
+}
+
 #[cfg(test)]
 mod test_passgrp {
     use super::chown::entries::{gid2grp, grp2gid, uid2usr, usr2uid};
@@ -49,116 +77,193 @@ fn test_invalid_option() {
 }
 
 #[test]
-fn test_chown_myself() {
+fn test_chown_only_owner() {
     // test chown username file.txt
+
     let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
     let result = scene.cmd("whoami").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "whoami: 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!("results {}", result.stdout_str());
+    let user_name = String::from(result.stdout_str().trim());
-    let username = result.stdout_str().trim_end();
+    assert!(!user_name.is_empty());
-
-    let (at, mut ucmd) = at_and_ucmd!();
-    let file1 = "test_install_target_dir_file_a1";
 
+    let file1 = "test_chown_file1";
     at.touch(file1);
-    let result = ucmd.arg(username).arg(file1).run();
-    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;
-    }
-    assert!(result.success);
-}
 
-#[test]
+    // since only superuser can change owner, we have to change from ourself to ourself
-fn test_chown_myself_second() {
+    let result = scene
-    // test chown username: file.txt
+        .ucmd()
-    let scene = TestScenario::new(util_name!());
+        .arg(user_name)
-    let result = scene.cmd("whoami").run();
+        .arg("--verbose")
-    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_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_str().trim_end().to_owned() + ":")
         .arg(file1)
         .run();
+    result.stderr_contains(&"retained as");
 
-    println!("result.stdout = {}", result.stdout_str());
+    // try to change to another existing user, e.g. 'root'
-    println!("result.stderr = {}", result.stderr_str());
+    scene
-    assert!(result.success);
+        .ucmd()
+        .arg("root")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
-fn test_chown_myself_group() {
+fn test_chown_only_owner_colon() {
-    // test chown username:group file.txt
+    // test chown username: file.txt
+
     let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
     let result = scene.cmd("whoami").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "whoami: 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!("user name = {}", result.stdout_str());
+    let user_name = String::from(result.stdout_str().trim());
-    let username = result.stdout_str().trim_end();
+    assert!(!user_name.is_empty());
+
+    let file1 = "test_chown_file1";
+    at.touch(file1);
+
+    scene
+        .ucmd()
+        .arg(format!("{}:", user_name))
+        .arg("--verbose")
+        .arg(file1)
+        .succeeds()
+        .stderr_contains(&"retained as");
+
+    scene
+        .ucmd()
+        .arg("root:")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
+}
+
+#[test]
+fn test_chown_only_colon() {
+    // test chown : file.txt
+
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let file1 = "test_chown_file1";
+    at.touch(file1);
+
+    // expected:
+    // $ chown -v : file.txt 2>out_err ; echo $? ; cat out_err
+    // ownership of 'file.txt' retained
+    // 0
+    let result = scene.ucmd().arg(":").arg("--verbose").arg(file1).run();
+    if skipping_test_is_okay(&result, "No such id") {
+        return;
+    }
+    result.stderr_contains(&"retained as"); // TODO: verbose is not printed to stderr in GNU chown
+
+    // test chown : file.txt
+    // expected:
+    // $ chown -v :: file.txt 2>out_err ; echo $? ; cat out_err
+    // 1
+    // chown: invalid group: ‘::’
+    scene
+        .ucmd()
+        .arg("::")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"invalid group: ‘::’");
+}
+
+#[test]
+fn test_chown_failed_stdout() {
+    // test chown root file.txt
+
+    // TODO: implement once output "failed to change" to stdout is fixed
+    // expected:
+    // $ chown -v root file.txt 2>out_err ; echo $? ; cat out_err
+    // failed to change ownership of 'file.txt' from jhs to root
+    // 1
+    // chown: changing ownership of 'file.txt': Operation not permitted
+}
+
+#[test]
+fn test_chown_owner_group() {
+    // test chown username:group file.txt
+
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let result = scene.cmd("whoami").run();
+    if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
+        return;
+    }
+
+    let user_name = String::from(result.stdout_str().trim());
+    assert!(!user_name.is_empty());
+
+    let file1 = "test_chown_file1";
+    at.touch(file1);
 
     let result = scene.cmd("id").arg("-gn").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
-        // 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_str());
+    let group_name = String::from(result.stdout_str().trim());
-    let group = result.stdout_str().trim_end();
+    assert!(!group_name.is_empty());
 
-    let (at, mut ucmd) = at_and_ucmd!();
+    let result = scene
-    let file1 = "test_install_target_dir_file_a1";
+        .ucmd()
-    let perm = username.to_owned() + ":" + group;
+        .arg(format!("{}:{}", user_name, group_name))
-    at.touch(file1);
+        .arg("--verbose")
-    let result = ucmd.arg(perm).arg(file1).run();
+        .arg(file1)
-    println!("result.stdout = {}", result.stdout_str());
+        .run();
-    println!("result.stderr = {}", result.stderr_str());
+    if skipping_test_is_okay(&result, "chown: invalid group:") {
-    if is_ci() && result.stderr_str().contains("chown: invalid group:") {
-        // With some Ubuntu into the CI, we can get this answer
         return;
     }
-    assert!(result.success);
+    result.stderr_contains(&"retained as");
+
+    // TODO: on macos group name is not recognized correctly: "chown: invalid group: 'root:root'
+    #[cfg(any(windows, all(unix, not(target_os = "macos"))))]
+    scene
+        .ucmd()
+        .arg("root:root")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
+// TODO: on macos group name is not recognized correctly: "chown: invalid group: ':groupname'
+#[cfg(any(windows, all(unix, not(target_os = "macos"))))]
 fn test_chown_only_group() {
     // test chown :group file.txt
+
     let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
     let result = scene.cmd("whoami").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "whoami: 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!("results {}", result.stdout_str());
+    let user_name = String::from(result.stdout_str().trim());
+    assert!(!user_name.is_empty());
 
-    let (at, mut ucmd) = at_and_ucmd!();
+    let file1 = "test_chown_file1";
-    let file1 = "test_install_target_dir_file_a1";
-    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_str());
-    println!("result.stderr = {}", result.stderr_str());
 
+    let result = scene
+        .ucmd()
+        .arg(format!(":{}", user_name))
+        .arg("--verbose")
+        .arg(file1)
+        .run();
     if is_ci() && result.stderr_str().contains("Operation not permitted") {
         // With ubuntu with old Rust in the CI, we can get an error
         return;
@@ -167,221 +272,232 @@ fn test_chown_only_group() {
         // With mac into the CI, we can get this answer
         return;
     }
-    assert!(result.success);
+    result.stderr_contains(&"retained as");
+    result.success();
+
+    scene
+        .ucmd()
+        .arg(":root")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
-fn test_chown_only_id() {
+fn test_chown_only_user_id() {
     // test chown 1111 file.txt
-    let result = TestScenario::new("id").ucmd_keepenv().arg("-u").run();
+
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    let scene = TestScenario::new(util_name!());
-        // In the CI, some server are failing to return whoami.
+    let at = &scene.fixtures;
-        // As seems to be a configuration issue, ignoring it
+
+    let result = scene.cmd_keepenv("id").arg("-u").run();
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
         return;
     }
-    println!("result.stdout = {}", result.stdout_str());
+    let user_id = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!user_id.is_empty());
-    let id = String::from(result.stdout_str().trim());
-
-    let (at, mut ucmd) = at_and_ucmd!();
-    let file1 = "test_install_target_dir_file_a1";
 
+    let file1 = "test_chown_file1";
     at.touch(file1);
-    let result = ucmd.arg(id).arg(file1).run();
 
-    println!("result.stdout = {}", result.stdout_str());
+    let result = scene.ucmd().arg(user_id).arg("--verbose").arg(file1).run();
-    println!("result.stderr = {}", result.stderr_str());
+    if skipping_test_is_okay(&result, "invalid user") {
-    if is_ci() && result.stderr_str().contains("chown: invalid user:") {
+        // From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)"
-        // With some Ubuntu into the CI, we can get this answer
+        // stderr: "chown: invalid user: '1001'
         return;
     }
-    assert!(result.success);
+    result.stderr_contains(&"retained as");
+
+    scene
+        .ucmd()
+        .arg("0")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
 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_str().contains("No such user/group") {
+    let scene = TestScenario::new(util_name!());
-        // In the CI, some server are failing to return whoami.
+    let at = &scene.fixtures;
-        // As seems to be a configuration issue, ignoring it
+
+    let result = scene.cmd_keepenv("id").arg("-g").run();
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
         return;
     }
-    println!("result.stdout = {}", result.stdout_str());
+    let group_id = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!group_id.is_empty());
-    let id = String::from(result.stdout_str().trim());
-
-    let (at, mut ucmd) = at_and_ucmd!();
-    let file1 = "test_install_target_dir_file_a1";
 
+    let file1 = "test_chown_file1";
     at.touch(file1);
-    let perm = ":".to_owned() + &id;
 
-    let result = ucmd.arg(perm).arg(file1).run();
+    let result = scene
-
+        .ucmd()
-    println!("result.stdout = {}", result.stdout_str());
+        .arg(format!(":{}", group_id))
-    println!("result.stderr = {}", result.stderr_str());
+        .arg("--verbose")
-    if is_ci() && result.stderr_str().contains("chown: invalid group:") {
+        .arg(file1)
+        .run();
+    if skipping_test_is_okay(&result, "chown: invalid group:") {
         // With mac into the CI, we can get this answer
         return;
     }
-    assert!(result.success);
+    result.stderr_contains(&"retained as");
+
+    // Apparently on CI "macos-latest, x86_64-apple-darwin, feat_os_macos"
+    // the process has the rights to change from runner:staff to runner:wheel
+    #[cfg(any(windows, all(unix, not(target_os = "macos"))))]
+    scene
+        .ucmd()
+        .arg(":0")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
-fn test_chown_both_id() {
+fn test_chown_owner_group_id() {
     // test chown 1111:1111 file.txt
-    let result = TestScenario::new("id").ucmd_keepenv().arg("-u").run();
+
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    let scene = TestScenario::new(util_name!());
-        // In the CI, some server are failing to return whoami.
+    let at = &scene.fixtures;
-        // As seems to be a configuration issue, ignoring it
+
+    let result = scene.cmd_keepenv("id").arg("-u").run();
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
         return;
     }
-    println!("result.stdout = {}", result.stdout_str());
+    let user_id = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!user_id.is_empty());
-    let id_user = String::from(result.stdout_str().trim());
 
-    let result = TestScenario::new("id").ucmd_keepenv().arg("-g").run();
+    let result = scene.cmd_keepenv("id").arg("-g").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "id: cannot find name for group 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_str());
+    let group_id = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!group_id.is_empty());
-    let id_group = String::from(result.stdout_str().trim());
-
-    let (at, mut ucmd) = at_and_ucmd!();
-    let file1 = "test_install_target_dir_file_a1";
 
+    let file1 = "test_chown_file1";
     at.touch(file1);
-    let perm = id_user + &":".to_owned() + &id_group;
 
-    let result = ucmd.arg(perm).arg(file1).run();
+    let result = scene
-    println!("result.stdout = {}", result.stdout_str());
+        .ucmd()
-    println!("result.stderr = {}", result.stderr_str());
+        .arg(format!("{}:{}", user_id, group_id))
-
+        .arg("--verbose")
-    if is_ci() && result.stderr_str().contains("invalid user") {
+        .arg(file1)
-        // In the CI, some server are failing to return id.
+        .run();
-        // As seems to be a configuration issue, ignoring it
+    if skipping_test_is_okay(&result, "invalid user") {
+        // From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)"
+        // stderr: "chown: invalid user: '1001:116'
         return;
     }
+    result.stderr_contains(&"retained as");
 
-    assert!(result.success);
+    scene
+        .ucmd()
+        .arg("0:0")
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
-fn test_chown_both_mix() {
+fn test_chown_owner_group_mix() {
-    // test chown 1111:1111 file.txt
+    // test chown 1111:group file.txt
-    let result = TestScenario::new("id").ucmd_keepenv().arg("-u").run();
-    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_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();
+    let scene = TestScenario::new(util_name!());
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    let at = &scene.fixtures;
-        // In the CI, some server are failing to return whoami.
+
-        // As seems to be a configuration issue, ignoring it
+    let result = scene.cmd_keepenv("id").arg("-u").run();
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
+        return;
+    }
+    let user_id = String::from(result.stdout_str().trim());
+    assert!(!user_id.is_empty());
+
+    let result = scene.cmd_keepenv("id").arg("-gn").run();
+    if skipping_test_is_okay(&result, "id: cannot find name for group ID") {
         return;
     }
-    println!("result.stdout = {}", result.stdout_str());
-    println!("result.stderr = {}", result.stderr_str());
     let group_name = String::from(result.stdout_str().trim());
+    assert!(!group_name.is_empty());
 
-    let (at, mut ucmd) = at_and_ucmd!();
+    let file1 = "test_chown_file1";
-    let file1 = "test_install_target_dir_file_a1";
-
     at.touch(file1);
-    let perm = id_user + &":".to_owned() + &group_name;
 
-    let result = ucmd.arg(perm).arg(file1).run();
+    let result = scene
+        .ucmd()
+        .arg(format!("{}:{}", user_id, group_name))
+        .arg("--verbose")
+        .arg(file1)
+        .run();
+    result.stderr_contains(&"retained as");
 
-    if is_ci() && result.stderr_str().contains("invalid user") {
+    // TODO: on macos group name is not recognized correctly: "chown: invalid group: '0:root'
-        // In the CI, some server are failing to return id.
+    #[cfg(any(windows, all(unix, not(target_os = "macos"))))]
-        // As seems to be a configuration issue, ignoring it
+    scene
-        return;
+        .ucmd()
-    }
+        .arg("0:root")
-    assert!(result.success);
+        .arg("--verbose")
+        .arg(file1)
+        .fails()
+        .stderr_contains(&"failed to change");
 }
 
 #[test]
 fn test_chown_recursive() {
     let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
     let result = scene.cmd("whoami").run();
-    if is_ci() && result.stderr_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "whoami: 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_str());
+    let user_name = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!user_name.is_empty());
-    let username = result.stdout_str().trim_end();
 
-    let (at, mut ucmd) = at_and_ucmd!();
+    at.mkdir_all("a/b/c");
-    at.mkdir("a");
-    at.mkdir("a/b");
-    at.mkdir("a/b/c");
     at.mkdir("z");
     at.touch(&at.plus_as_string("a/a"));
     at.touch(&at.plus_as_string("a/b/b"));
     at.touch(&at.plus_as_string("a/b/c/c"));
     at.touch(&at.plus_as_string("z/y"));
 
-    let result = ucmd
+    let result = scene
+        .ucmd()
         .arg("-R")
         .arg("--verbose")
-        .arg(username)
+        .arg(user_name)
         .arg("a")
         .arg("z")
         .run();
-    println!("result.stdout = {}", result.stdout_str());
+    result.stderr_contains(&"ownership of 'a/a' retained as");
-    println!("result.stderr = {}", result.stderr_str());
+    result.stderr_contains(&"ownership of 'z/y' retained as");
-    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;
-    }
-
-    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_str().contains("No such user/group") {
+    if skipping_test_is_okay(&result, "whoami: 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_str());
+    let user_name = String::from(result.stdout_str().trim());
-    println!("result.stderr = {}", result.stderr_str());
+    assert!(!user_name.is_empty());
-    let username = result.stdout_str().trim_end();
 
-    let result = new_ucmd!()
+    let result = scene
+        .ucmd()
         .arg("--preserve-root")
         .arg("-R")
-        .arg(username)
+        .arg(user_name)
         .arg("/")
         .fails();
-    println!("result.stdout = {}", result.stdout_str());
+    result.stderr_contains(&"chown: it is dangerous to operate recursively");
-    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("chown: it is dangerous to operate recursively"));
 }
 
 #[cfg(target_os = "linux")]
@@ -393,8 +509,34 @@ fn test_big_p() {
             .arg("bin")
             .arg("/proc/self/cwd")
             .fails()
-            .stderr_is(
+            .stderr_contains(
-                "chown: changing ownership of '/proc/self/cwd': Operation not permitted (os error 1)\n",
+                "chown: changing ownership of '/proc/self/cwd': Operation not permitted (os error 1)",
             );
     }
 }
+
+#[test]
+fn test_chown_file_notexisting() {
+    // test chown username not_existing
+
+    let scene = TestScenario::new(util_name!());
+
+    let result = scene.cmd("whoami").run();
+    if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
+        return;
+    }
+    let user_name = String::from(result.stdout_str().trim());
+    assert!(!user_name.is_empty());
+
+    let _result = scene
+        .ucmd()
+        .arg(user_name)
+        .arg("--verbose")
+        .arg("not_existing")
+        .fails();
+
+    // TODO: uncomment once "failed to change ownership of '{}' to {}" added to stdout
+    // result.stderr_contains(&"retained as");
+    // TODO: uncomment once message changed from "cannot dereference" to "cannot access"
+    // result.stderr_contains(&"cannot access 'not_existing': No such file or directory");
+}

tests/by-util/test_chroot.rs

@@ -4,14 +4,11 @@ use crate::common::util::*;
 fn test_missing_operand() {
     let result = new_ucmd!().run();
 
-    assert_eq!(
+    assert!(result
-        true,
+        .stderr_str()
-        result
+        .starts_with("error: The following required arguments were not provided"));
-            .stderr
-            .starts_with("error: The following required arguments were not provided")
-    );
 
-    assert_eq!(true, result.stderr.contains("<newroot>"));
+    assert!(result.stderr_str().contains("<newroot>"));
 }
 
 #[test]
@@ -20,14 +17,11 @@ fn test_enter_chroot_fails() {
 
     at.mkdir("jail");
 
-    let result = ucmd.arg("jail").run();
+    let result = ucmd.arg("jail").fails();
 
-    assert_eq!(
+    assert!(result
-        true,
+        .stderr_str()
-        result.stderr.starts_with(
+        .starts_with("chroot: error: cannot chroot to jail: Operation not permitted (os error 1)"));
-            "chroot: error: cannot chroot to jail: Operation not permitted (os error 1)"
-        )
-    )
 }
 
 #[test]
@@ -47,19 +41,18 @@ fn test_invalid_user_spec() {
 
     at.mkdir("a");
 
-    let result = ucmd.arg("a").arg("--userspec=ARABA:").run();
+    let result = ucmd.arg("a").arg("--userspec=ARABA:").fails();
 
-    assert_eq!(
+    assert!(result
-        true,
+        .stderr_str()
-        result.stderr.starts_with("chroot: error: invalid userspec")
+        .starts_with("chroot: error: invalid userspec"));
-    );
 }
 
 #[test]
 fn test_preference_of_userspec() {
     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;
@@ -73,7 +66,7 @@ fn test_preference_of_userspec() {
     println!("result.stdout = {}", result.stdout_str());
     println!("result.stderr = {}", result.stderr_str());
 
-    if is_ci() && result.stderr.contains("cannot find name for user ID") {
+    if is_ci() && result.stderr_str().contains("cannot find name for user ID") {
         // In the CI, some server are failing to return id.
         // As seems to be a configuration issue, ignoring it
         return;

tests/by-util/test_cksum.rs

@@ -31,7 +31,10 @@ fn test_empty() {
 
     at.touch("a");
 
-    ucmd.arg("a").succeeds().stdout.ends_with("0 a");
+    ucmd.arg("a")
+        .succeeds()
+        .no_stderr()
+        .normalized_newlines_stdout_is("4294967295 0 a\n");
 }
 
 #[test]
@@ -41,36 +44,37 @@ fn test_arg_overrides_stdin() {
 
     at.touch("a");
 
-    let result = ucmd
+    ucmd.arg("a")
-        .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();
+        .succeeds()
-
+        .no_stderr()
-    println!("{}, {}", result.stdout, result.stderr);
+        .normalized_newlines_stdout_is("4294967295 0 a\n");
-
-    assert!(result.stdout.ends_with("0 a\n"))
 }
 
 #[test]
 fn test_invalid_file() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let ts = TestScenario::new(util_name!());
+    let at = ts.fixtures.clone();
 
-    let ls = TestScenario::new("ls");
+    let folder_name = "asdf";
-    let files = ls.cmd("ls").arg("-l").run();
-    println!("{:?}", files.stdout);
-    println!("{:?}", files.stderr);
 
-    let folder_name = "asdf".to_string();
+    // First check when file doesn't exist
+    ts.ucmd()
+        .arg(folder_name)
+        .fails()
+        .no_stdout()
+        .stderr_contains("cksum: error: 'asdf' No such file or directory");
 
-    let result = ucmd.arg(&folder_name).run();
+    // Then check when the file is of an invalid type
-
+    at.mkdir(folder_name);
-    println!("stdout: {:?}", result.stdout);
+    ts.ucmd()
-    println!("stderr: {:?}", result.stderr);
+        .arg(folder_name)
-    assert!(result.stderr.contains("cksum: error: 'asdf'"));
+        .fails()
-    assert!(!result.success);
+        .no_stdout()
+        .stderr_contains("cksum: error: 'asdf' Is a directory");
 }
 
 // Make sure crc is correct for files larger than 32 bytes
@@ -79,14 +83,13 @@ fn test_invalid_file() {
 fn test_crc_for_bigger_than_32_bytes() {
     let (_, mut ucmd) = at_and_ucmd!();
 
-    let result = ucmd.arg("chars.txt").run();
+    let result = ucmd.arg("chars.txt").succeeds();
 
-    let mut stdout_splitted = result.stdout.split(" ");
+    let mut stdout_splitted = result.stdout_str().split(" ");
 
     let cksum: i64 = stdout_splitted.next().unwrap().parse().unwrap();
     let bytes_cnt: i64 = stdout_splitted.next().unwrap().parse().unwrap();
 
-    assert!(result.success);
     assert_eq!(cksum, 586047089);
     assert_eq!(bytes_cnt, 16);
 }
@@ -95,14 +98,13 @@ fn test_crc_for_bigger_than_32_bytes() {
 fn test_stdin_larger_than_128_bytes() {
     let (_, mut ucmd) = at_and_ucmd!();
 
-    let result = ucmd.arg("larger_than_2056_bytes.txt").run();
+    let result = ucmd.arg("larger_than_2056_bytes.txt").succeeds();
 
-    let mut stdout_splitted = result.stdout.split(" ");
+    let mut stdout_splitted = result.stdout_str().split(" ");
 
     let cksum: i64 = stdout_splitted.next().unwrap().parse().unwrap();
     let bytes_cnt: i64 = stdout_splitted.next().unwrap().parse().unwrap();
 
-    assert!(result.success);
     assert_eq!(cksum, 945881979);
     assert_eq!(bytes_cnt, 2058);
 }

tests/by-util/test_cp.rs

@@ -42,13 +42,9 @@ static TEST_MOUNT_OTHER_FILESYSTEM_FILE: &str = "mount/DO_NOT_copy_me.txt";
 fn test_cp_cp() {
     let (at, mut ucmd) = at_and_ucmd!();
     // Invoke our binary to make the copy.
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_HELLO_WORLD_DEST)
-        .run();
+        .succeeds();
-
-    // Check that the exit code represents a successful copy.
-    assert!(result.success);
 
     // Check the content of the destination file that was copied.
     assert_eq!(at.read(TEST_HELLO_WORLD_DEST), "Hello, World!\n");
@@ -57,12 +53,9 @@ fn test_cp_cp() {
 #[test]
 fn test_cp_existing_target() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_EXISTING_FILE)
-        .run();
+        .succeeds();
-
-    assert!(result.success);
 
     // Check the content of the destination file
     assert_eq!(at.read(TEST_EXISTING_FILE), "Hello, World!\n");
@@ -74,52 +67,41 @@ fn test_cp_existing_target() {
 #[test]
 fn test_cp_duplicate_files() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_COPY_TO_FOLDER)
-        .run();
+        .succeeds()
-
+        .stderr_contains("specified more than once");
-    assert!(result.success);
-    assert!(result.stderr.contains("specified more than once"));
     assert_eq!(at.read(TEST_COPY_TO_FOLDER_FILE), "Hello, World!\n");
 }
 
 #[test]
 fn test_cp_multiple_files_target_is_file() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!()
-    let result = ucmd
         .arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_EXISTING_FILE)
-        .run();
+        .fails()
-
+        .stderr_contains("not a directory");
-    assert!(!result.success);
-    assert!(result.stderr.contains("not a directory"));
 }
 
 #[test]
 fn test_cp_directory_not_recursive() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!()
-    let result = ucmd
         .arg(TEST_COPY_TO_FOLDER)
         .arg(TEST_HELLO_WORLD_DEST)
-        .run();
+        .fails()
-
+        .stderr_contains("omitting directory");
-    assert!(!result.success);
-    assert!(result.stderr.contains("omitting directory"));
 }
 
 #[test]
 fn test_cp_multiple_files() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_HOW_ARE_YOU_SOURCE)
         .arg(TEST_COPY_TO_FOLDER)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert_eq!(at.read(TEST_COPY_TO_FOLDER_FILE), "Hello, World!\n");
     assert_eq!(at.read(TEST_HOW_ARE_YOU_DEST), "How are you?\n");
 }
@@ -129,14 +111,11 @@ fn test_cp_multiple_files() {
 #[cfg(not(macos))]
 fn test_cp_recurse() {
     let (at, mut ucmd) = at_and_ucmd!();
-
+    ucmd.arg("-r")
-    let result = ucmd
-        .arg("-r")
         .arg(TEST_COPY_FROM_FOLDER)
         .arg(TEST_COPY_TO_FOLDER_NEW)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     // Check the content of the destination file that was copied.
     assert_eq!(at.read(TEST_COPY_TO_FOLDER_NEW_FILE), "Hello, World!\n");
 }
@@ -144,14 +123,10 @@ fn test_cp_recurse() {
 #[test]
 fn test_cp_with_dirs_t() {
     let (at, mut ucmd) = at_and_ucmd!();
-
+    ucmd.arg("-t")
-    //using -t option
-    let result_to_dir_t = ucmd
-        .arg("-t")
         .arg(TEST_COPY_TO_FOLDER)
         .arg(TEST_HELLO_WORLD_SOURCE)
-        .run();
+        .succeeds();
-    assert!(result_to_dir_t.success);
     assert_eq!(at.read(TEST_COPY_TO_FOLDER_FILE), "Hello, World!\n");
 }
 
@@ -162,63 +137,52 @@ fn test_cp_with_dirs() {
     let scene = TestScenario::new(util_name!());
     let at = &scene.fixtures;
 
-    //using -t option
+    scene
-    let result_to_dir = scene
         .ucmd()
         .arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_COPY_TO_FOLDER)
-        .run();
+        .succeeds();
-    assert!(result_to_dir.success);
     assert_eq!(at.read(TEST_COPY_TO_FOLDER_FILE), "Hello, World!\n");
 
-    let result_from_dir = scene
+    scene
         .ucmd()
         .arg(TEST_COPY_FROM_FOLDER_FILE)
         .arg(TEST_HELLO_WORLD_DEST)
-        .run();
+        .succeeds();
-    assert!(result_from_dir.success);
     assert_eq!(at.read(TEST_HELLO_WORLD_DEST), "Hello, World!\n");
 }
 
 #[test]
 fn test_cp_arg_target_directory() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("-t")
         .arg(TEST_COPY_TO_FOLDER)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert_eq!(at.read(TEST_COPY_TO_FOLDER_FILE), "Hello, World!\n");
 }
 
 #[test]
 fn test_cp_arg_no_target_directory() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!()
-    let result = ucmd
         .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("-v")
         .arg("-T")
         .arg(TEST_COPY_TO_FOLDER)
-        .run();
+        .fails()
-
+        .stderr_contains("cannot overwrite directory");
-    assert!(!result.success);
-    assert!(result.stderr.contains("cannot overwrite directory"));
 }
 
 #[test]
 fn test_cp_arg_interactive() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!()
-    let result = ucmd
         .arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_HOW_ARE_YOU_SOURCE)
         .arg("-i")
         .pipe_in("N\n")
-        .run();
+        .succeeds()
-
+        .stderr_contains("Not overwriting");
-    assert!(result.success);
-    assert!(result.stderr.contains("Not overwriting"));
 }
 
 #[test]
@@ -227,39 +191,33 @@ fn test_cp_arg_link() {
     use std::os::linux::fs::MetadataExt;
 
     let (at, mut ucmd) = at_and_ucmd!();
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("--link")
         .arg(TEST_HELLO_WORLD_DEST)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert_eq!(at.metadata(TEST_HELLO_WORLD_SOURCE).st_nlink(), 2);
 }
 
 #[test]
 fn test_cp_arg_symlink() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("--symbolic-link")
         .arg(TEST_HELLO_WORLD_DEST)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert!(at.is_symlink(TEST_HELLO_WORLD_DEST));
 }
 
 #[test]
 fn test_cp_arg_no_clobber() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("--no-clobber")
         .arg(TEST_HOW_ARE_YOU_SOURCE)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "How are you?\n");
 }
 
@@ -267,34 +225,31 @@ fn test_cp_arg_no_clobber() {
 fn test_cp_arg_no_clobber_twice() {
     let scene = TestScenario::new(util_name!());
     let at = &scene.fixtures;
+
     at.touch("source.txt");
-    let result = scene
+    scene
         .ucmd()
         .arg("--no-clobber")
         .arg("source.txt")
         .arg("dest.txt")
-        .run();
+        .succeeds()
+        .no_stderr();
 
-    println!("stderr = {:?}", result.stderr_str());
-    println!("stdout = {:?}", result.stdout_str());
-    assert!(result.success);
-    assert!(result.stderr.is_empty());
     assert_eq!(at.read("source.txt"), "");
 
     at.append("source.txt", "some-content");
-    let result = scene
+    scene
         .ucmd()
         .arg("--no-clobber")
         .arg("source.txt")
         .arg("dest.txt")
-        .run();
+        .succeeds()
+        .stdout_does_not_contain("Not overwriting");
 
-    assert!(result.success);
     assert_eq!(at.read("source.txt"), "some-content");
     // Should be empty as the "no-clobber" should keep
     // the previous version
     assert_eq!(at.read("dest.txt"), "");
-    assert!(!result.stderr.contains("Not overwriting"));
 }
 
 #[test]
@@ -311,16 +266,11 @@ fn test_cp_arg_force() {
     permissions.set_readonly(true);
     set_permissions(at.plus(TEST_HELLO_WORLD_DEST), permissions).unwrap();
 
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("--force")
         .arg(TEST_HELLO_WORLD_DEST)
-        .run();
+        .succeeds();
 
-    println!("{:?}", result.stderr_str());
-    println!("{:?}", result.stdout_str());
-
-    assert!(result.success);
     assert_eq!(at.read(TEST_HELLO_WORLD_DEST), "Hello, World!\n");
 }
 
@@ -342,13 +292,11 @@ fn test_cp_arg_remove_destination() {
     permissions.set_readonly(true);
     set_permissions(at.plus(TEST_HELLO_WORLD_DEST), permissions).unwrap();
 
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("--remove-destination")
         .arg(TEST_HELLO_WORLD_DEST)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert_eq!(at.read(TEST_HELLO_WORLD_DEST), "Hello, World!\n");
 }
 
@@ -356,13 +304,11 @@ fn test_cp_arg_remove_destination() {
 fn test_cp_arg_backup() {
     let (at, mut ucmd) = at_and_ucmd!();
 
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("--backup")
         .arg(TEST_HOW_ARE_YOU_SOURCE)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n");
     assert_eq!(
         at.read(&*format!("{}~", TEST_HOW_ARE_YOU_SOURCE)),
@@ -374,14 +320,12 @@ fn test_cp_arg_backup() {
 fn test_cp_arg_suffix() {
     let (at, mut ucmd) = at_and_ucmd!();
 
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("--suffix")
         .arg(".bak")
         .arg(TEST_HOW_ARE_YOU_SOURCE)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n");
     assert_eq!(
         at.read(&*format!("{}.bak", TEST_HOW_ARE_YOU_SOURCE)),
@@ -391,9 +335,8 @@ fn test_cp_arg_suffix() {
 
 #[test]
 fn test_cp_deref_conflicting_options() {
-    let (_at, mut ucmd) = at_and_ucmd!();
+    new_ucmd!()
-
+        .arg("-LP")
-    ucmd.arg("-LP")
         .arg(TEST_COPY_TO_FOLDER)
         .arg(TEST_HELLO_WORLD_SOURCE)
         .fails();
@@ -401,8 +344,7 @@ fn test_cp_deref_conflicting_options() {
 
 #[test]
 fn test_cp_deref() {
-    let scene = TestScenario::new(util_name!());
+    let (at, mut ucmd) = at_and_ucmd!();
-    let at = &scene.fixtures;
 
     #[cfg(not(windows))]
     let _r = fs::symlink(
@@ -415,16 +357,12 @@ fn test_cp_deref() {
         at.subdir.join(TEST_HELLO_WORLD_SOURCE_SYMLINK),
     );
     //using -L option
-    let result = scene
+    ucmd.arg("-L")
-        .ucmd()
-        .arg("-L")
         .arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_HELLO_WORLD_SOURCE_SYMLINK)
         .arg(TEST_COPY_TO_FOLDER)
-        .run();
+        .succeeds();
 
-    // Check that the exit code represents a successful copy.
-    assert!(result.success);
     let path_to_new_symlink = at
         .subdir
         .join(TEST_COPY_TO_FOLDER)
@@ -444,8 +382,7 @@ fn test_cp_deref() {
 }
 #[test]
 fn test_cp_no_deref() {
-    let scene = TestScenario::new(util_name!());
+    let (at, mut ucmd) = at_and_ucmd!();
-    let at = &scene.fixtures;
 
     #[cfg(not(windows))]
     let _r = fs::symlink(
@@ -458,16 +395,12 @@ fn test_cp_no_deref() {
         at.subdir.join(TEST_HELLO_WORLD_SOURCE_SYMLINK),
     );
     //using -P option
-    let result = scene
+    ucmd.arg("-P")
-        .ucmd()
-        .arg("-P")
         .arg(TEST_HELLO_WORLD_SOURCE)
         .arg(TEST_HELLO_WORLD_SOURCE_SYMLINK)
         .arg(TEST_COPY_TO_FOLDER)
-        .run();
+        .succeeds();
 
-    // Check that the exit code represents a successful copy.
-    assert!(result.success);
     let path_to_new_symlink = at
         .subdir
         .join(TEST_COPY_TO_FOLDER)
@@ -490,14 +423,10 @@ fn test_cp_strip_trailing_slashes() {
     let (at, mut ucmd) = at_and_ucmd!();
 
     //using --strip-trailing-slashes option
-    let result = ucmd
+    ucmd.arg("--strip-trailing-slashes")
-        .arg("--strip-trailing-slashes")
         .arg(format!("{}/", TEST_HELLO_WORLD_SOURCE))
         .arg(TEST_HELLO_WORLD_DEST)
-        .run();
+        .succeeds();
-
-    // Check that the exit code represents a successful copy.
-    assert!(result.success);
 
     // Check the content of the destination file that was copied.
     assert_eq!(at.read(TEST_HELLO_WORLD_DEST), "Hello, World!\n");
@@ -507,14 +436,11 @@ fn test_cp_strip_trailing_slashes() {
 fn test_cp_parents() {
     let (at, mut ucmd) = at_and_ucmd!();
 
-    let result = ucmd
+    ucmd.arg("--parents")
-        .arg("--parents")
         .arg(TEST_COPY_FROM_FOLDER_FILE)
         .arg(TEST_COPY_TO_FOLDER)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
-    // Check the content of the destination file that was copied.
     assert_eq!(
         at.read(&format!(
             "{}/{}",
@@ -528,14 +454,12 @@ fn test_cp_parents() {
 fn test_cp_parents_multiple_files() {
     let (at, mut ucmd) = at_and_ucmd!();
 
-    let result = ucmd
+    ucmd.arg("--parents")
-        .arg("--parents")
         .arg(TEST_COPY_FROM_FOLDER_FILE)
         .arg(TEST_HOW_ARE_YOU_SOURCE)
         .arg(TEST_COPY_TO_FOLDER)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert_eq!(
         at.read(&format!(
             "{}/{}",
@@ -554,20 +478,12 @@ fn test_cp_parents_multiple_files() {
 
 #[test]
 fn test_cp_parents_dest_not_directory() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!()
-
-    let result = ucmd
         .arg("--parents")
         .arg(TEST_COPY_FROM_FOLDER_FILE)
         .arg(TEST_HELLO_WORLD_DEST)
-        .run();
+        .fails()
-    println!("{:?}", result);
+        .stderr_contains("with --parents, the destination must be a directory");
-
-    // Check that we did not succeed in copying.
-    assert!(!result.success);
-    assert!(result
-        .stderr
-        .contains("with --parents, the destination must be a directory"));
 }
 
 #[test]
@@ -594,18 +510,14 @@ fn test_cp_deref_folder_to_folder() {
     assert!(env::set_current_dir(&cwd).is_ok());
 
     //using -P -R option
-    let result = scene
+    scene
         .ucmd()
         .arg("-L")
         .arg("-R")
         .arg("-v")
         .arg(TEST_COPY_FROM_FOLDER)
         .arg(TEST_COPY_TO_FOLDER_NEW)
-        .run();
+        .succeeds();
-    println!("cp output {}", result.stdout_str());
-
-    // Check that the exit code represents a successful copy.
-    assert!(result.success);
 
     #[cfg(not(windows))]
     {
@@ -698,18 +610,14 @@ fn test_cp_no_deref_folder_to_folder() {
     assert!(env::set_current_dir(&cwd).is_ok());
 
     //using -P -R option
-    let result = scene
+    scene
         .ucmd()
         .arg("-P")
         .arg("-R")
         .arg("-v")
         .arg(TEST_COPY_FROM_FOLDER)
         .arg(TEST_COPY_TO_FOLDER_NEW)
-        .run();
+        .succeeds();
-    println!("cp output {}", result.stdout_str());
-
-    // Check that the exit code represents a successful copy.
-    assert!(result.success);
 
     #[cfg(not(windows))]
     {
@@ -791,13 +699,11 @@ fn test_cp_archive() {
         previous,
     )
     .unwrap();
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("--archive")
         .arg(TEST_HOW_ARE_YOU_SOURCE)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n");
 
     let metadata = std_fs::metadata(at.subdir.join(TEST_HELLO_WORLD_SOURCE)).unwrap();
@@ -807,11 +713,10 @@ fn test_cp_archive() {
     let creation2 = metadata2.modified().unwrap();
 
     let scene2 = TestScenario::new("ls");
-    let result = scene2.cmd("ls").arg("-al").arg(at.subdir).run();
+    let result = scene2.cmd("ls").arg("-al").arg(at.subdir).succeeds();
 
     println!("ls dest {}", result.stdout_str());
     assert_eq!(creation, creation2);
-    assert!(result.success);
 }
 
 #[test]
@@ -850,11 +755,10 @@ fn test_cp_archive_recursive() {
     // Back to the initial cwd (breaks the other tests)
     assert!(env::set_current_dir(&cwd).is_ok());
 
-    let resultg = ucmd
+    ucmd.arg("--archive")
-        .arg("--archive")
         .arg(TEST_COPY_TO_FOLDER)
         .arg(TEST_COPY_TO_FOLDER_NEW)
-        .run();
+        .fails(); // fails for now
 
     let scene2 = TestScenario::new("ls");
     let result = scene2
@@ -865,7 +769,6 @@ fn test_cp_archive_recursive() {
 
     println!("ls dest {}", result.stdout_str());
 
-    let scene2 = TestScenario::new("ls");
     let result = scene2
         .cmd("ls")
         .arg("-al")
@@ -910,9 +813,6 @@ fn test_cp_archive_recursive() {
             .join("2.link")
             .to_string_lossy()
     ));
-
-    // fails for now
-    assert!(resultg.success);
 }
 
 #[test]
@@ -928,13 +828,11 @@ fn test_cp_preserve_timestamps() {
         previous,
     )
     .unwrap();
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("--preserve=timestamps")
         .arg(TEST_HOW_ARE_YOU_SOURCE)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n");
 
     let metadata = std_fs::metadata(at.subdir.join(TEST_HELLO_WORLD_SOURCE)).unwrap();
@@ -948,7 +846,6 @@ fn test_cp_preserve_timestamps() {
 
     println!("ls dest {}", result.stdout_str());
     assert_eq!(creation, creation2);
-    assert!(result.success);
 }
 
 #[test]
@@ -966,13 +863,11 @@ fn test_cp_dont_preserve_timestamps() {
     .unwrap();
     sleep(Duration::from_secs(3));
 
-    let result = ucmd
+    ucmd.arg(TEST_HELLO_WORLD_SOURCE)
-        .arg(TEST_HELLO_WORLD_SOURCE)
         .arg("--no-preserve=timestamps")
         .arg(TEST_HOW_ARE_YOU_SOURCE)
-        .run();
+        .succeeds();
 
-    assert!(result.success);
     assert_eq!(at.read(TEST_HOW_ARE_YOU_SOURCE), "Hello, World!\n");
 
     let metadata = std_fs::metadata(at.subdir.join(TEST_HELLO_WORLD_SOURCE)).unwrap();
@@ -992,7 +887,6 @@ fn test_cp_dont_preserve_timestamps() {
     // Some margins with time check
     assert!(res.as_secs() > 3595);
     assert!(res.as_secs() < 3605);
-    assert!(result.success);
 }
 
 #[test]
@@ -1017,7 +911,7 @@ fn test_cp_one_file_system() {
     let scene = TestScenario::new(util_name!());
 
     // Test must be run as root (or with `sudo -E`)
-    if scene.cmd("whoami").run().stdout != "root\n" {
+    if scene.cmd("whoami").run().stdout_str() != "root\n" {
         return;
     }
 
@@ -1029,7 +923,7 @@ fn test_cp_one_file_system() {
     at_src.mkdir(TEST_MOUNT_MOUNTPOINT);
     let mountpoint_path = &at_src.plus_as_string(TEST_MOUNT_MOUNTPOINT);
 
-    let _r = scene
+    scene
         .cmd("mount")
         .arg("-t")
         .arg("tmpfs")
@@ -1037,24 +931,21 @@ fn test_cp_one_file_system() {
         .arg("size=640k") // ought to be enough
         .arg("tmpfs")
         .arg(mountpoint_path)
-        .run();
+        .succeeds();
-    assert!(_r.code == Some(0), "{}", _r.stderr);
 
     at_src.touch(TEST_MOUNT_OTHER_FILESYSTEM_FILE);
 
     // Begin testing -x flag
-    let result = scene
+    scene
         .ucmd()
         .arg("-rx")
         .arg(TEST_MOUNT_COPY_FROM_FOLDER)
         .arg(TEST_COPY_TO_FOLDER_NEW)
-        .run();
+        .succeeds();
 
     // Ditch the mount before the asserts
-    let _r = scene.cmd("umount").arg(mountpoint_path).run();
+    scene.cmd("umount").arg(mountpoint_path).succeeds();
-    assert!(_r.code == Some(0), "{}", _r.stderr);
 
-    assert!(result.success);
     assert!(!at_dst.file_exists(TEST_MOUNT_OTHER_FILESYSTEM_FILE));
     // Check if the other files were copied from the source folder hirerarchy
     for entry in WalkDir::new(at_src.as_string()) {
@@ -1074,3 +965,59 @@ fn test_cp_one_file_system() {
         }
     }
 }
+
+#[test]
+#[cfg(target_os = "linux")]
+fn test_cp_reflink_always() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let result = ucmd
+        .arg("--reflink=always")
+        .arg(TEST_HELLO_WORLD_SOURCE)
+        .arg(TEST_EXISTING_FILE)
+        .run();
+
+    if result.success {
+        // Check the content of the destination file
+        assert_eq!(at.read(TEST_EXISTING_FILE), "Hello, World!\n");
+    } else {
+        // Older Linux versions do not support cloning.
+    }
+}
+
+#[test]
+#[cfg(target_os = "linux")]
+fn test_cp_reflink_auto() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    ucmd.arg("--reflink=auto")
+        .arg(TEST_HELLO_WORLD_SOURCE)
+        .arg(TEST_EXISTING_FILE)
+        .succeeds();
+
+    // Check the content of the destination file
+    assert_eq!(at.read(TEST_EXISTING_FILE), "Hello, World!\n");
+}
+
+#[test]
+#[cfg(target_os = "linux")]
+fn test_cp_reflink_never() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    ucmd.arg("--reflink=never")
+        .arg(TEST_HELLO_WORLD_SOURCE)
+        .arg(TEST_EXISTING_FILE)
+        .succeeds();
+
+    // Check the content of the destination file
+    assert_eq!(at.read(TEST_EXISTING_FILE), "Hello, World!\n");
+}
+
+#[test]
+#[cfg(target_os = "linux")]
+fn test_cp_reflink_bad() {
+    let (_, mut ucmd) = at_and_ucmd!();
+    let result = ucmd
+        .arg("--reflink=bad")
+        .arg(TEST_HELLO_WORLD_SOURCE)
+        .arg(TEST_EXISTING_FILE)
+        .fails()
+        .stderr_contains("invalid argument");
+}

tests/by-util/test_date.rs

@@ -7,174 +7,147 @@ use rust_users::*;
 
 #[test]
 fn test_date_email() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("--rfc-email").succeeds();
-    let result = ucmd.arg("--rfc-email").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_date_email2() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-R").succeeds();
-    let result = ucmd.arg("-R").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_date_rfc_3339() {
     let scene = TestScenario::new(util_name!());
 
-    let mut result = scene.ucmd().arg("--rfc-3339=ns").succeeds();
+    let rfc_regexp = concat!(
+        r#"(\d+)-(0[1-9]|1[012])-(0[1-9]|[12]\d|3[01])\s([01]\d|2[0-3]):"#,
+        r#"([0-5]\d):([0-5]\d|60)(\.\d+)?(([Zz])|([\+|\-]([01]\d|2[0-3])))"#
+    );
+    let re = Regex::new(rfc_regexp).unwrap();
 
     // 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])))";
+    scene
-    let re = Regex::new(rfc_regexp).unwrap();
+        .ucmd()
-    assert!(re.is_match(&result.stdout_str().trim()));
+        .arg("--rfc-3339=ns")
+        .succeeds()
+        .stdout_matches(&re);
 
-    result = scene.ucmd().arg("--rfc-3339=seconds").succeeds();
+    scene
-
+        .ucmd()
-    // Check that the output matches the regexp
+        .arg("--rfc-3339=seconds")
-    let re = Regex::new(rfc_regexp).unwrap();
+        .succeeds()
-    assert!(re.is_match(&result.stdout_str().trim()));
+        .stdout_matches(&re);
 }
 
 #[test]
 fn test_date_rfc_8601() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("--iso-8601=ns").succeeds();
-    let result = ucmd.arg("--iso-8601=ns").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_date_rfc_8601_second() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("--iso-8601=second").succeeds();
-    let result = ucmd.arg("--iso-8601=second").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_date_utc() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("--utc").succeeds();
-    let result = ucmd.arg("--utc").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_date_universal() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("--universal").succeeds();
-    let result = ucmd.arg("--universal").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_date_format_y() {
     let scene = TestScenario::new(util_name!());
 
-    let mut result = scene.ucmd().arg("+%Y").succeeds();
-
-    assert!(result.success);
     let mut re = Regex::new(r"^\d{4}$").unwrap();
-    assert!(re.is_match(&result.stdout_str().trim()));
+    scene.ucmd().arg("+%Y").succeeds().stdout_matches(&re);
 
-    result = scene.ucmd().arg("+%y").succeeds();
-
-    assert!(result.success);
     re = Regex::new(r"^\d{2}$").unwrap();
-    assert!(re.is_match(&result.stdout_str().trim()));
+    scene.ucmd().arg("+%y").succeeds().stdout_matches(&re);
 }
 
 #[test]
 fn test_date_format_m() {
     let scene = TestScenario::new(util_name!());
 
-    let mut result = scene.ucmd().arg("+%b").succeeds();
-
-    assert!(result.success);
     let mut re = Regex::new(r"\S+").unwrap();
-    assert!(re.is_match(&result.stdout_str().trim()));
+    scene.ucmd().arg("+%b").succeeds().stdout_matches(&re);
 
-    result = scene.ucmd().arg("+%m").succeeds();
-
-    assert!(result.success);
     re = Regex::new(r"^\d{2}$").unwrap();
-    assert!(re.is_match(&result.stdout_str().trim()));
+    scene.ucmd().arg("+%m").succeeds().stdout_matches(&re);
 }
 
 #[test]
 fn test_date_format_day() {
     let scene = TestScenario::new(util_name!());
 
-    let mut result = scene.ucmd().arg("+%a").succeeds();
-
-    assert!(result.success);
     let mut re = Regex::new(r"\S+").unwrap();
-    assert!(re.is_match(&result.stdout_str().trim()));
+    scene.ucmd().arg("+%a").succeeds().stdout_matches(&re);
-
-    result = scene.ucmd().arg("+%A").succeeds();
-
-    assert!(result.success);
 
     re = Regex::new(r"\S+").unwrap();
-    assert!(re.is_match(&result.stdout_str().trim()));
+    scene.ucmd().arg("+%A").succeeds().stdout_matches(&re);
 
-    result = scene.ucmd().arg("+%u").succeeds();
-
-    assert!(result.success);
     re = Regex::new(r"^\d{1}$").unwrap();
-    assert!(re.is_match(&result.stdout_str().trim()));
+    scene.ucmd().arg("+%u").succeeds().stdout_matches(&re);
 }
 
 #[test]
 fn test_date_format_full_day() {
-    let scene = TestScenario::new(util_name!());
-
-    let result = scene.ucmd().arg("+'%a %Y-%m-%d'").succeeds();
-
-    assert!(result.success);
     let re = Regex::new(r"\S+ \d{4}-\d{2}-\d{2}").unwrap();
-    assert!(re.is_match(&result.stdout_str().trim()));
+    new_ucmd!()
+        .arg("+'%a %Y-%m-%d'")
+        .succeeds()
+        .stdout_matches(&re);
 }
 
 #[test]
 #[cfg(all(unix, not(target_os = "macos")))]
 fn test_date_set_valid() {
     if get_effective_uid() == 0 {
-        let (_, mut ucmd) = at_and_ucmd!();
+        new_ucmd!()
-        let result = ucmd
             .arg("--set")
             .arg("2020-03-12 13:30:00+08:00")
-            .succeeds();
+            .succeeds()
-        result.no_stdout().no_stderr();
+            .no_stdout()
+            .no_stderr();
     }
 }
 
 #[test]
 #[cfg(any(windows, all(unix, not(target_os = "macos"))))]
 fn test_date_set_invalid() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let result = new_ucmd!().arg("--set").arg("123abcd").fails();
-    let result = ucmd.arg("--set").arg("123abcd").fails();
+    result.no_stdout();
-    let result = result.no_stdout();
+    assert!(result.stderr_str().starts_with("date: invalid date "));
-    assert!(result.stderr.starts_with("date: invalid date "));
 }
 
 #[test]
 #[cfg(all(unix, not(target_os = "macos")))]
 fn test_date_set_permissions_error() {
     if !(get_effective_uid() == 0 || uucore::os::is_wsl_1()) {
-        let (_, mut ucmd) = at_and_ucmd!();
+        let result = new_ucmd!()
-        let result = ucmd.arg("--set").arg("2020-03-11 21:45:00+08:00").fails();
+            .arg("--set")
-        let result = result.no_stdout();
+            .arg("2020-03-11 21:45:00+08:00")
-        assert!(result.stderr.starts_with("date: cannot set date: "));
+            .fails();
+        result.no_stdout();
+        assert!(result.stderr_str().starts_with("date: cannot set date: "));
     }
 }
 
 #[test]
 #[cfg(target_os = "macos")]
 fn test_date_set_mac_unavailable() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let result = new_ucmd!()
-    let result = ucmd.arg("--set").arg("2020-03-11 21:45:00+08:00").fails();
+        .arg("--set")
-    let result = result.no_stdout();
+        .arg("2020-03-11 21:45:00+08:00")
+        .fails();
+    result.no_stdout();
     assert!(result
-        .stderr
+        .stderr_str()
         .starts_with("date: setting the date is not supported by macOS"));
 }
 
@@ -183,13 +156,12 @@ fn test_date_set_mac_unavailable() {
 /// TODO: expected to fail currently; change to succeeds() when required.
 fn test_date_set_valid_2() {
     if get_effective_uid() == 0 {
-        let (_, mut ucmd) = at_and_ucmd!();
+        let result = new_ucmd!()
-        let result = ucmd
             .arg("--set")
             .arg("Sat 20 Mar 2021 14:53:01 AWST")
             .fails();
-        let result = result.no_stdout();
+        result.no_stdout();
-        assert!(result.stderr.starts_with("date: invalid date "));
+        assert!(result.stderr_str().starts_with("date: invalid date "));
     }
 }
 
@@ -198,13 +170,12 @@ fn test_date_set_valid_2() {
 /// TODO: expected to fail currently; change to succeeds() when required.
 fn test_date_set_valid_3() {
     if get_effective_uid() == 0 {
-        let (_, mut ucmd) = at_and_ucmd!();
+        let result = new_ucmd!()
-        let result = ucmd
             .arg("--set")
             .arg("Sat 20 Mar 2021 14:53:01") // Local timezone
             .fails();
-        let result = result.no_stdout();
+        result.no_stdout();
-        assert!(result.stderr.starts_with("date: invalid date "));
+        assert!(result.stderr_str().starts_with("date: invalid date "));
     }
 }
 
@@ -213,12 +184,11 @@ fn test_date_set_valid_3() {
 /// TODO: expected to fail currently; change to succeeds() when required.
 fn test_date_set_valid_4() {
     if get_effective_uid() == 0 {
-        let (_, mut ucmd) = at_and_ucmd!();
+        let result = new_ucmd!()
-        let result = ucmd
             .arg("--set")
             .arg("2020-03-11 21:45:00") // Local timezone
             .fails();
-        let result = result.no_stdout();
+        result.no_stdout();
-        assert!(result.stderr.starts_with("date: invalid date "));
+        assert!(result.stderr_str().starts_with("date: invalid date "));
     }
 }

tests/by-util/test_df.rs

@@ -2,30 +2,22 @@ use crate::common::util::*;
 
 #[test]
 fn test_df_compatible_no_size_arg() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-a").succeeds();
-    let result = ucmd.arg("-a").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_df_compatible() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-ah").succeeds();
-    let result = ucmd.arg("-ah").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_df_compatible_type() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-aT").succeeds();
-    let result = ucmd.arg("-aT").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_df_compatible_si() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-aH").succeeds();
-    let result = ucmd.arg("-aH").run();
-    assert!(result.success);
 }
 
 // ToDO: more tests...

tests/by-util/test_du.rs

@@ -7,12 +7,10 @@ const SUB_LINK: &str = "subdir/links/sublink.txt";
 
 #[test]
 fn test_du_basics() {
-    new_ucmd!()
+    new_ucmd!().succeeds().no_stderr();
-        .succeeds()
-        .no_stderr();
 }
 #[cfg(target_vendor = "apple")]
-fn _du_basics(s: String) {
+fn _du_basics(s: &str) {
     let answer = "32\t./subdir
 8\t./subdir/deeper
 24\t./subdir/links
@@ -32,11 +30,18 @@ fn _du_basics(s: &str) {
 
 #[test]
 fn test_du_basics_subdir() {
-    let (_at, mut ucmd) = at_and_ucmd!();
+    let scene = TestScenario::new(util_name!());
 
-    let result = ucmd.arg(SUB_DIR).run();
+    let result = scene.ucmd().arg(SUB_DIR).succeeds();
-    assert!(result.success);
+
-    assert_eq!(result.stderr, "");
+    #[cfg(target_os = "linux")]
+    {
+        let result_reference = scene.cmd("du").arg(SUB_DIR).run();
+        if result_reference.succeeded() {
+            assert_eq!(result.stdout_str(), result_reference.stdout_str());
+            return;
+        }
+    }
     _du_basics_subdir(result.stdout_str());
 }
 
@@ -60,26 +65,29 @@ fn _du_basics_subdir(s: &str) {
 
 #[test]
 fn test_du_basics_bad_name() {
-    let (_at, mut ucmd) = at_and_ucmd!();
+    new_ucmd!()
-
+        .arg("bad_name")
-    let result = ucmd.arg("bad_name").run();
+        .succeeds() // TODO: replace with ".fails()" once `du` is fixed
-    assert_eq!(result.stdout_str(), "");
+        .stderr_only("du: error: bad_name: No such file or directory\n");
-    assert_eq!(
-        result.stderr,
-        "du: error: bad_name: No such file or directory\n"
-    );
 }
 
 #[test]
 fn test_du_soft_link() {
-    let ts = TestScenario::new("du");
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
 
-    let link = ts.ccmd("ln").arg("-s").arg(SUB_FILE).arg(SUB_LINK).run();
+    at.symlink_file(SUB_FILE, SUB_LINK);
-    assert!(link.success);
 
-    let result = ts.ucmd().arg(SUB_DIR_LINKS).run();
+    let result = scene.ucmd().arg(SUB_DIR_LINKS).succeeds();
-    assert!(result.success);
+
-    assert_eq!(result.stderr, "");
+    #[cfg(target_os = "linux")]
+    {
+        let result_reference = scene.cmd("du").arg(SUB_DIR_LINKS).run();
+        if result_reference.succeeded() {
+            assert_eq!(result.stdout_str(), result_reference.stdout_str());
+            return;
+        }
+    }
     _du_soft_link(result.stdout_str());
 }
 
@@ -104,14 +112,23 @@ fn _du_soft_link(s: &str) {
 
 #[test]
 fn test_du_hard_link() {
-    let ts = TestScenario::new("du");
+    let scene = TestScenario::new(util_name!());
 
-    let link = ts.ccmd("ln").arg(SUB_FILE).arg(SUB_LINK).run();
+    let result_ln = scene.cmd("ln").arg(SUB_FILE).arg(SUB_LINK).run();
-    assert!(link.success);
+    if !result_ln.succeeded() {
+        scene.ccmd("ln").arg(SUB_FILE).arg(SUB_LINK).succeeds();
+    }
 
-    let result = ts.ucmd().arg(SUB_DIR_LINKS).run();
+    let result = scene.ucmd().arg(SUB_DIR_LINKS).succeeds();
-    assert!(result.success);
+
-    assert_eq!(result.stderr, "");
+    #[cfg(target_os = "linux")]
+    {
+        let result_reference = scene.cmd("du").arg(SUB_DIR_LINKS).run();
+        if result_reference.succeeded() {
+            assert_eq!(result.stdout_str(), result_reference.stdout_str());
+            return;
+        }
+    }
     // We do not double count hard links as the inodes are identical
     _du_hard_link(result.stdout_str());
 }
@@ -136,11 +153,23 @@ fn _du_hard_link(s: &str) {
 
 #[test]
 fn test_du_d_flag() {
-    let ts = TestScenario::new("du");
+    let scene = TestScenario::new(util_name!());
 
-    let result = ts.ucmd().arg("-d").arg("1").run();
+    let result = scene.ucmd().arg("-d1").succeeds();
-    assert!(result.success);
+
-    assert_eq!(result.stderr, "");
+    #[cfg(target_os = "linux")]
+    {
+        let result_reference = scene.cmd("du").arg("-d1").run();
+        if result_reference.succeeded() {
+            assert_eq!(
+                // TODO: gnu `du` doesn't use trailing "/" here
+                // result.stdout_str(), result_reference.stdout_str()
+                result.stdout_str().trim_end_matches("/\n"),
+                result_reference.stdout_str().trim_end_matches("\n")
+            );
+            return;
+        }
+    }
     _du_d_flag(result.stdout_str());
 }
 
@@ -164,9 +193,7 @@ fn _du_d_flag(s: &str) {
 
 #[test]
 fn test_du_h_flag_empty_file() {
-    let ts = TestScenario::new("du");
+    new_ucmd!()
-
-    ts.ucmd()
         .arg("-h")
         .arg("empty.txt")
         .succeeds()
@@ -176,17 +203,61 @@ fn test_du_h_flag_empty_file() {
 #[cfg(feature = "touch")]
 #[test]
 fn test_du_time() {
-    let ts = TestScenario::new("du");
+    let scene = TestScenario::new(util_name!());
 
-    let touch = ts.ccmd("touch").arg("-a").arg("-m").arg("-t").arg("201505150000").arg("date_test").run();
+    scene
-    assert!(touch.success);
+        .ccmd("touch")
+        .arg("-a")
+        .arg("-m")
+        .arg("-t")
+        .arg("201505150000")
+        .arg("date_test")
+        .succeeds();
 
-    let result = ts.ucmd().arg("--time").arg("date_test").run();
+    scene
-
+        .ucmd()
-    // cleanup by removing test file
+        .arg("--time")
-    ts.cmd("rm").arg("date_test").run();
+        .arg("date_test")
-
+        .succeeds()
-    assert!(result.success);
+        .stdout_only("0\t2015-05-15 00:00\tdate_test\n");
-    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 scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.mkdir_all(SUB_DIR_LINKS);
+
+    scene.ccmd("chmod").arg("-r").arg(SUB_DIR_LINKS).succeeds();
+
+    let result = scene.ucmd().arg(SUB_DIR_LINKS).run(); // TODO: replace with ".fails()" once `du` is fixed
+    result.stderr_contains(
+        "du: cannot read directory ‘subdir/links‘: Permission denied (os error 13)",
+    );
+
+    #[cfg(target_os = "linux")]
+    {
+        let result_reference = scene.cmd("du").arg(SUB_DIR_LINKS).fails();
+        if result_reference
+            .stderr_str()
+            .contains("du: cannot read directory 'subdir/links': Permission denied")
+        {
+            assert_eq!(result.stdout_str(), result_reference.stdout_str());
+            return;
+        }
+    }
+
+    _du_no_permission(result.stdout_str());
+}
+
+#[cfg(target_vendor = "apple")]
+fn _du_no_permission(s: &str) {
+    assert_eq!(s, "0\tsubdir/links\n");
+}
+#[cfg(all(not(target_vendor = "apple"), not(target_os = "windows")))]
+fn _du_no_permission(s: &str) {
+    assert_eq!(s, "4\tsubdir/links\n");
 }

tests/by-util/test_echo.rs

@@ -2,10 +2,7 @@ use crate::common::util::*;
 
 #[test]
 fn test_default() {
-    new_ucmd!()
+    new_ucmd!().arg("hi").succeeds().stdout_only("hi\n");
-        .arg("hi")
-        .succeeds()
-        .stdout_only("hi\n");
 }
 
 #[test]

tests/by-util/test_env.rs

@@ -26,17 +26,18 @@ fn test_env_version() {
 
 #[test]
 fn test_echo() {
-    let result = new_ucmd!()
+    let result = new_ucmd!().arg("echo").arg("FOO-bar").succeeds();
-        .arg("echo")
-        .arg("FOO-bar")
-        .succeeds();
 
     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_move_str();
+    let out = new_ucmd!()
+        .arg("-f")
+        .arg("vars.conf.txt")
+        .run()
+        .stdout_move_str();
 
     assert_eq!(
         out.lines()
@@ -89,7 +90,8 @@ fn test_multiple_name_value_pairs() {
     let out = new_ucmd!().arg("FOO=bar").arg("ABC=xyz").run();
 
     assert_eq!(
-        out.stdout_str().lines()
+        out.stdout_str()
+            .lines()
             .filter(|&line| line == "FOO=bar" || line == "ABC=xyz")
             .count(),
         2
@@ -138,8 +140,11 @@ fn test_unset_variable() {
 
 #[test]
 fn test_fail_null_with_program() {
-    let out = new_ucmd!().arg("--null").arg("cd").fails().stderr;
+    new_ucmd!()
-    assert!(out.contains("cannot specify --null (-0) with command"));
+        .arg("--null")
+        .arg("cd")
+        .fails()
+        .stderr_contains("cannot specify --null (-0) with command");
 }
 
 #[cfg(not(windows))]

tests/by-util/test_fmt.rs

@@ -29,7 +29,7 @@ fn test_fmt_w_too_big() {
         .run();
     //.stdout_is_fixture("call_graph.expected");
     assert_eq!(
-        result.stderr.trim(),
+        result.stderr_str().trim(),
         "fmt: error: invalid width: '2501': Numerical result out of range"
     );
 }

tests/by-util/test_groups.rs

@@ -10,7 +10,7 @@ fn test_groups() {
         // As seems to be a configuration issue, ignoring it
         return;
     }
-    assert!(result.success);
+    result.success();
     assert!(!result.stdout_str().trim().is_empty());
 }
 
@@ -30,16 +30,12 @@ fn test_groups_arg() {
 
     println!("result.stdout = {}", result.stdout_str());
     println!("result.stderr = {}", result.stderr_str());
-    assert!(result.success);
+    result.success();
     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!();
+    new_ucmd!().arg(username).succeeds();
-    let result = ucmd.arg(username).run();
-    println!("result.stdout = {}", result.stdout_str());
-    println!("result.stderr = {}", result.stderr_str());
-    assert!(result.success);
     assert!(!result.stdout_str().is_empty());
 }

tests/by-util/test_head.rs

@@ -156,14 +156,10 @@ fn test_negative_zero_bytes() {
 }
 #[test]
 fn test_no_such_file_or_directory() {
-    let result = new_ucmd!().arg("no_such_file.toml").run();
+    new_ucmd!()
-
+        .arg("no_such_file.toml")
-    assert_eq!(
+        .fails()
-        true,
+        .stderr_contains("cannot open 'no_such_file.toml' for reading: No such file or directory");
-        result
-            .stderr
-            .contains("cannot open 'no_such_file.toml' for reading: No such file or directory")
-    )
 }
 
 // there was a bug not caught by previous tests

tests/by-util/test_hostid.rs

@@ -4,10 +4,6 @@ use self::regex::Regex;
 
 #[test]
 fn test_normal() {
-    let (_, mut ucmd) = at_and_ucmd!();
-    let result = ucmd.run();
-
-    assert!(result.success);
     let re = Regex::new(r"^[0-9a-f]{8}").unwrap();
-    assert!(re.is_match(&result.stdout_str()));
+    new_ucmd!().succeeds().stdout_matches(&re);
 }

tests/by-util/test_hostname.rs

@@ -14,9 +14,7 @@ fn test_hostname() {
 #[cfg(not(target_vendor = "apple"))]
 #[test]
 fn test_hostname_ip() {
-    let result = new_ucmd!().arg("-i").run();
+    let result = new_ucmd!().arg("-i").succeeds();
-    println!("{:#?}", result);
-    assert!(result.success);
     assert!(!result.stdout_str().trim().is_empty());
 }
 
@@ -25,6 +23,8 @@ fn test_hostname_full() {
     let ls_short_res = new_ucmd!().arg("-s").succeeds();
     assert!(!ls_short_res.stdout_str().trim().is_empty());
 
-    new_ucmd!().arg("-f").succeeds()
+    new_ucmd!()
+        .arg("-f")
+        .succeeds()
         .stdout_contains(ls_short_res.stdout_str().trim());
 }

tests/by-util/test_id.rs

@@ -1,11 +1,32 @@
 use crate::common::util::*;
 
+// Apparently some CI environments have configuration issues, e.g. with 'whoami' and 'id'.
+// If we are running inside the CI and "needle" is in "stderr" skipping this test is
+// considered okay. If we are not inside the CI this calls assert!(result.success).
+//
+// From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)"
+// stderr: "whoami: cannot find name for user ID 1001"
+// Maybe: "adduser --uid 1001 username" can put things right?
+// stderr = id: error: Could not find uid 1001: No such id: 1001
+fn skipping_test_is_okay(result: &CmdResult, needle: &str) -> bool {
+    if !result.succeeded() {
+        println!("result.stdout = {}", result.stdout_str());
+        println!("result.stderr = {}", result.stderr_str());
+        if is_ci() && result.stderr_str().contains(needle) {
+            println!("test skipped:");
+            return true;
+        } else {
+            result.success();
+        }
+    }
+    false
+}
+
 fn return_whoami_username() -> String {
     let scene = TestScenario::new("whoami");
     let result = scene.cmd("whoami").run();
-    if is_ci() && result.stderr.contains("cannot find name for user ID") {
+    if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") {
-        // In the CI, some server are failing to return whoami.
+        println!("test skipped:");
-        // As seems to be a configuration issue, ignoring it
         return String::from("");
     }
 
@@ -14,39 +35,41 @@ fn return_whoami_username() -> String {
 
 #[test]
 fn test_id() {
-    let result = new_ucmd!().arg("-u").run();
+    let scene = TestScenario::new(util_name!());
-    if result.stderr.contains("cannot find name for user ID") {
+
-        // In the CI, some server are failing to return whoami.
+    let result = scene.ucmd().arg("-u").succeeds();
-        // As seems to be a configuration issue, ignoring it
+    let uid = result.stdout_str().trim();
+
+    let result = scene.ucmd().run();
+    if skipping_test_is_okay(&result, "Could not find uid") {
         return;
     }
 
-    let uid = result.success().stdout_str().trim();
+    // Verify that the id found by --user/-u exists in the list
-    let result = new_ucmd!().run();
+    result.stdout_contains(uid);
-    if is_ci() && result.stderr.contains("cannot find name for user ID") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
-        return;
-    }
-
-    if !result.stderr_str().contains("Could not find uid") {
-        // Verify that the id found by --user/-u exists in the list
-        result.success().stdout_contains(&uid);
-    }
 }
 
 #[test]
 fn test_id_from_name() {
     let username = return_whoami_username();
-    if username == "" {
+    if username.is_empty() {
-        // Sometimes, the CI is failing here
+        return;
+    }
+
+    let scene = TestScenario::new(util_name!());
+    let result = scene.ucmd().arg(&username).run();
+    if skipping_test_is_okay(&result, "Could not find uid") {
         return;
     }
 
-    let result = new_ucmd!().arg(&username).succeeds();
     let uid = result.stdout_str().trim();
 
-    new_ucmd!().succeeds()
+    let result = scene.ucmd().run();
+    if skipping_test_is_okay(&result, "Could not find uid") {
+        return;
+    }
+
+    result
         // Verify that the id found by --user/-u exists in the list
         .stdout_contains(uid)
         // Verify that the username found by whoami exists in the list
@@ -55,51 +78,42 @@ fn test_id_from_name() {
 
 #[test]
 fn test_id_name_from_id() {
-    let result = new_ucmd!().arg("-u").succeeds();
+    let result = new_ucmd!().arg("-nu").run();
-    let uid = result.stdout_str().trim();
 
-    let result = new_ucmd!().arg("-nu").arg(uid).run();
+    let username_id = result.stdout_str().trim();
-    if is_ci() && result.stderr.contains("No such user/group") {
+
-        // In the CI, some server are failing to return whoami.
+    let username_whoami = return_whoami_username();
-        // As seems to be a configuration issue, ignoring it
+    if username_whoami.is_empty() {
         return;
     }
 
-    let username_id = result
-        .success()
-        .stdout_str()
-        .trim();
-
-    let scene = TestScenario::new("whoami");
-    let result = scene.cmd("whoami").succeeds();
-
-    let username_whoami = result.stdout_str().trim();
-
     assert_eq!(username_id, username_whoami);
 }
 
 #[test]
 fn test_id_group() {
-    let mut result = new_ucmd!().arg("-g").succeeds();
+    let scene = TestScenario::new(util_name!());
+
+    let mut result = scene.ucmd().arg("-g").succeeds();
     let s1 = result.stdout_str().trim();
     assert!(s1.parse::<f64>().is_ok());
 
-    result = new_ucmd!().arg("--group").succeeds();
+    result = scene.ucmd().arg("--group").succeeds();
     let s1 = result.stdout_str().trim();
     assert!(s1.parse::<f64>().is_ok());
 }
 
 #[test]
 fn test_id_groups() {
-    let result = new_ucmd!().arg("-G").succeeds();
+    let scene = TestScenario::new(util_name!());
-    assert!(result.success);
+
+    let result = scene.ucmd().arg("-G").succeeds();
     let groups = result.stdout_str().trim().split_whitespace();
     for s in groups {
         assert!(s.parse::<f64>().is_ok());
     }
 
-    let result = new_ucmd!().arg("--groups").succeeds();
+    let result = scene.ucmd().arg("--groups").succeeds();
-    assert!(result.success);
     let groups = result.stdout_str().trim().split_whitespace();
     for s in groups {
         assert!(s.parse::<f64>().is_ok());
@@ -108,11 +122,13 @@ fn test_id_groups() {
 
 #[test]
 fn test_id_user() {
-    let mut result = new_ucmd!().arg("-u").succeeds();
+    let scene = TestScenario::new(util_name!());
+
+    let result = scene.ucmd().arg("-u").succeeds();
     let s1 = result.stdout_str().trim();
     assert!(s1.parse::<f64>().is_ok());
 
-    result = new_ucmd!().arg("--user").succeeds();
+    let result = scene.ucmd().arg("--user").succeeds();
     let s1 = result.stdout_str().trim();
     assert!(s1.parse::<f64>().is_ok());
 }
@@ -120,28 +136,34 @@ fn test_id_user() {
 #[test]
 fn test_id_pretty_print() {
     let username = return_whoami_username();
-    if username == "" {
+    if username.is_empty() {
-        // Sometimes, the CI is failing here
         return;
     }
 
-    let result = new_ucmd!().arg("-p").run();
+    let scene = TestScenario::new(util_name!());
-    if result.stdout_str().trim() == "" {
+    let result = scene.ucmd().arg("-p").run();
-        // Sometimes, the CI is failing here with
+    if result.stdout_str().trim().is_empty() {
-        // old rust versions on Linux
+        // this fails only on: "MinRustV (ubuntu-latest, feat_os_unix)"
+        // `rustc 1.40.0 (73528e339 2019-12-16)`
+        // run: /home/runner/work/coreutils/coreutils/target/debug/coreutils id -p
+        // thread 'test_id::test_id_pretty_print' panicked at 'Command was expected to succeed.
+        // stdout =
+        // stderr = ', tests/common/util.rs:157:13
+        println!("test skipped:");
         return;
     }
+
     result.success().stdout_contains(username);
 }
 
 #[test]
 fn test_id_password_style() {
     let username = return_whoami_username();
-    if username == "" {
+    if username.is_empty() {
-        // Sometimes, the CI is failing here
         return;
     }
 
     let result = new_ucmd!().arg("-P").succeeds();
+
     assert!(result.stdout_str().starts_with(&username));
 }

tests/by-util/test_install.rs

@@ -11,12 +11,10 @@ use std::thread::sleep;
 fn test_install_help() {
     let (_, mut ucmd) = at_and_ucmd!();
 
-    assert!(ucmd
+    ucmd.arg("--help")
-        .arg("--help")
         .succeeds()
         .no_stderr()
-        .stdout
+        .stdout_contains("FLAGS:");
-        .contains("FLAGS:"));
 }
 
 #[test]
@@ -59,13 +57,11 @@ fn test_install_failing_not_dir() {
     at.touch(file1);
     at.touch(file2);
     at.touch(file3);
-    assert!(ucmd
+    ucmd.arg(file1)
-        .arg(file1)
         .arg(file2)
         .arg(file3)
         .fails()
-        .stderr
+        .stderr_contains("not a directory");
-        .contains("not a directory"));
 }
 
 #[test]
@@ -77,13 +73,11 @@ fn test_install_unimplemented_arg() {
 
     at.touch(file);
     at.mkdir(dir);
-    assert!(ucmd
+    ucmd.arg(context_arg)
-        .arg(context_arg)
         .arg(file)
         .arg(dir)
         .fails()
-        .stderr
+        .stderr_contains("Unimplemented");
-        .contains("Unimplemented"));
 
     assert!(!at.file_exists(&format!("{}/{}", dir, file)));
 }
@@ -231,13 +225,11 @@ fn test_install_mode_failing() {
 
     at.touch(file);
     at.mkdir(dir);
-    assert!(ucmd
+    ucmd.arg(file)
-        .arg(file)
         .arg(dir)
         .arg(mode_arg)
         .fails()
-        .stderr
+        .stderr_contains("Invalid mode string: invalid digit found in string");
-        .contains("Invalid mode string: invalid digit found in string"));
 
     let dest_file = &format!("{}/{}", dir, file);
     assert!(at.file_exists(file));
@@ -336,7 +328,7 @@ fn test_install_target_new_file_with_owner() {
         .arg(format!("{}/{}", dir, file))
         .run();
 
-    if is_ci() && result.stderr.contains("error: no such user:") {
+    if is_ci() && result.stderr_str().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;
@@ -359,7 +351,7 @@ fn test_install_target_new_file_failing_nonexistent_parent() {
     ucmd.arg(file1)
         .arg(format!("{}/{}", dir, file2))
         .fails()
-        .stderr_contains(&"not a directory");
+        .stderr_contains(&"No such file or directory");
 }
 
 #[test]
@@ -443,9 +435,12 @@ fn test_install_failing_omitting_directory() {
     at.mkdir(dir2);
     at.touch(file1);
 
-    let r = ucmd.arg(dir1).arg(file1).arg(dir2).run();
+    ucmd.arg(dir1)
-    assert!(r.code == Some(1));
+        .arg(file1)
-    assert!(r.stderr.contains("omitting directory"));
+        .arg(dir2)
+        .fails()
+        .code_is(1)
+        .stderr_contains("omitting directory");
 }
 
 #[test]
@@ -458,9 +453,12 @@ fn test_install_failing_no_such_file() {
     at.mkdir(dir1);
     at.touch(file1);
 
-    let r = ucmd.arg(file1).arg(file2).arg(dir1).run();
+    ucmd.arg(file1)
-    assert!(r.code == Some(1));
+        .arg(file2)
-    assert!(r.stderr.contains("No such file or directory"));
+        .arg(dir1)
+        .fails()
+        .code_is(1)
+        .stderr_contains("No such file or directory");
 }
 
 #[test]
@@ -613,33 +611,64 @@ fn test_install_and_strip_with_program() {
 #[test]
 #[cfg(not(windows))]
 fn test_install_and_strip_with_invalid_program() {
-    let scene = TestScenario::new(util_name!());
+    new_ucmd!()
-
-    let stderr = scene
-        .ucmd()
         .arg("-s")
         .arg("--strip-program")
         .arg("/bin/date")
         .arg(strip_source_file())
         .arg(STRIP_TARGET_FILE)
         .fails()
-        .stderr;
+        .stderr_contains("strip program failed");
-    assert!(stderr.contains("strip program failed"));
 }
 
 #[test]
 #[cfg(not(windows))]
 fn test_install_and_strip_with_non_existent_program() {
-    let scene = TestScenario::new(util_name!());
+    new_ucmd!()
-
-    let stderr = scene
-        .ucmd()
         .arg("-s")
         .arg("--strip-program")
         .arg("/usr/bin/non_existent_program")
         .arg(strip_source_file())
         .arg(STRIP_TARGET_FILE)
         .fails()
-        .stderr;
+        .stderr_contains("No such file or directory");
-    assert!(stderr.contains("No such file or directory"));
+}
+
+#[test]
+fn test_install_creating_leading_dirs() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let source = "create_leading_test_file";
+    let target = "dir1/dir2/dir3/test_file";
+
+    at.touch(source);
+
+    scene
+        .ucmd()
+        .arg("-D")
+        .arg(source)
+        .arg(at.plus(target))
+        .succeeds()
+        .no_stderr();
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_install_creating_leading_dir_fails_on_long_name() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    let source = "create_leading_test_file";
+    let target = format!("{}/test_file", "d".repeat(libc::PATH_MAX as usize + 1));
+
+    at.touch(source);
+
+    scene
+        .ucmd()
+        .arg("-D")
+        .arg(source)
+        .arg(at.plus(target.as_str()))
+        .fails()
+        .stderr_contains("failed to create");
 }

tests/by-util/test_ln.rs

@@ -299,13 +299,11 @@ fn test_symlink_overwrite_dir_fail() {
     at.touch(path_a);
     at.mkdir(path_b);
 
-    assert!(
+    assert!(!ucmd
-        ucmd.args(&["-s", "-T", path_a, path_b])
+        .args(&["-s", "-T", path_a, path_b])
-            .fails()
+        .fails()
-            .stderr
+        .stderr_str()
-            .len()
+        .is_empty());
-            > 0
-    );
 }
 
 #[test]
@@ -358,7 +356,11 @@ fn test_symlink_target_only() {
 
     at.mkdir(dir);
 
-    assert!(ucmd.args(&["-s", "-t", dir]).fails().stderr.len() > 0);
+    assert!(!ucmd
+        .args(&["-s", "-t", dir])
+        .fails()
+        .stderr_str()
+        .is_empty());
 }
 
 #[test]

tests/by-util/test_ls.rs

@@ -5,6 +5,7 @@ use crate::common::util::*;
 extern crate regex;
 use self::regex::Regex;
 
+use std::path::Path;
 use std::thread::sleep;
 use std::time::Duration;
 
@@ -102,6 +103,20 @@ fn test_ls_width() {
             .succeeds()
             .stdout_only("test-width-1\ntest-width-2\ntest-width-3\ntest-width-4\n");
     }
+
+    scene
+        .ucmd()
+        .arg("-w=bad")
+        .fails()
+        .stderr_contains("invalid line width");
+
+    for option in &["-w 1a", "-w=1a", "--width=1a", "--width 1a"] {
+        scene
+            .ucmd()
+            .args(&option.split(" ").collect::<Vec<_>>())
+            .fails()
+            .stderr_only("ls: error: invalid line width: ‘1a’");
+    }
 }
 
 #[test]
@@ -435,6 +450,39 @@ fn test_ls_deref() {
     assert!(!re.is_match(result.stdout_str().trim()));
 }
 
+#[test]
+fn test_ls_sort_none() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.touch("test-3");
+    at.touch("test-1");
+    at.touch("test-2");
+
+    // Order is not specified so we just check that it doesn't
+    // give any errors.
+    scene.ucmd().arg("--sort=none").succeeds();
+    scene.ucmd().arg("-U").succeeds();
+}
+
+#[test]
+fn test_ls_sort_name() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.touch("test-3");
+    at.touch("test-1");
+    at.touch("test-2");
+
+    let sep = if cfg!(unix) { "\n" } else { "  " };
+
+    scene
+        .ucmd()
+        .arg("--sort=name")
+        .succeeds()
+        .stdout_is(["test-1", "test-2", "test-3\n"].join(sep));
+}
+
 #[test]
 fn test_ls_order_size() {
     let scene = TestScenario::new(util_name!());
@@ -463,6 +511,18 @@ fn test_ls_order_size() {
     result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
     #[cfg(windows)]
     result.stdout_only("test-1  test-2  test-3  test-4\n");
+
+    let result = scene.ucmd().arg("--sort=size").succeeds();
+    #[cfg(not(windows))]
+    result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
+    #[cfg(windows)]
+    result.stdout_only("test-4  test-3  test-2  test-1\n");
+
+    let result = scene.ucmd().arg("--sort=size").arg("-r").succeeds();
+    #[cfg(not(windows))]
+    result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
+    #[cfg(windows)]
+    result.stdout_only("test-1  test-2  test-3  test-4\n");
 }
 
 #[test]
@@ -471,13 +531,16 @@ fn test_ls_long_ctime() {
     let at = &scene.fixtures;
 
     at.touch("test-long-ctime-1");
-    let result = scene.ucmd().arg("-lc").succeeds();
 
-    // Should show the time on Unix, but question marks on windows.
+    for arg in &["-c", "--time=ctime", "--time=status"] {
-    #[cfg(unix)]
+        let result = scene.ucmd().arg("-l").arg(arg).succeeds();
-    result.stdout_contains(":");
+
-    #[cfg(not(unix))]
+        // Should show the time on Unix, but question marks on windows.
-    result.stdout_contains("???");
+        #[cfg(unix)]
+        result.stdout_contains(":");
+        #[cfg(not(unix))]
+        result.stdout_contains("???");
+    }
 }
 
 #[test]
@@ -518,32 +581,46 @@ fn test_ls_order_time() {
     #[cfg(windows)]
     result.stdout_only("test-4  test-3  test-2  test-1\n");
 
+    let result = scene.ucmd().arg("--sort=time").succeeds();
+    #[cfg(not(windows))]
+    result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
+    #[cfg(windows)]
+    result.stdout_only("test-4  test-3  test-2  test-1\n");
+
     let result = scene.ucmd().arg("-tr").succeeds();
     #[cfg(not(windows))]
     result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
     #[cfg(windows)]
     result.stdout_only("test-1  test-2  test-3  test-4\n");
 
+    let result = scene.ucmd().arg("--sort=time").arg("-r").succeeds();
+    #[cfg(not(windows))]
+    result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
+    #[cfg(windows)]
+    result.stdout_only("test-1  test-2  test-3  test-4\n");
+
     // 3 was accessed last in the read
     // So the order should be 2 3 4 1
-    let result = scene.ucmd().arg("-tu").succeeds();
+    for arg in &["-u", "--time=atime", "--time=access", "--time=use"] {
-    let file3_access = at.open("test-3").metadata().unwrap().accessed().unwrap();
+        let result = scene.ucmd().arg("-t").arg(arg).succeeds();
-    let file4_access = at.open("test-4").metadata().unwrap().accessed().unwrap();
+        let file3_access = at.open("test-3").metadata().unwrap().accessed().unwrap();
+        let file4_access = at.open("test-4").metadata().unwrap().accessed().unwrap();
 
-    // It seems to be dependent on the platform whether the access time is actually set
+        // It seems to be dependent on the platform whether the access time is actually set
-    if file3_access > file4_access {
+        if file3_access > file4_access {
-        if cfg!(not(windows)) {
+            if cfg!(not(windows)) {
-            result.stdout_only("test-3\ntest-4\ntest-2\ntest-1\n");
+                result.stdout_only("test-3\ntest-4\ntest-2\ntest-1\n");
+            } else {
+                result.stdout_only("test-3  test-4  test-2  test-1\n");
+            }
         } else {
-            result.stdout_only("test-3  test-4  test-2  test-1\n");
+            // Access time does not seem to be set on Windows and some other
-        }
+            // systems so the order is 4 3 2 1
-    } else {
+            if cfg!(not(windows)) {
-        // Access time does not seem to be set on Windows and some other
+                result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
-        // systems so the order is 4 3 2 1
+            } else {
-        if cfg!(not(windows)) {
+                result.stdout_only("test-4  test-3  test-2  test-1\n");
-            result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
+            }
-        } else {
-            result.stdout_only("test-4  test-3  test-2  test-1\n");
         }
     }
 
@@ -620,20 +697,27 @@ fn test_ls_recursive() {
     result.stdout_contains(&"a\\b:\nb");
 }
 
-#[cfg(unix)]
 #[test]
-fn test_ls_ls_color() {
+fn test_ls_color() {
     let scene = TestScenario::new(util_name!());
     let at = &scene.fixtures;
     at.mkdir("a");
-    at.mkdir("a/nested_dir");
+    let nested_dir = Path::new("a")
+        .join("nested_dir")
+        .to_string_lossy()
+        .to_string();
+    at.mkdir(&nested_dir);
     at.mkdir("z");
-    at.touch(&at.plus_as_string("a/nested_file"));
+    let nested_file = Path::new("a")
+        .join("nested_file")
+        .to_string_lossy()
+        .to_string();
+    at.touch(&nested_file);
     at.touch("test-color");
 
-    let a_with_colors = "\x1b[01;34ma\x1b[0m";
+    let a_with_colors = "\x1b[1;34ma\x1b[0m";
-    let z_with_colors = "\x1b[01;34mz\x1b[0m";
+    let z_with_colors = "\x1b[1;34mz\x1b[0m";
-    let nested_dir_with_colors = "\x1b[01;34mnested_dir\x1b[0m";
+    let nested_dir_with_colors = "\x1b[1;34mnested_dir\x1b[0m";
 
     // Color is disabled by default
     let result = scene.ucmd().succeeds();
@@ -669,14 +753,6 @@ fn test_ls_ls_color() {
         .succeeds()
         .stdout_contains(nested_dir_with_colors);
 
-    // Color has no effect
-    scene
-        .ucmd()
-        .arg("--color=always")
-        .arg("a/nested_file")
-        .succeeds()
-        .stdout_contains("a/nested_file\n");
-
     // No output
     scene
         .ucmd()
@@ -816,7 +892,7 @@ fn test_ls_indicator_style() {
     let options = vec!["classify", "file-type", "slash"];
     for opt in options {
         // Verify that classify and file-type both contain indicators for symlinks.
-        let result = scene
+        scene
             .ucmd()
             .arg(format!("--indicator-style={}", opt))
             .succeeds()
@@ -826,7 +902,7 @@ fn test_ls_indicator_style() {
     // Same test as above, but with the alternate flags.
     let options = vec!["--classify", "--file-type", "-p"];
     for opt in options {
-        let result = scene
+        scene
             .ucmd()
             .arg(format!("{}", opt))
             .succeeds()
@@ -837,7 +913,7 @@ fn test_ls_indicator_style() {
     let options = vec!["classify", "file-type"];
     for opt in options {
         // Verify that classify and file-type both contain indicators for symlinks.
-        let result = scene
+        scene
             .ucmd()
             .arg(format!("--indicator-style={}", opt))
             .succeeds()
@@ -961,7 +1037,7 @@ fn test_ls_hidden_windows() {
 
     let result = scene.ucmd().succeeds();
     assert!(!result.stdout_str().contains(file));
-    let result = scene.ucmd().arg("-a").succeeds().stdout_contains(file);
+    scene.ucmd().arg("-a").succeeds().stdout_contains(file);
 }
 
 #[test]
@@ -1051,9 +1127,11 @@ fn test_ls_quoting_style() {
     at.touch("one");
 
     // It seems that windows doesn't allow \n in filenames.
+    // And it also doesn't like \, of course.
     #[cfg(unix)]
     {
         at.touch("one\ntwo");
+        at.touch("one\\two");
         // Default is shell-escape
         scene
             .ucmd()
@@ -1115,6 +1193,42 @@ fn test_ls_quoting_style() {
                 .succeeds()
                 .stdout_only(format!("{}\n", correct));
         }
+
+        for (arg, correct) in &[
+            ("--quoting-style=literal", "one\\two"),
+            ("-N", "one\\two"),
+            ("--quoting-style=c", "\"one\\\\two\""),
+            ("-Q", "\"one\\\\two\""),
+            ("--quote-name", "\"one\\\\two\""),
+            ("--quoting-style=escape", "one\\\\two"),
+            ("-b", "one\\\\two"),
+            ("--quoting-style=shell-escape", "'one\\two'"),
+            ("--quoting-style=shell-escape-always", "'one\\two'"),
+            ("--quoting-style=shell", "'one\\two'"),
+            ("--quoting-style=shell-always", "'one\\two'"),
+        ] {
+            scene
+                .ucmd()
+                .arg(arg)
+                .arg("one\\two")
+                .succeeds()
+                .stdout_only(format!("{}\n", correct));
+        }
+
+        // Tests for a character that forces quotation in shell-style escaping
+        // after a character in a dollar expression
+        at.touch("one\n&two");
+        for (arg, correct) in &[
+            ("--quoting-style=shell-escape", "'one'$'\\n''&two'"),
+            ("--quoting-style=shell-escape-always", "'one'$'\\n''&two'"),
+        ] {
+            scene
+                .ucmd()
+                .arg(arg)
+                .arg("one\n&two")
+                .succeeds()
+                .stdout_only(format!("{}\n", correct));
+        }
     }
 
     scene
@@ -1314,3 +1428,256 @@ fn test_ls_ignore_hide() {
         .stderr_contains(&"Invalid pattern")
         .stdout_is("CONTRIBUTING.md\nREADME.md\nREADMECAREFULLY.md\nsome_other_file\n");
 }
+
+#[test]
+fn test_ls_ignore_backups() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.touch("somefile");
+    at.touch("somebackup~");
+    at.touch(".somehiddenfile");
+    at.touch(".somehiddenbackup~");
+
+    scene.ucmd().arg("-B").succeeds().stdout_is("somefile\n");
+    scene
+        .ucmd()
+        .arg("--ignore-backups")
+        .succeeds()
+        .stdout_is("somefile\n");
+
+    scene
+        .ucmd()
+        .arg("-aB")
+        .succeeds()
+        .stdout_contains(".somehiddenfile")
+        .stdout_contains("somefile")
+        .stdout_does_not_contain("somebackup")
+        .stdout_does_not_contain(".somehiddenbackup~");
+
+    scene
+        .ucmd()
+        .arg("-a")
+        .arg("--ignore-backups")
+        .succeeds()
+        .stdout_contains(".somehiddenfile")
+        .stdout_contains("somefile")
+        .stdout_does_not_contain("somebackup")
+        .stdout_does_not_contain(".somehiddenbackup~");
+}
+
+#[test]
+fn test_ls_directory() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.mkdir("some_dir");
+    at.symlink_dir("some_dir", "sym_dir");
+
+    at.touch(Path::new("some_dir").join("nested_file").to_str().unwrap());
+
+    scene
+        .ucmd()
+        .arg("some_dir")
+        .succeeds()
+        .stdout_is("nested_file\n");
+
+    scene
+        .ucmd()
+        .arg("--directory")
+        .arg("some_dir")
+        .succeeds()
+        .stdout_is("some_dir\n");
+
+    scene
+        .ucmd()
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_is("nested_file\n");
+}
+
+#[test]
+fn test_ls_deref_command_line() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.touch("some_file");
+    at.symlink_file("some_file", "sym_file");
+
+    scene
+        .ucmd()
+        .arg("sym_file")
+        .succeeds()
+        .stdout_is("sym_file\n");
+
+    // -l changes the default to no dereferencing
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("sym_file")
+        .succeeds()
+        .stdout_contains("sym_file ->");
+
+    scene
+        .ucmd()
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_file")
+        .succeeds()
+        .stdout_is("sym_file\n");
+
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_file")
+        .succeeds()
+        .stdout_contains("sym_file ->");
+
+    scene
+        .ucmd()
+        .arg("--dereference-command-line")
+        .arg("sym_file")
+        .succeeds()
+        .stdout_is("sym_file\n");
+
+    let result = scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line")
+        .arg("sym_file")
+        .succeeds();
+
+    assert!(!result.stdout_str().contains("->"));
+
+    let result = scene.ucmd().arg("-lH").arg("sym_file").succeeds();
+
+    assert!(!result.stdout_str().contains("sym_file ->"));
+
+    // If the symlink is not a command line argument, it must be shown normally
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line")
+        .succeeds()
+        .stdout_contains("sym_file ->");
+}
+
+#[test]
+fn test_ls_deref_command_line_dir() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+
+    at.mkdir("some_dir");
+    at.symlink_dir("some_dir", "sym_dir");
+
+    at.touch(Path::new("some_dir").join("nested_file").to_str().unwrap());
+
+    scene
+        .ucmd()
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    scene
+        .ucmd()
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    scene
+        .ucmd()
+        .arg("--dereference-command-line")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    scene
+        .ucmd()
+        .arg("-lH")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("nested_file");
+
+    // If the symlink is not a command line argument, it must be shown normally
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    scene
+        .ucmd()
+        .arg("-lH")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--dereference-command-line-symlink-to-dir")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    // --directory does not dereference anything by default
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--directory")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    let result = scene
+        .ucmd()
+        .arg("-l")
+        .arg("--directory")
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_dir")
+        .succeeds();
+
+    assert!(!result.stdout_str().ends_with("sym_dir"));
+
+    // --classify does not dereference anything by default
+    scene
+        .ucmd()
+        .arg("-l")
+        .arg("--directory")
+        .arg("sym_dir")
+        .succeeds()
+        .stdout_contains("sym_dir ->");
+
+    let result = scene
+        .ucmd()
+        .arg("-l")
+        .arg("--directory")
+        .arg("--dereference-command-line-symlink-to-dir")
+        .arg("sym_dir")
+        .succeeds();
+
+    assert!(!result.stdout_str().ends_with("sym_dir"));
+}

tests/by-util/test_mkfifo.rs

@@ -19,8 +19,7 @@ fn test_create_one_fifo_with_invalid_mode() {
         .arg("-m")
         .arg("invalid")
         .fails()
-        .stderr
+        .stderr_contains("invalid mode");
-        .contains("invalid mode");
 }
 
 #[test]

tests/by-util/test_mktemp.rs

@@ -113,17 +113,14 @@ fn test_mktemp_mktemp_t() {
         .arg("-t")
         .arg(TEST_TEMPLATE7)
         .succeeds();
-    let result = scene
+    scene
         .ucmd()
         .env(TMPDIR, &pathname)
         .arg("-t")
         .arg(TEST_TEMPLATE8)
-        .fails();
+        .fails()
-    println!("stdout {}", result.stdout);
+        .no_stdout()
-    println!("stderr {}", result.stderr);
+        .stderr_contains("error: suffix cannot contain any path separators");
-    assert!(result
-        .stderr
-        .contains("error: suffix cannot contain any path separators"));
 }
 
 #[test]
@@ -391,10 +388,8 @@ fn test_mktemp_tmpdir_one_arg() {
         .arg("--tmpdir")
         .arg("apt-key-gpghome.XXXXXXXXXX")
         .succeeds();
-    println!("stdout {}", result.stdout);
+    result.no_stderr().stdout_contains("apt-key-gpghome.");
-    println!("stderr {}", result.stderr);
+    assert!(PathBuf::from(result.stdout_str().trim()).is_file());
-    assert!(result.stdout.contains("apt-key-gpghome."));
-    assert!(PathBuf::from(result.stdout.trim()).is_file());
 }
 
 #[test]
@@ -407,8 +402,6 @@ fn test_mktemp_directory_tmpdir() {
         .arg("--tmpdir")
         .arg("apt-key-gpghome.XXXXXXXXXX")
         .succeeds();
-    println!("stdout {}", result.stdout);
+    result.no_stderr().stdout_contains("apt-key-gpghome.");
-    println!("stderr {}", result.stderr);
+    assert!(PathBuf::from(result.stdout_str().trim()).is_dir());
-    assert!(result.stdout.contains("apt-key-gpghome."));
-    assert!(PathBuf::from(result.stdout.trim()).is_dir());
 }

tests/by-util/test_more.rs

@@ -2,18 +2,15 @@ use crate::common::util::*;
 
 #[test]
 fn test_more_no_arg() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    // stderr = more: Reading from stdin isn't supported yet.
-    let result = ucmd.run();
+    new_ucmd!().fails();
-    assert!(!result.success);
 }
 
 #[test]
 fn test_more_dir_arg() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let result = new_ucmd!().arg(".").run();
-    ucmd.arg(".");
+    result.failure();
-    let result = ucmd.run();
-    assert!(!result.success);
     const EXPECTED_ERROR_MESSAGE: &str =
         "more: '.' is a directory.\nTry 'more --help' for more information.";
-    assert_eq!(result.stderr.trim(), EXPECTED_ERROR_MESSAGE);
+    assert_eq!(result.stderr_str().trim(), EXPECTED_ERROR_MESSAGE);
 }

tests/by-util/test_mv.rs

@@ -476,16 +476,9 @@ fn test_mv_overwrite_nonempty_dir() {
     // GNU:     "mv: cannot move ‘a’ to ‘b’: Directory not empty"
 
     // Verbose output for the move should not be shown on failure
-    assert!(
+    let result = ucmd.arg("-vT").arg(dir_a).arg(dir_b).fails();
-        ucmd.arg("-vT")
+    result.no_stdout();
-            .arg(dir_a)
+    assert!(!result.stderr_str().is_empty());
-            .arg(dir_b)
-            .fails()
-            .no_stdout()
-            .stderr
-            .len()
-            > 0
-    );
 
     assert!(at.dir_exists(dir_a));
     assert!(at.dir_exists(dir_b));
@@ -526,15 +519,15 @@ fn test_mv_errors() {
 
     // $ mv -T -t a b
     // mv: cannot combine --target-directory (-t) and --no-target-directory (-T)
-    let result = scene
+    scene
         .ucmd()
         .arg("-T")
         .arg("-t")
         .arg(dir)
         .arg(file_a)
         .arg(file_b)
-        .fails();
+        .fails()
-    assert!(result.stderr.contains("cannot be used with"));
+        .stderr_contains("cannot be used with");
 
     // $ at.touch file && at.mkdir dir
     // $ mv -T file dir
@@ -553,7 +546,13 @@ fn test_mv_errors() {
     // $ at.mkdir dir && at.touch file
     // $ mv dir file
     // err == mv: cannot overwrite non-directory ‘file’ with directory ‘dir’
-    assert!(scene.ucmd().arg(dir).arg(file_a).fails().stderr.len() > 0);
+    assert!(!scene
+        .ucmd()
+        .arg(dir)
+        .arg(file_a)
+        .fails()
+        .stderr_str()
+        .is_empty());
 }
 
 #[test]

tests/by-util/test_nice.rs

@@ -16,7 +16,7 @@ fn test_negative_adjustment() {
 
     let res = new_ucmd!().args(&["-n", "-1", "true"]).run();
     assert!(res
-        .stderr
+        .stderr_str()
         .starts_with("nice: warning: setpriority: Permission denied"));
 }
 

tests/by-util/test_nproc.rs

@@ -2,54 +2,46 @@ use crate::common::util::*;
 
 #[test]
 fn test_nproc() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let nproc: u8 = new_ucmd!().succeeds().stdout_str().trim().parse().unwrap();
-    let result = ucmd.run();
-    assert!(result.success);
-    let nproc: u8 = result.stdout.trim().parse().unwrap();
     assert!(nproc > 0);
 }
 
 #[test]
 fn test_nproc_all_omp() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let result = new_ucmd!().arg("--all").succeeds();
-    let result = ucmd.arg("--all").run();
+
-    assert!(result.success);
+    let nproc: u8 = result.stdout_str().trim().parse().unwrap();
-    let nproc: u8 = result.stdout.trim().parse().unwrap();
     assert!(nproc > 0);
 
     let result = TestScenario::new(util_name!())
         .ucmd_keepenv()
         .env("OMP_NUM_THREADS", "1")
-        .run();
+        .succeeds();
-    assert!(result.success);
+
-    let nproc_omp: u8 = result.stdout.trim().parse().unwrap();
+    let nproc_omp: u8 = result.stdout_str().trim().parse().unwrap();
     assert!(nproc - 1 == nproc_omp);
 
     let result = TestScenario::new(util_name!())
         .ucmd_keepenv()
         .env("OMP_NUM_THREADS", "1") // Has no effect
         .arg("--all")
-        .run();
+        .succeeds();
-    assert!(result.success);
+    let nproc_omp: u8 = result.stdout_str().trim().parse().unwrap();
-    let nproc_omp: u8 = result.stdout.trim().parse().unwrap();
     assert!(nproc == nproc_omp);
 }
 
 #[test]
 fn test_nproc_ignore() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let result = new_ucmd!().succeeds();
-    let result = ucmd.run();
+    let nproc: u8 = result.stdout_str().trim().parse().unwrap();
-    assert!(result.success);
-    let nproc: u8 = result.stdout.trim().parse().unwrap();
     if nproc > 1 {
         // Ignore all CPU but one
         let result = TestScenario::new(util_name!())
             .ucmd_keepenv()
             .arg("--ignore")
             .arg((nproc - 1).to_string())
-            .run();
+            .succeeds();
-        assert!(result.success);
+        let nproc: u8 = result.stdout_str().trim().parse().unwrap();
-        let nproc: u8 = result.stdout.trim().parse().unwrap();
         assert!(nproc == 1);
     }
 }

tests/by-util/test_pinky.rs

@@ -43,11 +43,9 @@ fn test_short_format_i() {
     let actual = TestScenario::new(util_name!())
         .ucmd()
         .args(&args)
-        .run()
+        .succeeds()
-        .stdout;
+        .stdout_move_str();
     let expect = expected_result(&args);
-    println!("actual: {:?}", actual);
-    println!("expect: {:?}", expect);
     let v_actual: Vec<&str> = actual.split_whitespace().collect();
     let v_expect: Vec<&str> = expect.split_whitespace().collect();
     assert_eq!(v_actual, v_expect);
@@ -62,11 +60,9 @@ fn test_short_format_q() {
     let actual = TestScenario::new(util_name!())
         .ucmd()
         .args(&args)
-        .run()
+        .succeeds()
-        .stdout;
+        .stdout_move_str();
     let expect = expected_result(&args);
-    println!("actual: {:?}", actual);
-    println!("expect: {:?}", expect);
     let v_actual: Vec<&str> = actual.split_whitespace().collect();
     let v_expect: Vec<&str> = expect.split_whitespace().collect();
     assert_eq!(v_actual, v_expect);
@@ -79,5 +75,5 @@ fn expected_result(args: &[&str]) -> String {
         .env("LANGUAGE", "C")
         .args(args)
         .run()
-        .stdout
+        .stdout_move_str()
 }

tests/by-util/test_printenv.rs

@@ -7,10 +7,11 @@ fn test_get_all() {
     env::set_var(key, "VALUE");
     assert_eq!(env::var(key), Ok("VALUE".to_string()));
 
-    let result = TestScenario::new(util_name!()).ucmd_keepenv().run();
+    TestScenario::new(util_name!())
-    assert!(result.success);
+        .ucmd_keepenv()
-    assert!(result.stdout.contains("HOME="));
+        .succeeds()
-    assert!(result.stdout.contains("KEY=VALUE"));
+        .stdout_contains("HOME=")
+        .stdout_contains("KEY=VALUE");
 }
 
 #[test]
@@ -22,9 +23,8 @@ fn test_get_var() {
     let result = TestScenario::new(util_name!())
         .ucmd_keepenv()
         .arg("KEY")
-        .run();
+        .succeeds();
 
-    assert!(result.success);
+    assert!(!result.stdout_str().is_empty());
-    assert!(!result.stdout.is_empty());
+    assert!(result.stdout_str().trim() == "VALUE");
-    assert!(result.stdout.trim() == "VALUE");
 }

tests/by-util/test_readlink.rs

@@ -5,7 +5,7 @@ static GIBBERISH: &'static str = "supercalifragilisticexpialidocious";
 #[test]
 fn test_canonicalize() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let actual = ucmd.arg("-f").arg(".").run().stdout;
+    let actual = ucmd.arg("-f").arg(".").run().stdout_move_str();
     let expect = at.root_dir_resolved() + "\n";
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -15,7 +15,7 @@ fn test_canonicalize() {
 #[test]
 fn test_canonicalize_existing() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let actual = ucmd.arg("-e").arg(".").run().stdout;
+    let actual = ucmd.arg("-e").arg(".").run().stdout_move_str();
     let expect = at.root_dir_resolved() + "\n";
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -25,7 +25,7 @@ fn test_canonicalize_existing() {
 #[test]
 fn test_canonicalize_missing() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let actual = ucmd.arg("-m").arg(GIBBERISH).run().stdout;
+    let actual = ucmd.arg("-m").arg(GIBBERISH).run().stdout_move_str();
     let expect = path_concat!(at.root_dir_resolved(), GIBBERISH) + "\n";
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -37,7 +37,7 @@ fn test_long_redirection_to_current_dir() {
     let (at, mut ucmd) = at_and_ucmd!();
     // Create a 256-character path to current directory
     let dir = path_concat!(".", ..128);
-    let actual = ucmd.arg("-n").arg("-m").arg(dir).run().stdout;
+    let actual = ucmd.arg("-n").arg("-m").arg(dir).run().stdout_move_str();
     let expect = at.root_dir_resolved();
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -48,7 +48,12 @@ fn test_long_redirection_to_current_dir() {
 fn test_long_redirection_to_root() {
     // Create a 255-character path to root
     let dir = path_concat!("..", ..85);
-    let actual = new_ucmd!().arg("-n").arg("-m").arg(dir).run().stdout;
+    let actual = new_ucmd!()
+        .arg("-n")
+        .arg("-m")
+        .arg(dir)
+        .run()
+        .stdout_move_str();
     let expect = get_root_path();
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);

tests/by-util/test_shred.rs

@@ -36,9 +36,7 @@ fn test_shred_force() {
     at.set_readonly(file);
 
     // Try shred -u.
-    let result = scene.ucmd().arg("-u").arg(file).run();
+    scene.ucmd().arg("-u").arg(file).run();
-    println!("stderr = {:?}", result.stderr);
-    println!("stdout = {:?}", result.stdout);
 
     // file_a was not deleted because it is readonly.
     assert!(at.file_exists(file));

tests/by-util/test_shuf.rs

@@ -9,35 +9,28 @@ fn test_output_is_random_permutation() {
         .collect::<Vec<String>>()
         .join("\n");
 
-    let result = new_ucmd!()
+    let result = new_ucmd!().pipe_in(input.as_bytes()).succeeds();
-        .pipe_in(input.as_bytes())
+    result.no_stderr();
-        .succeeds()
-        .no_stderr()
-        .stdout
-        .clone();
 
     let mut result_seq: Vec<i32> = result
+        .stdout_str()
         .split("\n")
         .filter(|x| !x.is_empty())
         .map(|x| x.parse().unwrap())
         .collect();
     result_seq.sort();
-    assert_ne!(result, input, "Output is not randomised");
+    assert_ne!(result.stdout_str(), input, "Output is not randomised");
     assert_eq!(result_seq, input_seq, "Output is not a permutation");
 }
 
 #[test]
 fn test_zero_termination() {
     let input_seq = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10];
-    let result = new_ucmd!()
+    let result = new_ucmd!().arg("-z").arg("-i1-10").succeeds();
-        .arg("-z")
+    result.no_stderr();
-        .arg("-i1-10")
-        .succeeds()
-        .no_stderr()
-        .stdout
-        .clone();
 
     let mut result_seq: Vec<i32> = result
+        .stdout_str()
         .split("\0")
         .filter(|x| !x.is_empty())
         .map(|x| x.parse().unwrap())
@@ -57,12 +50,11 @@ fn test_echo() {
                 .map(|x| x.to_string())
                 .collect::<Vec<String>>(),
         )
-        .succeeds()
+        .succeeds();
-        .no_stderr()
+    result.no_stderr();
-        .stdout
-        .clone();
 
     let mut result_seq: Vec<i32> = result
+        .stdout_str()
         .split("\n")
         .filter(|x| !x.is_empty())
         .map(|x| x.parse().unwrap())
@@ -84,12 +76,11 @@ fn test_head_count() {
     let result = new_ucmd!()
         .args(&["-n", &repeat_limit.to_string()])
         .pipe_in(input.as_bytes())
-        .succeeds()
+        .succeeds();
-        .no_stderr()
+    result.no_stderr();
-        .stdout
-        .clone();
 
     let mut result_seq: Vec<i32> = result
+        .stdout_str()
         .split("\n")
         .filter(|x| !x.is_empty())
         .map(|x| x.parse().unwrap())
@@ -99,7 +90,7 @@ fn test_head_count() {
     assert!(
         result_seq.iter().all(|x| input_seq.contains(x)),
         "Output includes element not from input: {}",
-        result
+        result.stdout_str()
     )
 }
 
@@ -117,12 +108,11 @@ fn test_repeat() {
         .arg("-r")
         .args(&["-n", &repeat_limit.to_string()])
         .pipe_in(input.as_bytes())
-        .succeeds()
+        .succeeds();
-        .no_stderr()
+    result.no_stderr();
-        .stdout
-        .clone();
 
     let result_seq: Vec<i32> = result
+        .stdout_str()
         .split("\n")
         .filter(|x| !x.is_empty())
         .map(|x| x.parse().unwrap())
@@ -146,14 +136,11 @@ fn test_repeat() {
 fn test_file_input() {
     let expected_seq = vec![11, 12, 13, 14, 15, 16, 17, 18, 19, 20];
 
-    let result = new_ucmd!()
+    let result = new_ucmd!().arg("file_input.txt").succeeds();
-        .arg("file_input.txt")
+    result.no_stderr();
-        .succeeds()
-        .no_stderr()
-        .stdout
-        .clone();
 
     let mut result_seq: Vec<i32> = result
+        .stdout_str()
         .split("\n")
         .filter(|x| !x.is_empty())
         .map(|x| x.parse().unwrap())
@@ -164,52 +151,50 @@ fn test_file_input() {
 
 #[test]
 fn test_shuf_echo_and_input_range_not_allowed() {
-    let result = new_ucmd!().args(&["-e", "0", "-i", "0-2"]).run();
+    new_ucmd!()
-
+        .args(&["-e", "0", "-i", "0-2"])
-    assert!(!result.success);
+        .fails()
-    assert!(result
+        .stderr_contains(
-        .stderr
+            "The argument '--input-range <LO-HI>' cannot be used with '--echo <ARG>...'",
-        .contains("The argument '--input-range <LO-HI>' cannot be used with '--echo <ARG>...'"));
+        );
 }
 
 #[test]
 fn test_shuf_input_range_and_file_not_allowed() {
-    let result = new_ucmd!().args(&["-i", "0-9", "file"]).run();
+    new_ucmd!()
-
+        .args(&["-i", "0-9", "file"])
-    assert!(!result.success);
+        .fails()
-    assert!(result
+        .stderr_contains("The argument '<file>' cannot be used with '--input-range <LO-HI>'");
-        .stderr
-        .contains("The argument '<file>' cannot be used with '--input-range <LO-HI>'"));
 }
 
 #[test]
 fn test_shuf_invalid_input_range_one() {
-    let result = new_ucmd!().args(&["-i", "0"]).run();
+    new_ucmd!()
-
+        .args(&["-i", "0"])
-    assert!(!result.success);
+        .fails()
-    assert!(result.stderr.contains("invalid input range"));
+        .stderr_contains("invalid input range");
 }
 
 #[test]
 fn test_shuf_invalid_input_range_two() {
-    let result = new_ucmd!().args(&["-i", "a-9"]).run();
+    new_ucmd!()
-
+        .args(&["-i", "a-9"])
-    assert!(!result.success);
+        .fails()
-    assert!(result.stderr.contains("invalid input range: 'a'"));
+        .stderr_contains("invalid input range: 'a'");
 }
 
 #[test]
 fn test_shuf_invalid_input_range_three() {
-    let result = new_ucmd!().args(&["-i", "0-b"]).run();
+    new_ucmd!()
-
+        .args(&["-i", "0-b"])
-    assert!(!result.success);
+        .fails()
-    assert!(result.stderr.contains("invalid input range: 'b'"));
+        .stderr_contains("invalid input range: 'b'");
 }
 
 #[test]
 fn test_shuf_invalid_input_line_count() {
-    let result = new_ucmd!().args(&["-n", "a"]).run();
+    new_ucmd!()
-
+        .args(&["-n", "a"])
-    assert!(!result.success);
+        .fails()
-    assert!(result.stderr.contains("invalid line count: 'a'"));
+        .stderr_contains("invalid line count: 'a'");
 }

tests/by-util/test_sort.rs

@@ -16,10 +16,10 @@ fn test_months_whitespace() {
 #[test]
 fn test_version_empty_lines() {
     new_ucmd!()
-    .arg("-V")
+        .arg("-V")
-    .arg("version-empty-lines.txt")
+        .arg("version-empty-lines.txt")
-    .succeeds()
+        .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");
+        .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]
@@ -38,11 +38,7 @@ fn test_multiple_decimals_general() {
 
 #[test]
 fn test_multiple_decimals_numeric() {
-    new_ucmd!()
+    test_helper("multiple_decimals_numeric", "-n")
-        .arg("-n")
-        .arg("multiple_decimals_numeric.txt")
-        .succeeds()
-        .stdout_is("-2028789030\n-896689\n-8.90880\n-1\n-.05\n\n\n\n\n\n\n\n\n000\nCARAvan\n00000001\n1\n1.040000000\n1.444\n1.58590\n8.013\n45\n46.89\n               4567.\n4567.1\n4567.34\n\t\t\t\t\t\t\t\t\t\t4567..457\n\t\t\t\t37800\n\t\t\t\t\t\t45670.89079.098\n\t\t\t\t\t\t45670.89079.1\n576,446.88800000\n576,446.890\n4798908.340000000000\n4798908.45\n4798908.8909800\n");
 }
 
 #[test]
@@ -69,7 +65,7 @@ fn test_random_shuffle_len() {
     // check whether output is the same length as the input
     const FILE: &'static str = "default_unsorted_ints.expected";
     let (at, _ucmd) = at_and_ucmd!();
-    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
     let expected = at.read(FILE);
 
     assert_ne!(result, expected);
@@ -81,9 +77,9 @@ fn test_random_shuffle_contains_all_lines() {
     // check whether lines of input are all in output
     const FILE: &'static str = "default_unsorted_ints.expected";
     let (at, _ucmd) = at_and_ucmd!();
-    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
     let expected = at.read(FILE);
-    let result_sorted = new_ucmd!().pipe_in(result.clone()).run().stdout;
+    let result_sorted = new_ucmd!().pipe_in(result.clone()).run().stdout_move_str();
 
     assert_ne!(result, expected);
     assert_eq!(result_sorted, expected);
@@ -96,9 +92,9 @@ fn test_random_shuffle_two_runs_not_the_same() {
     // as the starting order, or if both random sorts end up having the same order.
     const FILE: &'static str = "default_unsorted_ints.expected";
     let (at, _ucmd) = at_and_ucmd!();
-    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
     let expected = at.read(FILE);
-    let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
 
     assert_ne!(result, expected);
     assert_ne!(result, unexpected);
@@ -111,9 +107,9 @@ fn test_random_shuffle_contains_two_runs_not_the_same() {
     // as the starting order, or if both random sorts end up having the same order.
     const FILE: &'static str = "default_unsorted_ints.expected";
     let (at, _ucmd) = at_and_ucmd!();
-    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let result = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
     let expected = at.read(FILE);
-    let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout;
+    let unexpected = new_ucmd!().arg("-R").arg(FILE).run().stdout_move_str();
 
     assert_ne!(result, expected);
     assert_ne!(result, unexpected);
@@ -585,3 +581,12 @@ fn test_check_silent() {
         .fails()
         .stdout_is("");
 }
+
+#[test]
+fn test_trailing_separator() {
+    new_ucmd!()
+    .args(&["-t", "x", "-k", "1,1"])
+    .pipe_in("aax\naaa\n")
+    .succeeds()
+    .stdout_is("aax\naaa\n");
+}

tests/by-util/test_stat.rs

@@ -194,7 +194,7 @@ fn test_terse_normal_format() {
     // note: contains birth/creation date which increases test fragility
     // * results may vary due to built-in `stat` limitations as well as linux kernel and rust version capability variations
     let args = ["-t", "/"];
-    let actual = new_ucmd!().args(&args).run().stdout;
+    let actual = new_ucmd!().args(&args).succeeds().stdout_move_str();
     let expect = expected_result(&args);
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -216,7 +216,7 @@ fn test_terse_normal_format() {
 #[cfg(target_os = "linux")]
 fn test_format_created_time() {
     let args = ["-c", "%w", "/boot"];
-    let actual = new_ucmd!().args(&args).run().stdout;
+    let actual = new_ucmd!().args(&args).succeeds().stdout_move_str();
     let expect = expected_result(&args);
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -240,7 +240,7 @@ fn test_format_created_time() {
 #[cfg(target_os = "linux")]
 fn test_format_created_seconds() {
     let args = ["-c", "%W", "/boot"];
-    let actual = new_ucmd!().args(&args).run().stdout;
+    let actual = new_ucmd!().args(&args).succeeds().stdout_move_str();
     let expect = expected_result(&args);
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
@@ -337,5 +337,5 @@ fn expected_result(args: &[&str]) -> String {
         .env("LANGUAGE", "C")
         .args(args)
         .run()
-        .stdout
+        .stdout_move_str()
 }

tests/by-util/test_sync.rs

@@ -5,8 +5,7 @@ use tempfile::tempdir;
 
 #[test]
 fn test_sync_default() {
-    let result = new_ucmd!().run();
+    new_ucmd!().succeeds();
-    assert!(result.success);
 }
 
 #[test]
@@ -18,8 +17,10 @@ fn test_sync_incorrect_arg() {
 fn test_sync_fs() {
     let temporary_directory = tempdir().unwrap();
     let temporary_path = fs::canonicalize(temporary_directory.path()).unwrap();
-    let result = new_ucmd!().arg("--file-system").arg(&temporary_path).run();
+    new_ucmd!()
-    assert!(result.success);
+        .arg("--file-system")
+        .arg(&temporary_path)
+        .succeeds();
 }
 
 #[test]
@@ -27,12 +28,14 @@ fn test_sync_data() {
     // Todo add a second arg
     let temporary_directory = tempdir().unwrap();
     let temporary_path = fs::canonicalize(temporary_directory.path()).unwrap();
-    let result = new_ucmd!().arg("--data").arg(&temporary_path).run();
+    new_ucmd!().arg("--data").arg(&temporary_path).succeeds();
-    assert!(result.success);
 }
 
 #[test]
 fn test_sync_no_existing_files() {
-    let result = new_ucmd!().arg("--data").arg("do-no-exist").fails();
+    new_ucmd!()
-    assert!(result.stderr.contains("error: cannot stat"));
+        .arg("--data")
+        .arg("do-no-exist")
+        .fails()
+        .stderr_contains("error: cannot stat");
 }

tests/by-util/test_tac.rs

@@ -52,18 +52,19 @@ fn test_single_non_newline_separator_before() {
 
 #[test]
 fn test_invalid_input() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
 
-    ucmd.arg("b")
+    scene
-        .run()
+        .ucmd()
-        .stderr
+        .arg("b")
-        .contains("tac: error: failed to open 'b' for reading");
+        .fails()
-
+        .stderr_contains("failed to open 'b' for reading: No such file or directory");
-    let (at, mut ucmd) = at_and_ucmd!();
 
     at.mkdir("a");
-    ucmd.arg("a")
+    scene
-        .run()
+        .ucmd()
-        .stderr
+        .arg("a")
-        .contains("tac: error: failed to read 'a'");
+        .fails()
+        .stderr_contains("dir: read error: Invalid argument");
 }

tests/by-util/test_tail.rs

@@ -226,8 +226,8 @@ fn test_bytes_big() {
         .arg(FILE)
         .arg("-c")
         .arg(format!("{}", N_ARG))
-        .run()
+        .succeeds()
-        .stdout;
+        .stdout_move_str();
     let expected = at.read(EXPECTED_FILE);
 
     assert_eq!(result.len(), expected.len());
@@ -340,6 +340,15 @@ fn test_negative_indexing() {
 
     let negative_bytes_index = new_ucmd!().arg("-c").arg("-20").arg(FOOBAR_TXT).run();
 
-    assert_eq!(positive_lines_index.stdout, negative_lines_index.stdout);
+    assert_eq!(positive_lines_index.stdout(), negative_lines_index.stdout());
-    assert_eq!(positive_bytes_index.stdout, negative_bytes_index.stdout);
+    assert_eq!(positive_bytes_index.stdout(), negative_bytes_index.stdout());
+}
+
+#[test]
+fn test_sleep_interval() {
+    new_ucmd!()
+        .arg("-s")
+        .arg("10")
+        .arg(FOOBAR_TXT)
+        .succeeds();
 }

tests/by-util/test_touch.rs

@@ -367,7 +367,58 @@ fn test_touch_mtime_dst_succeeds() {
 
     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);
 }
+
+// is_dst_switch_hour returns true if timespec ts is just before the switch
+// to Daylight Saving Time.
+// For example, in EST (UTC-5), Timespec { sec: 1583647200, nsec: 0 }
+// for March 8 2020 01:00:00 AM
+// is just before the switch because on that day clock jumps by 1 hour,
+// so 1 minute after 01:59:00 is 03:00:00.
+fn is_dst_switch_hour(ts: time::Timespec) -> bool {
+    let ts_after = ts + time::Duration::hours(1);
+    let tm = time::at(ts);
+    let tm_after = time::at(ts_after);
+    tm_after.tm_hour == tm.tm_hour + 2
+}
+
+// get_dstswitch_hour returns date string for which touch -m -t fails.
+// For example, in EST (UTC-5), that will be "202003080200" so
+// touch -m -t 202003080200 somefile
+// fails (that date/time does not exist).
+// In other locales it will be a different date/time, and in some locales
+// it doesn't exist at all, in which case this function will return None.
+fn get_dstswitch_hour() -> Option<String> {
+    let now = time::now();
+    // Start from January 1, 2020, 00:00.
+    let mut tm = time::strptime("20200101-0000", "%Y%m%d-%H%M").unwrap();
+    tm.tm_isdst = -1;
+    tm.tm_utcoff = now.tm_utcoff;
+    let mut ts = tm.to_timespec();
+    // Loop through all hours in year 2020 until we find the hour just
+    // before the switch to DST.
+    for _i in 0..(366 * 24) {
+        if is_dst_switch_hour(ts) {
+            let mut tm = time::at(ts);
+            tm.tm_hour = tm.tm_hour + 1;
+            let s = time::strftime("%Y%m%d%H%M", &tm).unwrap().to_string();
+            return Some(s);
+        }
+        ts = ts + time::Duration::hours(1);
+    }
+    None
+}
+
+#[test]
+fn test_touch_mtime_dst_fails() {
+    let (_at, mut ucmd) = at_and_ucmd!();
+    let file = "test_touch_set_mtime_dst_fails";
+
+    match get_dstswitch_hour() {
+        Some(s) => {
+            ucmd.args(&["-m", "-t", &s, file]).fails();
+        }
+        None => (),
+    }
+}

tests/by-util/test_tr.rs

@@ -120,19 +120,15 @@ fn test_truncate_with_set1_shorter_than_set2() {
 #[test]
 fn missing_args_fails() {
     let (_, mut ucmd) = at_and_ucmd!();
-    let result = ucmd.run();
+    ucmd.fails().stderr_contains("missing operand");
-
-    assert!(!result.success);
-    assert!(result.stderr.contains("missing operand"));
 }
 
 #[test]
 fn missing_required_second_arg_fails() {
     let (_, mut ucmd) = at_and_ucmd!();
-    let result = ucmd.args(&["foo"]).run();
+    ucmd.args(&["foo"])
-
+        .fails()
-    assert!(!result.success);
+        .stderr_contains("missing operand after");
-    assert!(result.stderr.contains("missing operand after"));
 }
 
 #[test]

tests/by-util/test_truncate.rs

@@ -53,6 +53,16 @@ fn test_decrease_file_size() {
     assert!(file.seek(SeekFrom::Current(0)).unwrap() == 6);
 }
 
+#[test]
+fn test_space_in_size() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let mut file = at.make_file(TFILE2);
+    file.write_all(b"1234567890").unwrap();
+    ucmd.args(&["--size", " 4", TFILE2]).succeeds();
+    file.seek(SeekFrom::End(0)).unwrap();
+    assert!(file.seek(SeekFrom::Current(0)).unwrap() == 4);
+}
+
 #[test]
 fn test_failed() {
     new_ucmd!().fails();
@@ -69,3 +79,4 @@ fn test_failed_incorrect_arg() {
     let (_at, mut ucmd) = at_and_ucmd!();
     ucmd.args(&["-s", "+5A", TFILE1]).fails();
 }
+

tests/by-util/test_tsort.rs

@@ -18,33 +18,35 @@ fn test_sort_self_loop() {
 
 #[test]
 fn test_no_such_file() {
-    let result = new_ucmd!().arg("invalid_file_txt").run();
+    new_ucmd!()
-
+        .arg("invalid_file_txt")
-    assert_eq!(true, result.stderr.contains("No such file or directory"));
+        .fails()
+        .stderr_contains("No such file or directory");
 }
 
 #[test]
 fn test_version_flag() {
-    let version_short = new_ucmd!().arg("-V").run();
+    let version_short = new_ucmd!().arg("-V").succeeds();
-    let version_long = new_ucmd!().arg("--version").run();
+    let version_long = new_ucmd!().arg("--version").succeeds();
 
-    assert_eq!(version_short.stdout, version_long.stdout);
+    assert_eq!(version_short.stdout_str(), version_long.stdout_str());
 }
 
 #[test]
 fn test_help_flag() {
-    let help_short = new_ucmd!().arg("-h").run();
+    let help_short = new_ucmd!().arg("-h").succeeds();
-    let help_long = new_ucmd!().arg("--help").run();
+    let help_long = new_ucmd!().arg("--help").succeeds();
 
-    assert_eq!(help_short.stdout, help_long.stdout);
+    assert_eq!(help_short.stdout_str(), help_long.stdout_str());
 }
 
 #[test]
 fn test_multiple_arguments() {
-    let result = new_ucmd!()
+    new_ucmd!()
         .arg("call_graph.txt")
-        .arg("invalid_file.txt")
+        .arg("invalid_file")
-        .run();
+        .fails()
-
+        .stderr_contains(
-    assert_eq!(true, result.stderr.contains("error: Found argument 'invalid_file.txt' which wasn't expected, or isn't valid in this context"))
+            "Found argument 'invalid_file' which wasn't expected, or isn't valid in this context",
+        );
 }

tests/by-util/test_uname.rs

@@ -2,60 +2,41 @@ use crate::common::util::*;
 
 #[test]
 fn test_uname_compatible() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-a").succeeds();
-
-    let result = ucmd.arg("-a").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_uname_name() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-n").succeeds();
-
-    let result = ucmd.arg("-n").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_uname_processor() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let result = new_ucmd!().arg("-p").succeeds();
-
+    assert_eq!(result.stdout_str().trim_end(), "unknown");
-    let result = ucmd.arg("-p").run();
-    assert!(result.success);
-    assert_eq!(result.stdout.trim_end(), "unknown");
 }
 
 #[test]
 fn test_uname_hwplatform() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let result = new_ucmd!().arg("-i").succeeds();
-
+    assert_eq!(result.stdout_str().trim_end(), "unknown");
-    let result = ucmd.arg("-i").run();
-    assert!(result.success);
-    assert_eq!(result.stdout.trim_end(), "unknown");
 }
 
 #[test]
 fn test_uname_machine() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-m").succeeds();
-
-    let result = ucmd.arg("-m").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_uname_kernel_version() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("-v").succeeds();
-
-    let result = ucmd.arg("-v").run();
-    assert!(result.success);
 }
 
 #[test]
 fn test_uname_kernel() {
     let (_, mut ucmd) = at_and_ucmd!();
 
-    let result = ucmd.arg("-o").run();
+    let result = ucmd.arg("-o").succeeds();
-    assert!(result.success);
     #[cfg(target_os = "linux")]
-    assert!(result.stdout.to_lowercase().contains("linux"));
+    assert!(result.stdout_str().to_lowercase().contains("linux"));
 }

tests/by-util/test_uptime.rs

@@ -4,33 +4,23 @@ use crate::common::util::*;
 
 #[test]
 fn test_uptime() {
-    let result = TestScenario::new(util_name!()).ucmd_keepenv().run();
+    TestScenario::new(util_name!())
+        .ucmd_keepenv()
+        .succeeds()
+        .stdout_contains("load average:")
+        .stdout_contains(" up ");
 
-    println!("stdout = {}", result.stdout);
-    println!("stderr = {}", result.stderr);
-
-    assert!(result.success);
-    assert!(result.stdout.contains("load average:"));
-    assert!(result.stdout.contains(" up "));
     // Don't check for users as it doesn't show in some CI
 }
 
 #[test]
 fn test_uptime_since() {
-    let scene = TestScenario::new(util_name!());
-
-    let result = scene.ucmd().arg("--since").succeeds();
-
-    println!("stdout = {}", result.stdout);
-    println!("stderr = {}", result.stderr);
-
-    assert!(result.success);
     let re = Regex::new(r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}").unwrap();
-    assert!(re.is_match(&result.stdout.trim()));
+
+    new_ucmd!().arg("--since").succeeds().stdout_matches(&re);
 }
 
 #[test]
 fn test_failed() {
-    let (_at, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().arg("willfail").fails();
-    ucmd.arg("willfail").fails();
 }

tests/by-util/test_users.rs

@@ -3,14 +3,11 @@ use std::env;
 
 #[test]
 fn test_users_noarg() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    new_ucmd!().succeeds();
-    let result = ucmd.run();
-    assert!(result.success);
 }
 #[test]
 fn test_users_check_name() {
-    let result = TestScenario::new(util_name!()).ucmd_keepenv().run();
+    let result = TestScenario::new(util_name!()).ucmd_keepenv().succeeds();
-    assert!(result.success);
 
     // Expectation: USER is often set
     let key = "USER";
@@ -21,9 +18,9 @@ fn test_users_check_name() {
         // Check if "users" contains the name of the user
         {
             println!("username found {}", &username);
-            println!("result.stdout {}", &result.stdout);
+            // println!("result.stdout {}", &result.stdout);
-            if !&result.stdout.is_empty() {
+            if !result.stdout_str().is_empty() {
-                assert!(result.stdout.contains(&username))
+                result.stdout_contains(&username);
             }
         }
     }

tests/by-util/test_who.rs

@@ -23,7 +23,7 @@ fn test_heading() {
     for opt in vec!["-H"] {
         // allow whitespace variation
         // * minor whitespace differences occur between platform built-in outputs; specifically number of TABs between "TIME" and "COMMENT" may be variant
-        let actual = new_ucmd!().arg(opt).run().stdout;
+        let actual = new_ucmd!().arg(opt).run().stdout_move_str();
         let expect = expected_result(opt);
         println!("actual: {:?}", actual);
         println!("expect: {:?}", expect);
@@ -80,5 +80,5 @@ fn expected_result(arg: &str) -> String {
         .env("LANGUAGE", "C")
         .args(&[arg])
         .run()
-        .stdout
+        .stdout_move_str()
 }

tests/by-util/test_whoami.rs

@@ -1,50 +1,63 @@
 use crate::common::util::*;
-use std::env;
+
+// Apparently some CI environments have configuration issues, e.g. with 'whoami' and 'id'.
+// If we are running inside the CI and "needle" is in "stderr" skipping this test is
+// considered okay. If we are not inside the CI this calls assert!(result.success).
+//
+// From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)"
+// stderr: "whoami: error: failed to get username"
+// Maybe: "adduser --uid 1001 username" can put things right?
+fn skipping_test_is_okay(result: &CmdResult, needle: &str) -> bool {
+    if !result.succeeded() {
+        println!("result.stdout = {}", result.stdout_str());
+        println!("result.stderr = {}", result.stderr_str());
+        if is_ci() && result.stderr_str().contains(needle) {
+            println!("test skipped:");
+            return true;
+        } else {
+            result.success();
+        }
+    }
+    false
+}
 
 #[test]
 fn test_normal() {
     let (_, mut ucmd) = at_and_ucmd!();
 
     let result = ucmd.run();
-    println!("result.stdout = {}", result.stdout);
-    println!("result.stderr = {}", result.stderr);
-    println!("env::var(CI).is_ok() = {}", env::var("CI").is_ok());
 
-    for (key, value) in env::vars() {
+    // use std::env;
-        println!("{}: {}", key, value);
+    // println!("env::var(CI).is_ok() = {}", env::var("CI").is_ok());
-    }
+    // for (key, value) in env::vars() {
-    if is_ci() && result.stderr.contains("failed to get username") {
+    //     println!("{}: {}", key, value);
-        // In the CI, some server are failing to return whoami.
+    // }
-        // As seems to be a configuration issue, ignoring it
+
+    if skipping_test_is_okay(&result, "failed to get username") {
         return;
     }
 
-    assert!(result.success);
+    result.no_stderr();
-    assert!(!result.stdout.trim().is_empty());
+    assert!(!result.stdout_str().trim().is_empty());
 }
 
 #[test]
 #[cfg(not(windows))]
 fn test_normal_compare_id() {
-    let (_, mut ucmd) = at_and_ucmd!();
+    let scene = TestScenario::new(util_name!());
 
-    let result = ucmd.run();
+    let result_ucmd = scene.ucmd().run();
-
+    if skipping_test_is_okay(&result_ucmd, "failed to get username") {
-    println!("result.stdout = {}", result.stdout);
-    println!("result.stderr = {}", result.stderr);
-    if is_ci() && result.stderr.contains("failed to get username") {
-        // In the CI, some server are failing to return whoami.
-        // As seems to be a configuration issue, ignoring it
         return;
     }
-    assert!(result.success);
-    let ts = TestScenario::new("id");
-    let id = ts.cmd("id").arg("-un").run();
 
-    if is_ci() && id.stderr.contains("cannot find name for user ID") {
+    let result_cmd = scene.cmd("id").arg("-un").run();
-        // In the CI, some server are failing to return whoami.
+    if skipping_test_is_okay(&result_cmd, "cannot find name for user ID") {
-        // As seems to be a configuration issue, ignoring it
         return;
     }
-    assert_eq!(result.stdout.trim(), id.stdout.trim());
+
+    assert_eq!(
+        result_ucmd.stdout_str().trim(),
+        result_cmd.stdout_str().trim()
+    );
 }

tests/common/util.rs

@@ -55,14 +55,14 @@ pub struct CmdResult {
     //tmpd is used for convenience functions for asserts against fixtures
     tmpd: Option<Rc<TempDir>>,
     /// exit status for command (if there is one)
-    pub code: Option<i32>,
+    code: Option<i32>,
     /// zero-exit from running the Command?
     /// see [`success`]
-    pub success: bool,
+    success: bool,
     /// captured standard output after running the Command
-    pub stdout: String,
+    stdout: String,
     /// captured standard error after running the Command
-    pub stderr: String,
+    stderr: String,
 }
 
 impl CmdResult {
@@ -204,6 +204,13 @@ impl CmdResult {
         self
     }
 
+    /// Like `stdout_is` but newlines are normalized to `\n`.
+    pub fn normalized_newlines_stdout_is<T: AsRef<str>>(&self, msg: T) -> &CmdResult {
+        let msg = msg.as_ref().replace("\r\n", "\n");
+        assert_eq!(self.stdout.replace("\r\n", "\n"), msg);
+        self
+    }
+
     /// asserts that the command resulted in stdout stream output,
     /// whose bytes equal those of the passed in slice
     pub fn stdout_is_bytes<T: AsRef<[u8]>>(&self, msg: T) -> &CmdResult {
@@ -306,14 +313,14 @@ impl CmdResult {
     }
 
     pub fn stdout_matches(&self, regex: &regex::Regex) -> &CmdResult {
-        if !regex.is_match(self.stdout_str()) {
+        if !regex.is_match(self.stdout_str().trim()) {
             panic!("Stdout does not match regex:\n{}", self.stdout_str())
         }
         self
     }
 
     pub fn stdout_does_not_match(&self, regex: &regex::Regex) -> &CmdResult {
-        if regex.is_match(self.stdout_str()) {
+        if regex.is_match(self.stdout_str().trim()) {
             panic!("Stdout matches regex:\n{}", self.stdout_str())
         }
         self
@@ -673,8 +680,11 @@ pub struct UCommand {
     comm_string: String,
     tmpd: Option<Rc<TempDir>>,
     has_run: bool,
-    stdin: Option<Vec<u8>>,
     ignore_stdin_write_error: bool,
+    stdin: Option<Stdio>,
+    stdout: Option<Stdio>,
+    stderr: Option<Stdio>,
+    bytes_into_stdin: Option<Vec<u8>>,
 }
 
 impl UCommand {
@@ -703,8 +713,11 @@ impl UCommand {
                 cmd
             },
             comm_string: String::from(arg.as_ref().to_str().unwrap()),
-            stdin: None,
             ignore_stdin_write_error: false,
+            bytes_into_stdin: None,
+            stdin: None,
+            stdout: None,
+            stderr: None,
         }
     }
 
@@ -715,6 +728,21 @@ impl UCommand {
         ucmd
     }
 
+    pub fn set_stdin<T: Into<Stdio>>(&mut self, stdin: T) -> &mut UCommand {
+        self.stdin = Some(stdin.into());
+        self
+    }
+
+    pub fn set_stdout<T: Into<Stdio>>(&mut self, stdout: T) -> &mut UCommand {
+        self.stdout = Some(stdout.into());
+        self
+    }
+
+    pub fn set_stderr<T: Into<Stdio>>(&mut self, stderr: T) -> &mut UCommand {
+        self.stderr = Some(stderr.into());
+        self
+    }
+
     /// Add a parameter to the invocation. Path arguments are treated relative
     /// to the test environment directory.
     pub fn arg<S: AsRef<OsStr>>(&mut self, arg: S) -> &mut UCommand {
@@ -744,10 +772,10 @@ impl UCommand {
 
     /// provides stdinput to feed in to the command when spawned
     pub fn pipe_in<T: Into<Vec<u8>>>(&mut self, input: T) -> &mut UCommand {
-        if self.stdin.is_some() {
+        if self.bytes_into_stdin.is_some() {
             panic!("{}", MULTIPLE_STDIN_MEANINGLESS);
         }
-        self.stdin = Some(input.into());
+        self.bytes_into_stdin = Some(input.into());
         self
     }
 
@@ -761,7 +789,7 @@ impl UCommand {
     /// 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() {
+        if self.bytes_into_stdin.is_none() {
             panic!("{}", NO_STDIN_MEANINGLESS);
         }
         self.ignore_stdin_write_error = true;
@@ -790,13 +818,13 @@ impl UCommand {
         log_info("run", &self.comm_string);
         let mut child = self
             .raw
-            .stdin(Stdio::piped())
+            .stdin(self.stdin.take().unwrap_or_else(|| Stdio::piped()))
-            .stdout(Stdio::piped())
+            .stdout(self.stdout.take().unwrap_or_else(|| Stdio::piped()))
-            .stderr(Stdio::piped())
+            .stderr(self.stderr.take().unwrap_or_else(|| Stdio::piped()))
             .spawn()
             .unwrap();
 
-        if let Some(ref input) = self.stdin {
+        if let Some(ref input) = self.bytes_into_stdin {
             let write_result = child
                 .stdin
                 .take()
@@ -1080,4 +1108,33 @@ mod tests {
 
         res.stdout_does_not_match(&positive);
     }
+
+    #[test]
+    fn test_normalized_newlines_stdout_is() {
+        let res = CmdResult {
+            tmpd: None,
+            code: None,
+            success: true,
+            stdout: "A\r\nB\nC".into(),
+            stderr: "".into(),
+        };
+
+        res.normalized_newlines_stdout_is("A\r\nB\nC");
+        res.normalized_newlines_stdout_is("A\nB\nC");
+        res.normalized_newlines_stdout_is("A\nB\r\nC");
+    }
+
+    #[test]
+    #[should_panic]
+    fn test_normalized_newlines_stdout_is_fail() {
+        let res = CmdResult {
+            tmpd: None,
+            code: None,
+            success: true,
+            stdout: "A\r\nB\nC".into(),
+            stderr: "".into(),
+        };
+
+        res.normalized_newlines_stdout_is("A\r\nB\nC\n");
+    }
 }

tests/fixtures/sort/multiple_decimals_numeric.expected

@@ -0,0 +1,35 @@
+-2028789030
+-896689
+-8.90880
+-1
+-.05
+
+
+
+
+
+
+
+
+000
+CARAvan
+00000001
+1
+1.040000000
+1.444
+1.58590
+8.013
+45
+46.89
+										4567..457
+               4567.
+4567.1
+4567.34
+				37800
+						45670.89079.098
+						45670.89079.1
+576,446.88800000
+576,446.890
+4798908.340000000000
+4798908.45
+4798908.8909800