diff --git a/.github/workflows/GNU.yml b/.github/workflows/GNU.yml index 35efccbe5..a68f0a083 100644 --- a/.github/workflows/GNU.yml +++ b/.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 diff --git a/Cargo.lock b/Cargo.lock index 461716b1b..7ae0d078f 100644 --- a/Cargo.lock +++ b/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", ] diff --git a/README.md b/README.md index 76ea92ab5..12dfb5609 100644 --- a/README.md +++ b/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: diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index 7d56a7485..e507c5acd 100644 --- a/src/uu/cat/src/cat.rs +++ b/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(handle: &mut InputHandle) -> 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(handle: &mut InputHandle) -> 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(handle: &mut InputHandle, writer: RawFd) -> CatResult { - 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( diff --git a/src/uu/cat/src/splice.rs b/src/uu/cat/src/splice.rs new file mode 100644 index 000000000..ccc625467 --- /dev/null +++ b/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( + handle: &mut InputHandle, + write_fd: RawFd, +) -> CatResult { + 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(()) +} diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 0e3273b3b..23fb030ba 100644 --- a/src/uu/chown/src/chown.rs +++ b/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, Option), String> { - let args = spec.split(':').collect::>(); - let usr_only = args.len() == 1; - let grp_only = args.len() == 2 && args[0].is_empty() && !args[1].is_empty(); + let args = spec.split_terminator(':').collect::>(); + let usr_only = args.len() == 1 && !args[0].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, Option), 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 { diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 4e245b298..da8c9037e 100644 --- a/src/uu/cp/src/cp.rs +++ b/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, 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()); diff --git a/src/uu/du/src/du.rs b/src/uu/du/src/du.rs index 07635881a..fa3b3c80a 100644 --- a/src/uu/du/src/du.rs +++ b/src/uu/du/src/du.rs @@ -15,7 +15,7 @@ use chrono::Local; use std::collections::HashSet; use std::env; use std::fs; -use std::io::{stderr, Result, Write}; +use std::io::{stderr, ErrorKind, Result, Write}; use std::iter; #[cfg(not(windows))] use std::os::unix::fs::MetadataExt; @@ -296,7 +296,21 @@ fn du( } } } - Err(error) => show_error!("{}", error), + Err(error) => match error.kind() { + ErrorKind::PermissionDenied => { + let description = format!( + "cannot access '{}'", + entry + .path() + .as_os_str() + .to_str() + .unwrap_or("") + ); + let error_message = "Permission denied"; + show_error_custom_description!(description, "{}", error_message) + } + _ => show_error!("{}", error), + }, }, Err(error) => show_error!("{}", error), } @@ -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() }; diff --git a/src/uu/install/src/install.rs b/src/uu/install/src/install.rs index a75ce45be..4ce665b80 100644 --- a/src/uu/install/src/install.rs +++ b/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 { .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, b: Behavior) -> i32 { .iter() .map(PathBuf::from) .collect::>(); + let target = Path::new(paths.last().unwrap()); - if (target.is_file() || is_new_file_path(target)) && sources.len() == 1 { - copy_file_to_file(&sources[0], &target.to_path_buf(), &b) - } else { + if sources.len() > 1 || (target.exists() && target.is_dir()) { 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 + } } } diff --git a/src/uu/ls/BENCHMARKING.md b/src/uu/ls/BENCHMARKING.md new file mode 100644 index 000000000..84a0c3d84 --- /dev/null +++ b/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` diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml index dacdc7cd9..9fa06c6a6 100644 --- a/src/uu/ls/Cargo.toml +++ b/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" diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index aebaa6b44..3feee0f07 100644 --- a/src/uu/ls/src/ls.rs +++ b/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::{DirEntry, FileType, Metadata}; -#[cfg(unix)] -use std::os::unix::fs::FileTypeExt; +use std::fs::{self, DirEntry, FileType, Metadata}; #[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; -#[cfg(unix)] -use uucore::libc::{mode_t, S_ISGID, S_ISUID, S_ISVTX, S_IWOTH, S_IXGRP, S_IXOTH, S_IXUSR}; +use uucore::libc::{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: bool, + color: Option, long: LongFormat, width: Option, quoting_style: QuotingStyle, @@ -326,8 +308,7 @@ impl Config { Time::Modification }; - #[cfg(unix)] - let color = match options.value_of(options::COLOR) { + let needs_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 got from symlink_metadata() or metadata() based on config + md: std::io::Result, + // 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, config: Config) -> i32 { let number_of_locs = locs.len(); - let mut files = Vec::::new(); - let mut dirs = Vec::::new(); + let mut files = Vec::::new(); + let mut dirs = Vec::::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, config: Config) -> i32 { has_failed = true; continue; } - let mut dir = false; - if p.is_dir() && !config.directory { - dir = true; - if config.format == Format::Long && !config.dereference { - if let Ok(md) = p.symlink_metadata() { - if md.file_type().is_symlink() && !p.ends_with("/") { - dir = false; - } - } - } - } - if dir { - dirs.push(p); + let path_data = PathData::new(p, &config, true); + + let show_dir_contents = if let Ok(md) = path_data.md.as_ref() { + !config.directory && md.is_dir() } 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, 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, config: Config) -> i32 { } } -fn sort_entries(entries: &mut Vec, config: &Config) { +fn sort_entries(entries: &mut Vec, 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_by_key(|k| Reverse(get_metadata(k, config).map(|md| md.len()).unwrap_or(0))), + Sort::Size => { + 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::Version => entries.sort_by(|a, b| version_cmp::version_cmp(a, b)), + Sort::Name => entries.sort_by_key(|k| k.file_name.to_lowercase()), + 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) { - let mut entries: Vec<_> = safe_unwrap!(fs::read_dir(dir).and_then(Iterator::collect)); - - entries.retain(|e| should_display(e, config)); - - 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); +fn enter_directory(dir: &PathData, config: &Config) { + let mut entries: Vec<_> = if config.files == Files::All { + vec![ + PathData::new(dir.p_buf.join("."), config, false), + PathData::new(dir.p_buf.join(".."), config, false), + ] } 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()) { - println!("\n{}:", e.to_string_lossy()); + for e in entries + .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 { - if config.dereference { +fn get_metadata(entry: &Path, config: &Config, command_line: bool) -> std::io::Result { + 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) { - if let Ok(md) = get_metadata(entry, config) { +fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize) { + 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, 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> = 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> = 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))] -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); - 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)] +fn file_is_executable(md: &Metadata) -> bool { + // Mode always returns u32, but the flags might not be, based on the platform + // e.g. linux has u32, mac has u16. + // S_IXUSR -> user has execute permission + // S_IXGRP -> group has execute persmission + // S_IXOTH -> other users have execute permission + md.mode() & ((S_IXUSR | S_IXGRP | S_IXOTH) as u32) != 0 } -#[cfg(unix)] -fn color_name(name: String, typ: &str) -> String { - let mut typ = typ; - if !COLOR_MAP.contains_key(typ) { - if typ == "or" { - typ = "ln"; - } else if typ == "mi" { - typ = "fi"; - } - }; - if let Some(code) = COLOR_MAP.get(typ) { - format!( - "{}{}{}{}{}{}{}{}", - *LEFT_CODE, code, *RIGHT_CODE, name, *END_CODE, *LEFT_CODE, *RESET_CODE, *RIGHT_CODE, - ) +#[allow(clippy::clippy::collapsible_else_if)] +fn classify_file(md: &Metadata) -> Option { + let file_type = md.file_type(); + + if file_type.is_dir() { + Some('/') + } else if file_type.is_symlink() { + Some('@') } else { - name - } -} - -#[cfg(unix)] -macro_rules! has { - ($mode:expr, $perm:expr) => { - $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) { + #[cfg(unix)] + { + if file_type.is_socket() { + Some('=') + } else if file_type.is_fifo() { + Some('|') + } else if file_type.is_file() && file_is_executable(&md) { 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 - let code = if target.exists() { "fi" } else { "mi" }; - let target_name = color_name(target.to_string_lossy().to_string(), code); + // We don't bother updating width here because it's not used for long name.push_str(" -> "); - name.push_str(&target_name); + name.push_str(&target.to_string_lossy()); } } - Cell { - contents: name, - width, + name.into() +} + +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, } } diff --git a/src/uu/ls/src/quoting_style.rs b/src/uu/ls/src/quoting_style.rs index ceb54466c..49456fc22 100644 --- a/src/uu/ls/src/quoting_style.rs +++ b/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"), + ], + ); + } } diff --git a/src/uu/printf/src/cli.rs b/src/uu/printf/src/cli.rs index 12e80a925..a5e9c9775 100644 --- a/src/uu/printf/src/cli.rs +++ b/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); - stdout().flush(); + let _ = stdout().write(bslice); + let _ = stdout().flush(); } diff --git a/src/uu/printf/src/printf.rs b/src/uu/printf/src/printf.rs index c2952e5a9..d947a7d83 100644 --- a/src/uu/printf/src/printf.rs +++ b/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]... diff --git a/src/uu/printf/src/tokenize/num_format/formatters/base_conv/mod.rs b/src/uu/printf/src/tokenize/num_format/formatters/base_conv/mod.rs index 04d33b52c..82971df3e 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/base_conv/mod.rs +++ b/src/uu/printf/src/tokenize/num_format/formatters/base_conv/mod.rs @@ -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 = ret_rev.iter().rev().map(|x| *x).collect(); + let ret: Vec = ret_rev.into_iter().rev().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, -// 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::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 { 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 = ret_rev.iter().rev().map(|x| *x).collect(); + let ret: Vec = ret_rev.into_iter().rev().collect(); ret } @@ -219,8 +153,7 @@ pub fn unsigned_to_arrnum(src: u16) -> Vec { } // 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, diff --git a/src/uu/printf/src/tokenize/num_format/formatters/cninetyninehexfloatf.rs b/src/uu/printf/src/tokenize/num_format/formatters/cninetyninehexfloatf.rs index 10e58cc32..870e64712 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/cninetyninehexfloatf.rs +++ b/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, - analysis: &FloatAnalysis, - last_dec_place: usize, + _str_in: &str, + _analysis: &FloatAnalysis, + _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 diff --git a/src/uu/printf/src/tokenize/num_format/formatters/decf.rs b/src/uu/printf/src/tokenize/num_format/formatters/decf.rs index 6b2baa890..448771f22 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/decf.rs +++ b/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 { - as_num: f64, -} +pub struct Decf; + impl Decf { pub fn new() -> Decf { - Decf { as_num: 0.0 } + Decf } } impl Formatter for Decf { diff --git a/src/uu/printf/src/tokenize/num_format/formatters/floatf.rs b/src/uu/printf/src/tokenize/num_format/formatters/floatf.rs index 97ceafe8d..b3de2f98a 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/floatf.rs +++ b/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 { - as_num: f64, -} +pub struct Floatf; impl Floatf { pub fn new() -> Floatf { - Floatf { as_num: 0.0 } + Floatf } } impl Formatter for Floatf { diff --git a/src/uu/printf/src/tokenize/num_format/formatters/intf.rs b/src/uu/printf/src/tokenize/num_format/formatters/intf.rs index 9231bd027..2e4e67047 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/intf.rs +++ b/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 diff --git a/src/uu/printf/src/tokenize/num_format/formatters/scif.rs b/src/uu/printf/src/tokenize/num_format/formatters/scif.rs index 69a703042..ebac1565e 100644 --- a/src/uu/printf/src/tokenize/num_format/formatters/scif.rs +++ b/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 { - as_num: f64, -} +pub struct Scif; + impl Scif { pub fn new() -> Scif { - Scif { as_num: 0.0 } + Scif } } impl Formatter for Scif { diff --git a/src/uu/printf/src/tokenize/unescaped_text.rs b/src/uu/printf/src/tokenize/unescaped_text.rs index 3b9f0123e..084014ae9 100644 --- a/src/uu/printf/src/tokenize/unescaped_text.rs +++ b/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, - args: &mut Peekable>, + _: &mut Peekable>, ) -> Option> { UnescapedText::from_it_core(it, false) } } -#[allow(unused_variables)] impl token::Token for UnescapedText { - fn print(&self, pf_args_it: &mut Peekable>) { + fn print(&self, _: &mut Peekable>) { cli::flush_bytes(&self.0[..]); } } diff --git a/src/uu/sort/BENCHMARKING.md b/src/uu/sort/BENCHMARKING.md index b20db014d..1caea0326 100644 --- a/src/uu/sort/BENCHMARKING.md +++ b/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. -- 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"`. + +- Get a wordlist, for example with [words]() 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. +- 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: +
+ Rust script + + ## 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` + +
+ +- 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. \ No newline at end of file + +- 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`. + +
+ Example output + + Command being timed: "target/release/coreutils sort shuffled_numbers.txt" + User time (seconds): 0.10 + System time (seconds): 0.00 + Percent of CPU this job got: 365% + Elapsed (wall clock) time (h:mm:ss or m:ss): 0:00.02 + Average shared text size (kbytes): 0 + Average unshared data size (kbytes): 0 + Average stack size (kbytes): 0 + Average total size (kbytes): 0 + Maximum resident set size (kbytes): 25360 + Average resident set size (kbytes): 0 + Major (requiring I/O) page faults: 0 + Minor (reclaiming a frame) page faults: 5802 + Voluntary context switches: 462 + Involuntary context switches: 73 + Swaps: 0 + File system inputs: 1184 + File system outputs: 0 + Socket messages sent: 0 + Socket messages received: 0 + Signals delivered: 0 + Page size (bytes): 4096 + Exit status: 0 + +
+ +Useful metrics to look at could be: + +- User time +- Percent of CPU this job got +- Maximum resident set size diff --git a/src/uu/sort/src/numeric_str_cmp.rs b/src/uu/sort/src/numeric_str_cmp.rs new file mode 100644 index 000000000..a50734ebd --- /dev/null +++ b/src/uu/sort/src/numeric_str_cmp.rs @@ -0,0 +1,455 @@ +// * This file is part of the uutils coreutils package. +// * +// * (c) Michael Debertol +// * +// * 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, + pub decimal_pt: Option, +} + +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) { + 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 + ) + ); + } +} diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index 8bf6eb1e8..7515ca1c9 100644 --- a/src/uu/sort/src/sort.rs +++ b/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 Selection { +enum SelectionRange { /// 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), } +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) { + 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 { - Selection::String(string) => string.as_str(), - Selection::ByIndex(range) => &line.line[range.to_owned()], - } + self.range.get_str(&line.line) } } type Field = Range; -#[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()) { - if let Some(transformed) = - transform(&line[range.to_owned()], &selector.settings) - { - Selection::String(transformed) + let mut range = + if let Some(range) = selector.get_selection(&line, fields.as_deref()) { + if let Some(transformed) = + transform(&line[range.to_owned()], &selector.settings) + { + 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. - Selection::ByIndex(0..0) - } + NumCache::None + }; + Selection { range, num_cache } }) .collect(); Self { line, selections } @@ -284,20 +351,18 @@ fn tokenize_default(line: &str) -> Vec { /// Split between separators. These separators are not included in fields. fn tokenize_with_separator(line: &str, separator: char) -> Vec { - let mut tokens = vec![0..0]; - let mut previous_was_separator = false; - for (idx, char) in line.char_indices() { - if previous_was_separator { - tokens.push(idx..0); - } - if char == separator { - tokens.last_mut().unwrap().end = idx; - previous_was_separator = true; - } else { - previous_was_separator = false; - } + let mut tokens = vec![]; + let separator_indices = + line.char_indices() + .filter_map(|(i, c)| if c == separator { Some(i) } else { None }); + let mut start = 0; + for sep_idx in separator_indices { + tokens.push(start..sep_idx); + start = sep_idx + 1; + } + if start < line.len() { + 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, 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 b = b.selections[idx].get_str(b); + let a_selection = &a.selections[idx]; + 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 { - SortMode::Numeric => numeric_compare, - SortMode::GeneralNumeric => general_numeric_compare, - SortMode::HumanNumeric => human_numeric_size_compare, - SortMode::Month => month_compare, - SortMode::Version => version_compare, - SortMode::Default => default_compare, - })(a, b) + match settings.mode { + SortMode::Numeric | SortMode::HumanNumeric => numeric_str_cmp( + (a_str, a_selection.num_cache.as_num_info()), + (b_str, b_selection.num_cache.as_num_info()), + ), + SortMode::GeneralNumeric => general_numeric_compare( + a_selection.num_cache.as_f64(), + 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>>(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>>(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 { - 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 { + } else if a < b { 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]); + } } diff --git a/src/uu/stdbuf/src/stdbuf.rs b/src/uu/stdbuf/src/stdbuf.rs index 8c65a5c7e..ddbd76133 100644 --- a/src/uu/stdbuf/src/stdbuf.rs +++ b/src/uu/stdbuf/src/stdbuf.rs @@ -144,7 +144,9 @@ fn check_option(matches: &ArgMatches, name: &str) -> Result 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) } diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index 666ba3384..1fb6489da 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -91,10 +91,15 @@ fn tac(filenames: Vec, before: bool, _: bool, separator: &str) -> i32 { } else { let path = Path::new(filename); if path.is_dir() || path.metadata().is_err() { - show_error!( - "failed to open '{}' for reading: No such file or directory", - filename - ); + if path.is_dir() { + show_error!("dir: read error: Invalid argument"); + } else { + show_error!( + "failed to open '{}' for reading: No such file or directory", + filename + ); + } + exit_code = 1; continue; } match File::open(path) { diff --git a/src/uu/tail/src/tail.rs b/src/uu/tail/src/tail.rs index ffe27e26c..fec88e841 100644 --- a/src/uu/tail/src/tail.rs +++ b/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"), ) diff --git a/src/uu/truncate/src/truncate.rs b/src/uu/truncate/src/truncate.rs index 9cd5865b7..2c232a3d1 100644 --- a/src/uu/truncate/src/truncate.rs +++ b/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() { - number *= match size.chars().last().unwrap().to_ascii_uppercase() { - 'B' => match size + if clean_size.chars().last().unwrap().is_alphabetic() { + number *= match clean_size.chars().last().unwrap().to_ascii_uppercase() { + 'B' => match clean_size .chars() - .nth(size.len() - 2) + .nth(clean_size.len() - 2) .unwrap() .to_ascii_uppercase() { diff --git a/src/uu/unexpand/src/unexpand.rs b/src/uu/unexpand/src/unexpand.rs index 3d80bd6e9..a811d3b66 100644 --- a/src/uu/unexpand/src/unexpand.rs +++ b/src/uu/unexpand/src/unexpand.rs @@ -150,7 +150,7 @@ fn next_tabstop(tabstops: &[usize], col: usize) -> Option { } 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) } } diff --git a/src/uucore/src/lib/macros.rs b/src/uucore/src/lib/macros.rs index 24b392ebd..637e91f8f 100644 --- a/src/uucore/src/lib/macros.rs +++ b/src/uucore/src/lib/macros.rs @@ -31,6 +31,14 @@ macro_rules! show_error( ); /// Show a warning to stderr in a silimar style to GNU coreutils. +#[macro_export] +macro_rules! show_error_custom_description ( + ($err:expr,$($args:tt)+) => ({ + eprint!("{}: {}: ", executable!(), $err); + eprintln!($($args)+); + }) +); + #[macro_export] macro_rules! show_warning( ($($args:tt)+) => ({ diff --git a/tests/by-util/test_cat.rs b/tests/by-util/test_cat.rs index eb6cc9148..9f7ebdd37 100644 --- a/tests/by-util/test_cat.rs +++ b/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!() - .args(&[socket_path]) - .succeeds() - .stdout_only("a\tb"); + let child = new_ucmd!().args(&[socket_path]).run_no_wait(); + barrier.wait(); + let stdout = &child.wait_with_output().unwrap().stdout.clone(); + let output = String::from_utf8_lossy(&stdout); + assert_eq!("a\tb", output); thread.join().unwrap(); } diff --git a/tests/by-util/test_chmod.rs b/tests/by-util/test_chmod.rs index d60b8a50b..3958c0a36 100644 --- a/tests/by-util/test_chmod.rs +++ b/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") diff --git a/tests/by-util/test_chown.rs b/tests/by-util/test_chown.rs index e27fba3d4..3d94632a6 100644 --- a/tests/by-util/test_chown.rs +++ b/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") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") { return; } - println!("results {}", result.stdout_str()); - let username = result.stdout_str().trim_end(); - - let (at, mut ucmd) = at_and_ucmd!(); - let file1 = "test_install_target_dir_file_a1"; + let user_name = String::from(result.stdout_str().trim()); + assert!(!user_name.is_empty()); + 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] -fn test_chown_myself_second() { - // test chown username: file.txt - let scene = TestScenario::new(util_name!()); - let result = scene.cmd("whoami").run(); - if is_ci() && result.stderr_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() + ":") + // since only superuser can change owner, we have to change from ourself to ourself + let result = scene + .ucmd() + .arg(user_name) + .arg("--verbose") .arg(file1) .run(); + result.stderr_contains(&"retained as"); - println!("result.stdout = {}", result.stdout_str()); - println!("result.stderr = {}", result.stderr_str()); - assert!(result.success); + // try to change to another existing user, e.g. 'root' + scene + .ucmd() + .arg("root") + .arg("--verbose") + .arg(file1) + .fails() + .stderr_contains(&"failed to change"); } #[test] -fn test_chown_myself_group() { - // test chown username:group file.txt +fn test_chown_only_owner_colon() { + // 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") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") { return; } - println!("user name = {}", result.stdout_str()); - let username = result.stdout_str().trim_end(); + let user_name = String::from(result.stdout_str().trim()); + 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") { - // 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, "id: cannot find name for group ID") { return; } - println!("group name = {}", result.stdout_str()); - let group = result.stdout_str().trim_end(); + let group_name = String::from(result.stdout_str().trim()); + assert!(!group_name.is_empty()); - let (at, mut ucmd) = at_and_ucmd!(); - let file1 = "test_install_target_dir_file_a1"; - let perm = username.to_owned() + ":" + group; - at.touch(file1); - let result = ucmd.arg(perm).arg(file1).run(); - println!("result.stdout = {}", result.stdout_str()); - println!("result.stderr = {}", result.stderr_str()); - if is_ci() && result.stderr_str().contains("chown: invalid group:") { - // With some Ubuntu into the CI, we can get this answer + let result = scene + .ucmd() + .arg(format!("{}:{}", user_name, group_name)) + .arg("--verbose") + .arg(file1) + .run(); + if skipping_test_is_okay(&result, "chown: invalid group:") { 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") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") { 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_install_target_dir_file_a1"; - let perm = ":".to_owned() + result.stdout_str().trim_end(); + let file1 = "test_chown_file1"; 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") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + 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()); - println!("result.stderr = {}", result.stderr_str()); - let id = String::from(result.stdout_str().trim()); - - let (at, mut ucmd) = at_and_ucmd!(); - let file1 = "test_install_target_dir_file_a1"; + let user_id = String::from(result.stdout_str().trim()); + assert!(!user_id.is_empty()); + let file1 = "test_chown_file1"; at.touch(file1); - let result = ucmd.arg(id).arg(file1).run(); - println!("result.stdout = {}", result.stdout_str()); - println!("result.stderr = {}", result.stderr_str()); - if is_ci() && result.stderr_str().contains("chown: invalid user:") { - // With some Ubuntu into the CI, we can get this answer + let result = scene.ucmd().arg(user_id).arg("--verbose").arg(file1).run(); + 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' 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") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + 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()); - println!("result.stderr = {}", result.stderr_str()); - let id = String::from(result.stdout_str().trim()); - - let (at, mut ucmd) = at_and_ucmd!(); - let file1 = "test_install_target_dir_file_a1"; + let group_id = String::from(result.stdout_str().trim()); + assert!(!group_id.is_empty()); + let file1 = "test_chown_file1"; at.touch(file1); - let perm = ":".to_owned() + &id; - let result = ucmd.arg(perm).arg(file1).run(); - - println!("result.stdout = {}", result.stdout_str()); - println!("result.stderr = {}", result.stderr_str()); - if is_ci() && result.stderr_str().contains("chown: invalid group:") { + let result = scene + .ucmd() + .arg(format!(":{}", group_id)) + .arg("--verbose") + .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") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + 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()); - println!("result.stderr = {}", result.stderr_str()); - let id_user = String::from(result.stdout_str().trim()); + let user_id = String::from(result.stdout_str().trim()); + assert!(!user_id.is_empty()); - let result = TestScenario::new("id").ucmd_keepenv().arg("-g").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 + 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()); - println!("result.stderr = {}", result.stderr_str()); - let id_group = String::from(result.stdout_str().trim()); - - let (at, mut ucmd) = at_and_ucmd!(); - let file1 = "test_install_target_dir_file_a1"; + let group_id = String::from(result.stdout_str().trim()); + assert!(!group_id.is_empty()); + let file1 = "test_chown_file1"; at.touch(file1); - let perm = id_user + &":".to_owned() + &id_group; - let result = ucmd.arg(perm).arg(file1).run(); - println!("result.stdout = {}", result.stdout_str()); - println!("result.stderr = {}", result.stderr_str()); - - if is_ci() && result.stderr_str().contains("invalid user") { - // In the CI, some server are failing to return id. - // As seems to be a configuration issue, ignoring it + let result = scene + .ucmd() + .arg(format!("{}:{}", user_id, group_id)) + .arg("--verbose") + .arg(file1) + .run(); + 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() { - // 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") { - // 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()); +fn test_chown_owner_group_mix() { + // test chown 1111:group file.txt - let result = TestScenario::new("id").ucmd_keepenv().arg("-gn").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 + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + 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_install_target_dir_file_a1"; - + let file1 = "test_chown_file1"; 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") { - // In the CI, some server are failing to return id. - // As seems to be a configuration issue, ignoring it - return; - } - assert!(result.success); + // TODO: on macos group name is not recognized correctly: "chown: invalid group: '0:root' + #[cfg(any(windows, all(unix, not(target_os = "macos"))))] + scene + .ucmd() + .arg("0:root") + .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") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") { return; } - println!("result.stdout = {}", result.stdout_str()); - println!("result.stderr = {}", result.stderr_str()); - let username = result.stdout_str().trim_end(); + let user_name = String::from(result.stdout_str().trim()); + assert!(!user_name.is_empty()); - let (at, mut ucmd) = at_and_ucmd!(); - at.mkdir("a"); - at.mkdir("a/b"); - at.mkdir("a/b/c"); + at.mkdir_all("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()); - 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; - } - - result - .stderr_contains(&"ownership of 'a/a' retained as") - .stderr_contains(&"ownership of 'z/y' retained as") - .success(); + result.stderr_contains(&"ownership of 'a/a' retained as"); + result.stderr_contains(&"ownership of 'z/y' retained as"); } #[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") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") { return; } - println!("result.stdout = {}", result.stdout_str()); - println!("result.stderr = {}", result.stderr_str()); - let username = result.stdout_str().trim_end(); + let user_name = String::from(result.stdout_str().trim()); + assert!(!user_name.is_empty()); - 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()); - 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")); + 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( - "chown: changing ownership of '/proc/self/cwd': Operation not permitted (os error 1)\n", + .stderr_contains( + "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"); +} diff --git a/tests/by-util/test_chroot.rs b/tests/by-util/test_chroot.rs index 05efd23ae..e2e355e14 100644 --- a/tests/by-util/test_chroot.rs +++ b/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!( - true, - result - .stderr - .starts_with("error: The following required arguments were not provided") - ); + assert!(result + .stderr_str() + .starts_with("error: The following required arguments were not provided")); - assert_eq!(true, result.stderr.contains("")); + assert!(result.stderr_str().contains("")); } #[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!( - true, - result.stderr.starts_with( - "chroot: error: cannot chroot to jail: Operation not permitted (os error 1)" - ) - ) + assert!(result + .stderr_str() + .starts_with("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!( - true, - result.stderr.starts_with("chroot: error: invalid userspec") - ); + assert!(result + .stderr_str() + .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; diff --git a/tests/by-util/test_cksum.rs b/tests/by-util/test_cksum.rs index 1a0915cd5..592e45c58 100644 --- a/tests/by-util/test_cksum.rs +++ b/tests/by-util/test_cksum.rs @@ -31,7 +31,10 @@ fn test_empty() { at.touch("a"); - ucmd.arg("a").succeeds().stdout.ends_with("0 a"); + ucmd.arg("a") + .succeeds() + .no_stderr() + .normalized_newlines_stdout_is("4294967295 0 a\n"); } #[test] @@ -41,36 +44,37 @@ fn test_arg_overrides_stdin() { at.touch("a"); - let result = ucmd - .arg("a") + ucmd.arg("a") .pipe_in(input.as_bytes()) // the command might have exited before all bytes have been pipe in. // in that case, we don't care about the error (broken pipe) .ignore_stdin_write_error() - .run(); - - println!("{}, {}", result.stdout, result.stderr); - - assert!(result.stdout.ends_with("0 a\n")) + .succeeds() + .no_stderr() + .normalized_newlines_stdout_is("4294967295 0 a\n"); } #[test] fn test_invalid_file() { - let (_, mut ucmd) = at_and_ucmd!(); + let ts = TestScenario::new(util_name!()); + let at = ts.fixtures.clone(); - let ls = TestScenario::new("ls"); - let files = ls.cmd("ls").arg("-l").run(); - println!("{:?}", files.stdout); - println!("{:?}", files.stderr); + let folder_name = "asdf"; - let folder_name = "asdf".to_string(); + // First check when file doesn't exist + ts.ucmd() + .arg(folder_name) + .fails() + .no_stdout() + .stderr_contains("cksum: error: 'asdf' No such file or directory"); - let result = ucmd.arg(&folder_name).run(); - - println!("stdout: {:?}", result.stdout); - println!("stderr: {:?}", result.stderr); - assert!(result.stderr.contains("cksum: error: 'asdf'")); - assert!(!result.success); + // Then check when the file is of an invalid type + at.mkdir(folder_name); + ts.ucmd() + .arg(folder_name) + .fails() + .no_stdout() + .stderr_contains("cksum: error: 'asdf' Is a directory"); } // Make sure crc is correct for files larger than 32 bytes @@ -79,14 +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); } diff --git a/tests/by-util/test_cp.rs b/tests/by-util/test_cp.rs index 07880d5c0..bf5f9919d 100644 --- a/tests/by-util/test_cp.rs +++ b/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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_HELLO_WORLD_DEST) - .run(); - - // Check that the exit code represents a successful copy. - assert!(result.success); + .succeeds(); // 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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_EXISTING_FILE) - .run(); - - assert!(result.success); + .succeeds(); // 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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_COPY_TO_FOLDER) - .run(); - - assert!(result.success); - assert!(result.stderr.contains("specified more than once")); + .succeeds() + .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!(); - let result = ucmd + new_ucmd!() .arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_EXISTING_FILE) - .run(); - - assert!(!result.success); - assert!(result.stderr.contains("not a directory")); + .fails() + .stderr_contains("not a directory"); } #[test] fn test_cp_directory_not_recursive() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd + new_ucmd!() .arg(TEST_COPY_TO_FOLDER) .arg(TEST_HELLO_WORLD_DEST) - .run(); - - assert!(!result.success); - assert!(result.stderr.contains("omitting directory")); + .fails() + .stderr_contains("omitting directory"); } #[test] fn test_cp_multiple_files() { let (at, mut ucmd) = at_and_ucmd!(); - let result = ucmd - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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!(); - - let result = ucmd - .arg("-r") + 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!(); - - //using -t option - let result_to_dir_t = ucmd - .arg("-t") + ucmd.arg("-t") .arg(TEST_COPY_TO_FOLDER) .arg(TEST_HELLO_WORLD_SOURCE) - .run(); - assert!(result_to_dir_t.success); + .succeeds(); 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 - let result_to_dir = scene + scene .ucmd() .arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_COPY_TO_FOLDER) - .run(); - assert!(result_to_dir.success); + .succeeds(); 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(); - assert!(result_from_dir.success); + .succeeds(); 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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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!(); - let result = ucmd + new_ucmd!() .arg(TEST_HELLO_WORLD_SOURCE) .arg("-v") .arg("-T") .arg(TEST_COPY_TO_FOLDER) - .run(); - - assert!(!result.success); - assert!(result.stderr.contains("cannot overwrite directory")); + .fails() + .stderr_contains("cannot overwrite directory"); } #[test] fn test_cp_arg_interactive() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd + new_ucmd!() .arg(TEST_HELLO_WORLD_SOURCE) .arg(TEST_HOW_ARE_YOU_SOURCE) .arg("-i") .pipe_in("N\n") - .run(); - - assert!(result.success); - assert!(result.stderr.contains("Not overwriting")); + .succeeds() + .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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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!(); - - ucmd.arg("-LP") + new_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 = &scene.fixtures; + let (at, mut ucmd) = at_and_ucmd!(); #[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 = &scene.fixtures; + let (at, mut ucmd) = at_and_ucmd!(); #[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 - .arg("--strip-trailing-slashes") + ucmd.arg("--strip-trailing-slashes") .arg(format!("{}/", TEST_HELLO_WORLD_SOURCE)) .arg(TEST_HELLO_WORLD_DEST) - .run(); - - // Check that the exit code represents a successful copy. - assert!(result.success); + .succeeds(); // 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 - .arg("--parents") + ucmd.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 - .arg("--parents") + ucmd.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!(); - - let result = ucmd + new_ucmd!() .arg("--parents") .arg(TEST_COPY_FROM_FOLDER_FILE) .arg(TEST_HELLO_WORLD_DEST) - .run(); - println!("{:?}", result); - - // Check that we did not succeed in copying. - assert!(!result.success); - assert!(result - .stderr - .contains("with --parents, the destination must be a directory")); + .fails() + .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(); - println!("cp output {}", result.stdout_str()); - - // Check that the exit code represents a successful copy. - assert!(result.success); + .succeeds(); #[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(); - println!("cp output {}", result.stdout_str()); - - // Check that the exit code represents a successful copy. - assert!(result.success); + .succeeds(); #[cfg(not(windows))] { @@ -791,13 +699,11 @@ fn test_cp_archive() { previous, ) .unwrap(); - let result = ucmd - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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 - .arg("--archive") + ucmd.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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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 - .arg(TEST_HELLO_WORLD_SOURCE) + ucmd.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(); - assert!(_r.code == Some(0), "{}", _r.stderr); + .succeeds(); 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(); - assert!(_r.code == Some(0), "{}", _r.stderr); + scene.cmd("umount").arg(mountpoint_path).succeeds(); - assert!(result.success); assert!(!at_dst.file_exists(TEST_MOUNT_OTHER_FILESYSTEM_FILE)); // 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"); +} diff --git a/tests/by-util/test_date.rs b/tests/by-util/test_date.rs index 1a2e83e17..0ca0a74ea 100644 --- a/tests/by-util/test_date.rs +++ b/tests/by-util/test_date.rs @@ -7,174 +7,147 @@ use rust_users::*; #[test] fn test_date_email() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("--rfc-email").run(); - assert!(result.success); + new_ucmd!().arg("--rfc-email").succeeds(); } #[test] fn test_date_email2() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("-R").run(); - assert!(result.success); + new_ucmd!().arg("-R").succeeds(); } #[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])))"; - let re = Regex::new(rfc_regexp).unwrap(); - assert!(re.is_match(&result.stdout_str().trim())); + scene + .ucmd() + .arg("--rfc-3339=ns") + .succeeds() + .stdout_matches(&re); - result = scene.ucmd().arg("--rfc-3339=seconds").succeeds(); - - // Check that the output matches the regexp - let re = Regex::new(rfc_regexp).unwrap(); - assert!(re.is_match(&result.stdout_str().trim())); + scene + .ucmd() + .arg("--rfc-3339=seconds") + .succeeds() + .stdout_matches(&re); } #[test] fn test_date_rfc_8601() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("--iso-8601=ns").run(); - assert!(result.success); + new_ucmd!().arg("--iso-8601=ns").succeeds(); } #[test] fn test_date_rfc_8601_second() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("--iso-8601=second").run(); - assert!(result.success); + new_ucmd!().arg("--iso-8601=second").succeeds(); } #[test] fn test_date_utc() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("--utc").run(); - assert!(result.success); + new_ucmd!().arg("--utc").succeeds(); } #[test] fn test_date_universal() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("--universal").run(); - assert!(result.success); + new_ucmd!().arg("--universal").succeeds(); } #[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())); - - result = scene.ucmd().arg("+%A").succeeds(); - - assert!(result.success); + scene.ucmd().arg("+%a").succeeds().stdout_matches(&re); 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!(); - let result = ucmd + new_ucmd!() .arg("--set") .arg("2020-03-12 13:30:00+08:00") - .succeeds(); - result.no_stdout().no_stderr(); + .succeeds() + .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 = ucmd.arg("--set").arg("123abcd").fails(); - let result = result.no_stdout(); - assert!(result.stderr.starts_with("date: invalid date ")); + let result = new_ucmd!().arg("--set").arg("123abcd").fails(); + result.no_stdout(); + assert!(result.stderr_str().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 = ucmd.arg("--set").arg("2020-03-11 21:45:00+08:00").fails(); - let result = result.no_stdout(); - assert!(result.stderr.starts_with("date: cannot set date: ")); + let result = new_ucmd!() + .arg("--set") + .arg("2020-03-11 21:45:00+08:00") + .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 = ucmd.arg("--set").arg("2020-03-11 21:45:00+08:00").fails(); - let result = result.no_stdout(); + let result = new_ucmd!() + .arg("--set") + .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 = ucmd + let result = new_ucmd!() .arg("--set") .arg("Sat 20 Mar 2021 14:53:01 AWST") .fails(); - let result = result.no_stdout(); - assert!(result.stderr.starts_with("date: invalid date ")); + result.no_stdout(); + 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 = ucmd + let result = new_ucmd!() .arg("--set") .arg("Sat 20 Mar 2021 14:53:01") // Local timezone .fails(); - let result = result.no_stdout(); - assert!(result.stderr.starts_with("date: invalid date ")); + result.no_stdout(); + 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 = ucmd + let result = new_ucmd!() .arg("--set") .arg("2020-03-11 21:45:00") // Local timezone .fails(); - let result = result.no_stdout(); - assert!(result.stderr.starts_with("date: invalid date ")); + result.no_stdout(); + assert!(result.stderr_str().starts_with("date: invalid date ")); } } diff --git a/tests/by-util/test_df.rs b/tests/by-util/test_df.rs index f79d1beb5..0ae8d2339 100644 --- a/tests/by-util/test_df.rs +++ b/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!(); - let result = ucmd.arg("-a").run(); - assert!(result.success); + new_ucmd!().arg("-a").succeeds(); } #[test] fn test_df_compatible() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("-ah").run(); - assert!(result.success); + new_ucmd!().arg("-ah").succeeds(); } #[test] fn test_df_compatible_type() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("-aT").run(); - assert!(result.success); + new_ucmd!().arg("-aT").succeeds(); } #[test] fn test_df_compatible_si() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("-aH").run(); - assert!(result.success); + new_ucmd!().arg("-aH").succeeds(); } // ToDO: more tests... diff --git a/tests/by-util/test_du.rs b/tests/by-util/test_du.rs index 96788d741..111f2dc90 100644 --- a/tests/by-util/test_du.rs +++ b/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!() - .succeeds() - .no_stderr(); + new_ucmd!().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(); - assert!(result.success); - assert_eq!(result.stderr, ""); + let result = scene.ucmd().arg(SUB_DIR).succeeds(); + + #[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!(); - - let result = ucmd.arg("bad_name").run(); - assert_eq!(result.stdout_str(), ""); - assert_eq!( - result.stderr, - "du: error: bad_name: No such file or directory\n" - ); + new_ucmd!() + .arg("bad_name") + .succeeds() // TODO: replace with ".fails()" once `du` is fixed + .stderr_only("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(); - assert!(link.success); + at.symlink_file(SUB_FILE, SUB_LINK); - let result = ts.ucmd().arg(SUB_DIR_LINKS).run(); - assert!(result.success); - assert_eq!(result.stderr, ""); + let result = scene.ucmd().arg(SUB_DIR_LINKS).succeeds(); + + #[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(); - assert!(link.success); + let result_ln = scene.cmd("ln").arg(SUB_FILE).arg(SUB_LINK).run(); + if !result_ln.succeeded() { + scene.ccmd("ln").arg(SUB_FILE).arg(SUB_LINK).succeeds(); + } - let result = ts.ucmd().arg(SUB_DIR_LINKS).run(); - assert!(result.success); - assert_eq!(result.stderr, ""); + let result = scene.ucmd().arg(SUB_DIR_LINKS).succeeds(); + + #[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(); - assert!(result.success); - assert_eq!(result.stderr, ""); + let result = scene.ucmd().arg("-d1").succeeds(); + + #[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"); - - ts.ucmd() + new_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(); - assert!(touch.success); + scene + .ccmd("touch") + .arg("-a") + .arg("-m") + .arg("-t") + .arg("201505150000") + .arg("date_test") + .succeeds(); - let result = ts.ucmd().arg("--time").arg("date_test").run(); - - // cleanup by removing test file - ts.cmd("rm").arg("date_test").run(); - - assert!(result.success); - assert_eq!(result.stderr, ""); - assert_eq!(result.stdout, "0\t2015-05-15 00:00\tdate_test\n"); + scene + .ucmd() + .arg("--time") + .arg("date_test") + .succeeds() + .stdout_only("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"); } diff --git a/tests/by-util/test_echo.rs b/tests/by-util/test_echo.rs index 99c8f3a1e..5d1b68e6c 100644 --- a/tests/by-util/test_echo.rs +++ b/tests/by-util/test_echo.rs @@ -2,10 +2,7 @@ use crate::common::util::*; #[test] fn test_default() { - new_ucmd!() - .arg("hi") - .succeeds() - .stdout_only("hi\n"); + new_ucmd!().arg("hi").succeeds().stdout_only("hi\n"); } #[test] diff --git a/tests/by-util/test_env.rs b/tests/by-util/test_env.rs index 19ecd7afb..e86a41783 100644 --- a/tests/by-util/test_env.rs +++ b/tests/by-util/test_env.rs @@ -26,17 +26,18 @@ fn test_env_version() { #[test] fn test_echo() { - let result = new_ucmd!() - .arg("echo") - .arg("FOO-bar") - .succeeds(); + let result = new_ucmd!().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; - assert!(out.contains("cannot specify --null (-0) with command")); + new_ucmd!() + .arg("--null") + .arg("cd") + .fails() + .stderr_contains("cannot specify --null (-0) with command"); } #[cfg(not(windows))] diff --git a/tests/by-util/test_fmt.rs b/tests/by-util/test_fmt.rs index f962a9137..21a5f3396 100644 --- a/tests/by-util/test_fmt.rs +++ b/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" ); } diff --git a/tests/by-util/test_fold.rs b/tests/by-util/test_fold.rs index ffcd65737..5224a50dc 100644 --- a/tests/by-util/test_fold.rs +++ b/tests/by-util/test_fold.rs @@ -542,4 +542,4 @@ fn test_obsolete_syntax() { .arg("space_separated_words.txt") .succeeds() .stdout_is("test1\n \ntest2\n \ntest3\n \ntest4\n \ntest5\n \ntest6\n "); -} \ No newline at end of file +} diff --git a/tests/by-util/test_groups.rs b/tests/by-util/test_groups.rs index 32a16cc1a..cee13bdc3 100644 --- a/tests/by-util/test_groups.rs +++ b/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!(); - let result = ucmd.arg(username).run(); - println!("result.stdout = {}", result.stdout_str()); - println!("result.stderr = {}", result.stderr_str()); - assert!(result.success); + new_ucmd!().arg(username).succeeds(); assert!(!result.stdout_str().is_empty()); } diff --git a/tests/by-util/test_head.rs b/tests/by-util/test_head.rs index d91cc1289..4f009c800 100755 --- a/tests/by-util/test_head.rs +++ b/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(); - - assert_eq!( - true, - result - .stderr - .contains("cannot open 'no_such_file.toml' for reading: No such file or directory") - ) + new_ucmd!() + .arg("no_such_file.toml") + .fails() + .stderr_contains("cannot open 'no_such_file.toml' for reading: No such file or directory"); } // there was a bug not caught by previous tests diff --git a/tests/by-util/test_hostid.rs b/tests/by-util/test_hostid.rs index b5b668901..3ea818480 100644 --- a/tests/by-util/test_hostid.rs +++ b/tests/by-util/test_hostid.rs @@ -4,10 +4,6 @@ use self::regex::Regex; #[test] fn test_normal() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.run(); - - assert!(result.success); let re = Regex::new(r"^[0-9a-f]{8}").unwrap(); - assert!(re.is_match(&result.stdout_str())); + new_ucmd!().succeeds().stdout_matches(&re); } diff --git a/tests/by-util/test_hostname.rs b/tests/by-util/test_hostname.rs index 9fa63241f..3fcb1ae8b 100644 --- a/tests/by-util/test_hostname.rs +++ b/tests/by-util/test_hostname.rs @@ -14,9 +14,7 @@ fn test_hostname() { #[cfg(not(target_vendor = "apple"))] #[test] fn test_hostname_ip() { - let result = new_ucmd!().arg("-i").run(); - println!("{:#?}", result); - assert!(result.success); + let result = new_ucmd!().arg("-i").succeeds(); assert!(!result.stdout_str().trim().is_empty()); } @@ -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()); } diff --git a/tests/by-util/test_id.rs b/tests/by-util/test_id.rs index 7e2791467..534736a32 100644 --- a/tests/by-util/test_id.rs +++ b/tests/by-util/test_id.rs @@ -1,11 +1,32 @@ use crate::common::util::*; +// Apparently some CI environments have configuration issues, e.g. with 'whoami' and 'id'. +// If we are running inside the CI and "needle" is in "stderr" skipping this test is +// considered okay. If we are not inside the CI this calls assert!(result.success). +// +// From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)" +// stderr: "whoami: cannot find name for user ID 1001" +// Maybe: "adduser --uid 1001 username" can put things right? +// stderr = id: error: Could not find uid 1001: No such id: 1001 +fn skipping_test_is_okay(result: &CmdResult, needle: &str) -> bool { + if !result.succeeded() { + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + if is_ci() && result.stderr_str().contains(needle) { + println!("test skipped:"); + return true; + } else { + result.success(); + } + } + false +} + fn return_whoami_username() -> String { let scene = TestScenario::new("whoami"); let result = scene.cmd("whoami").run(); - if is_ci() && result.stderr.contains("cannot find name for user ID") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + if skipping_test_is_okay(&result, "whoami: cannot find name for user ID") { + println!("test skipped:"); return String::from(""); } @@ -14,39 +35,41 @@ fn return_whoami_username() -> String { #[test] fn test_id() { - let result = new_ucmd!().arg("-u").run(); - if result.stderr.contains("cannot find name for user ID") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + let scene = TestScenario::new(util_name!()); + + let result = scene.ucmd().arg("-u").succeeds(); + let uid = result.stdout_str().trim(); + + let result = scene.ucmd().run(); + if skipping_test_is_okay(&result, "Could not find uid") { return; } - let uid = result.success().stdout_str().trim(); - let result = new_ucmd!().run(); - if is_ci() && result.stderr.contains("cannot find name for user ID") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it - return; - } - - if !result.stderr_str().contains("Could not find uid") { - // Verify that the id found by --user/-u exists in the list - result.success().stdout_contains(&uid); - } + // Verify that the id found by --user/-u exists in the list + result.stdout_contains(uid); } #[test] fn test_id_from_name() { let username = return_whoami_username(); - if username == "" { - // Sometimes, the CI is failing here + if username.is_empty() { + return; + } + + let scene = TestScenario::new(util_name!()); + let result = scene.ucmd().arg(&username).run(); + if skipping_test_is_okay(&result, "Could not find uid") { return; } - let result = new_ucmd!().arg(&username).succeeds(); let uid = result.stdout_str().trim(); - new_ucmd!().succeeds() + let result = scene.ucmd().run(); + if skipping_test_is_okay(&result, "Could not find uid") { + return; + } + + result // Verify that the id found by --user/-u exists in the list .stdout_contains(uid) // Verify that the username found by whoami exists in the list @@ -55,51 +78,42 @@ fn test_id_from_name() { #[test] fn test_id_name_from_id() { - let result = new_ucmd!().arg("-u").succeeds(); - let uid = result.stdout_str().trim(); + let result = new_ucmd!().arg("-nu").run(); - let result = new_ucmd!().arg("-nu").arg(uid).run(); - if is_ci() && result.stderr.contains("No such user/group") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + let username_id = result.stdout_str().trim(); + + let username_whoami = return_whoami_username(); + if username_whoami.is_empty() { return; } - let username_id = result - .success() - .stdout_str() - .trim(); - - let scene = TestScenario::new("whoami"); - let result = scene.cmd("whoami").succeeds(); - - let username_whoami = result.stdout_str().trim(); - assert_eq!(username_id, username_whoami); } #[test] fn test_id_group() { - let mut result = new_ucmd!().arg("-g").succeeds(); + let scene = TestScenario::new(util_name!()); + + let mut result = scene.ucmd().arg("-g").succeeds(); let s1 = result.stdout_str().trim(); assert!(s1.parse::().is_ok()); - result = new_ucmd!().arg("--group").succeeds(); + result = scene.ucmd().arg("--group").succeeds(); let s1 = result.stdout_str().trim(); assert!(s1.parse::().is_ok()); } #[test] fn test_id_groups() { - let result = new_ucmd!().arg("-G").succeeds(); - assert!(result.success); + let scene = TestScenario::new(util_name!()); + + let result = scene.ucmd().arg("-G").succeeds(); let groups = result.stdout_str().trim().split_whitespace(); for s in groups { assert!(s.parse::().is_ok()); } - let result = new_ucmd!().arg("--groups").succeeds(); - assert!(result.success); + let result = scene.ucmd().arg("--groups").succeeds(); let groups = result.stdout_str().trim().split_whitespace(); for s in groups { assert!(s.parse::().is_ok()); @@ -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::().is_ok()); - result = new_ucmd!().arg("--user").succeeds(); + let result = scene.ucmd().arg("--user").succeeds(); let s1 = result.stdout_str().trim(); assert!(s1.parse::().is_ok()); } @@ -120,28 +136,34 @@ fn test_id_user() { #[test] fn test_id_pretty_print() { let username = return_whoami_username(); - if username == "" { - // Sometimes, the CI is failing here + if username.is_empty() { return; } - let result = new_ucmd!().arg("-p").run(); - if result.stdout_str().trim() == "" { - // Sometimes, the CI is failing here with - // old rust versions on Linux + let scene = TestScenario::new(util_name!()); + let result = scene.ucmd().arg("-p").run(); + if result.stdout_str().trim().is_empty() { + // this fails only on: "MinRustV (ubuntu-latest, feat_os_unix)" + // `rustc 1.40.0 (73528e339 2019-12-16)` + // run: /home/runner/work/coreutils/coreutils/target/debug/coreutils id -p + // thread 'test_id::test_id_pretty_print' panicked at 'Command was expected to succeed. + // stdout = + // stderr = ', tests/common/util.rs:157:13 + println!("test skipped:"); return; } + result.success().stdout_contains(username); } #[test] fn test_id_password_style() { let username = return_whoami_username(); - if username == "" { - // Sometimes, the CI is failing here + if username.is_empty() { return; } let result = new_ucmd!().arg("-P").succeeds(); + assert!(result.stdout_str().starts_with(&username)); } diff --git a/tests/by-util/test_install.rs b/tests/by-util/test_install.rs index 32df8d460..fc4459072 100644 --- a/tests/by-util/test_install.rs +++ b/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 - .arg("--help") + ucmd.arg("--help") .succeeds() .no_stderr() - .stdout - .contains("FLAGS:")); + .stdout_contains("FLAGS:"); } #[test] @@ -59,13 +57,11 @@ fn test_install_failing_not_dir() { at.touch(file1); at.touch(file2); at.touch(file3); - assert!(ucmd - .arg(file1) + ucmd.arg(file1) .arg(file2) .arg(file3) .fails() - .stderr - .contains("not a directory")); + .stderr_contains("not a directory"); } #[test] @@ -77,13 +73,11 @@ fn test_install_unimplemented_arg() { at.touch(file); at.mkdir(dir); - assert!(ucmd - .arg(context_arg) + ucmd.arg(context_arg) .arg(file) .arg(dir) .fails() - .stderr - .contains("Unimplemented")); + .stderr_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 - .arg(file) + ucmd.arg(file) .arg(dir) .arg(mode_arg) .fails() - .stderr - .contains("Invalid mode string: invalid digit found in string")); + .stderr_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(); - assert!(r.code == Some(1)); - assert!(r.stderr.contains("omitting directory")); + ucmd.arg(dir1) + .arg(file1) + .arg(dir2) + .fails() + .code_is(1) + .stderr_contains("omitting directory"); } #[test] @@ -458,9 +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(); - assert!(r.code == Some(1)); - assert!(r.stderr.contains("No such file or directory")); + ucmd.arg(file1) + .arg(file2) + .arg(dir1) + .fails() + .code_is(1) + .stderr_contains("No such file or directory"); } #[test] @@ -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!()); - - let stderr = scene - .ucmd() + new_ucmd!() .arg("-s") .arg("--strip-program") .arg("/bin/date") .arg(strip_source_file()) .arg(STRIP_TARGET_FILE) .fails() - .stderr; - assert!(stderr.contains("strip program failed")); + .stderr_contains("strip program failed"); } #[test] #[cfg(not(windows))] fn test_install_and_strip_with_non_existent_program() { - let scene = TestScenario::new(util_name!()); - - let stderr = scene - .ucmd() + new_ucmd!() .arg("-s") .arg("--strip-program") .arg("/usr/bin/non_existent_program") .arg(strip_source_file()) .arg(STRIP_TARGET_FILE) .fails() - .stderr; - assert!(stderr.contains("No such file or directory")); + .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"); } diff --git a/tests/by-util/test_ln.rs b/tests/by-util/test_ln.rs index d7a13b0d4..646091b09 100644 --- a/tests/by-util/test_ln.rs +++ b/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!( - ucmd.args(&["-s", "-T", path_a, path_b]) - .fails() - .stderr - .len() - > 0 - ); + assert!(!ucmd + .args(&["-s", "-T", path_a, path_b]) + .fails() + .stderr_str() + .is_empty()); } #[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] diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index f0db7ca9c..09e02f264 100644 --- a/tests/by-util/test_ls.rs +++ b/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::>()) + .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. - #[cfg(unix)] - result.stdout_contains(":"); - #[cfg(not(unix))] - result.stdout_contains("???"); + for arg in &["-c", "--time=ctime", "--time=status"] { + let result = scene.ucmd().arg("-l").arg(arg).succeeds(); + + // Should show the time on Unix, but question marks on windows. + #[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(); - let file3_access = at.open("test-3").metadata().unwrap().accessed().unwrap(); - let file4_access = at.open("test-4").metadata().unwrap().accessed().unwrap(); + for arg in &["-u", "--time=atime", "--time=access", "--time=use"] { + let result = scene.ucmd().arg("-t").arg(arg).succeeds(); + 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 - if file3_access > file4_access { - if cfg!(not(windows)) { - result.stdout_only("test-3\ntest-4\ntest-2\ntest-1\n"); + // It seems to be dependent on the platform whether the access time is actually set + if file3_access > file4_access { + if cfg!(not(windows)) { + 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"); - } - } else { - // Access time does not seem to be set on Windows and some other - // systems so the order is 4 3 2 1 - if cfg!(not(windows)) { - 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"); + // Access time does not seem to be set on Windows and some other + // systems so the order is 4 3 2 1 + if cfg!(not(windows)) { + 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 z_with_colors = "\x1b[01;34mz\x1b[0m"; - let nested_dir_with_colors = "\x1b[01;34mnested_dir\x1b[0m"; + let a_with_colors = "\x1b[1;34ma\x1b[0m"; + let z_with_colors = "\x1b[1;34mz\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")); +} diff --git a/tests/by-util/test_mkfifo.rs b/tests/by-util/test_mkfifo.rs index f60c0a4b8..23108d976 100644 --- a/tests/by-util/test_mkfifo.rs +++ b/tests/by-util/test_mkfifo.rs @@ -19,8 +19,7 @@ fn test_create_one_fifo_with_invalid_mode() { .arg("-m") .arg("invalid") .fails() - .stderr - .contains("invalid mode"); + .stderr_contains("invalid mode"); } #[test] diff --git a/tests/by-util/test_mktemp.rs b/tests/by-util/test_mktemp.rs index 2639a2c2f..c273c407c 100644 --- a/tests/by-util/test_mktemp.rs +++ b/tests/by-util/test_mktemp.rs @@ -113,17 +113,14 @@ fn test_mktemp_mktemp_t() { .arg("-t") .arg(TEST_TEMPLATE7) .succeeds(); - let result = scene + scene .ucmd() .env(TMPDIR, &pathname) .arg("-t") .arg(TEST_TEMPLATE8) - .fails(); - println!("stdout {}", result.stdout); - println!("stderr {}", result.stderr); - assert!(result - .stderr - .contains("error: suffix cannot contain any path separators")); + .fails() + .no_stdout() + .stderr_contains("error: suffix cannot contain any path separators"); } #[test] @@ -391,10 +388,8 @@ fn test_mktemp_tmpdir_one_arg() { .arg("--tmpdir") .arg("apt-key-gpghome.XXXXXXXXXX") .succeeds(); - println!("stdout {}", result.stdout); - println!("stderr {}", result.stderr); - assert!(result.stdout.contains("apt-key-gpghome.")); - assert!(PathBuf::from(result.stdout.trim()).is_file()); + result.no_stderr().stdout_contains("apt-key-gpghome."); + assert!(PathBuf::from(result.stdout_str().trim()).is_file()); } #[test] @@ -407,8 +402,6 @@ fn test_mktemp_directory_tmpdir() { .arg("--tmpdir") .arg("apt-key-gpghome.XXXXXXXXXX") .succeeds(); - println!("stdout {}", result.stdout); - println!("stderr {}", result.stderr); - assert!(result.stdout.contains("apt-key-gpghome.")); - assert!(PathBuf::from(result.stdout.trim()).is_dir()); + result.no_stderr().stdout_contains("apt-key-gpghome."); + assert!(PathBuf::from(result.stdout_str().trim()).is_dir()); } diff --git a/tests/by-util/test_more.rs b/tests/by-util/test_more.rs index 736fb6956..9245733ca 100644 --- a/tests/by-util/test_more.rs +++ b/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!(); - let result = ucmd.run(); - assert!(!result.success); + // stderr = more: Reading from stdin isn't supported yet. + new_ucmd!().fails(); } #[test] fn test_more_dir_arg() { - let (_, mut ucmd) = at_and_ucmd!(); - ucmd.arg("."); - let result = ucmd.run(); - assert!(!result.success); + let result = new_ucmd!().arg(".").run(); + result.failure(); 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); } diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 0caeb1ef1..e8ba43282 100644 --- a/tests/by-util/test_mv.rs +++ b/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!( - ucmd.arg("-vT") - .arg(dir_a) - .arg(dir_b) - .fails() - .no_stdout() - .stderr - .len() - > 0 - ); + let result = ucmd.arg("-vT").arg(dir_a).arg(dir_b).fails(); + result.no_stdout(); + assert!(!result.stderr_str().is_empty()); 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(); - assert!(result.stderr.contains("cannot be used with")); + .fails() + .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] diff --git a/tests/by-util/test_nice.rs b/tests/by-util/test_nice.rs index 7e704fc00..d3457c686 100644 --- a/tests/by-util/test_nice.rs +++ b/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")); } diff --git a/tests/by-util/test_nproc.rs b/tests/by-util/test_nproc.rs index 055b4890d..abf758829 100644 --- a/tests/by-util/test_nproc.rs +++ b/tests/by-util/test_nproc.rs @@ -2,54 +2,46 @@ use crate::common::util::*; #[test] fn test_nproc() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.run(); - assert!(result.success); - let nproc: u8 = result.stdout.trim().parse().unwrap(); + let nproc: u8 = new_ucmd!().succeeds().stdout_str().trim().parse().unwrap(); assert!(nproc > 0); } #[test] fn test_nproc_all_omp() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("--all").run(); - assert!(result.success); - let nproc: u8 = result.stdout.trim().parse().unwrap(); + let result = new_ucmd!().arg("--all").succeeds(); + + let nproc: u8 = result.stdout_str().trim().parse().unwrap(); assert!(nproc > 0); let result = TestScenario::new(util_name!()) .ucmd_keepenv() .env("OMP_NUM_THREADS", "1") - .run(); - assert!(result.success); - let nproc_omp: u8 = result.stdout.trim().parse().unwrap(); + .succeeds(); + + let nproc_omp: u8 = result.stdout_str().trim().parse().unwrap(); assert!(nproc - 1 == nproc_omp); let result = TestScenario::new(util_name!()) .ucmd_keepenv() .env("OMP_NUM_THREADS", "1") // Has no effect .arg("--all") - .run(); - assert!(result.success); - let nproc_omp: u8 = result.stdout.trim().parse().unwrap(); + .succeeds(); + let nproc_omp: u8 = result.stdout_str().trim().parse().unwrap(); assert!(nproc == nproc_omp); } #[test] fn test_nproc_ignore() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.run(); - assert!(result.success); - let nproc: u8 = result.stdout.trim().parse().unwrap(); + let result = new_ucmd!().succeeds(); + let nproc: u8 = result.stdout_str().trim().parse().unwrap(); if nproc > 1 { // Ignore all CPU but one let result = TestScenario::new(util_name!()) .ucmd_keepenv() .arg("--ignore") .arg((nproc - 1).to_string()) - .run(); - assert!(result.success); - let nproc: u8 = result.stdout.trim().parse().unwrap(); + .succeeds(); + let nproc: u8 = result.stdout_str().trim().parse().unwrap(); assert!(nproc == 1); } } diff --git a/tests/by-util/test_pinky.rs b/tests/by-util/test_pinky.rs index c8e8334ab..7a4a3f3df 100644 --- a/tests/by-util/test_pinky.rs +++ b/tests/by-util/test_pinky.rs @@ -43,11 +43,9 @@ fn test_short_format_i() { let actual = TestScenario::new(util_name!()) .ucmd() .args(&args) - .run() - .stdout; + .succeeds() + .stdout_move_str(); let expect = expected_result(&args); - println!("actual: {:?}", actual); - println!("expect: {:?}", expect); let v_actual: Vec<&str> = actual.split_whitespace().collect(); let v_expect: Vec<&str> = expect.split_whitespace().collect(); assert_eq!(v_actual, v_expect); @@ -62,11 +60,9 @@ fn test_short_format_q() { let actual = TestScenario::new(util_name!()) .ucmd() .args(&args) - .run() - .stdout; + .succeeds() + .stdout_move_str(); let expect = expected_result(&args); - println!("actual: {:?}", actual); - println!("expect: {:?}", expect); let v_actual: Vec<&str> = actual.split_whitespace().collect(); let v_expect: Vec<&str> = expect.split_whitespace().collect(); assert_eq!(v_actual, v_expect); @@ -79,5 +75,5 @@ fn expected_result(args: &[&str]) -> String { .env("LANGUAGE", "C") .args(args) .run() - .stdout + .stdout_move_str() } diff --git a/tests/by-util/test_printenv.rs b/tests/by-util/test_printenv.rs index 9d90051ea..bc0bc0f3c 100644 --- a/tests/by-util/test_printenv.rs +++ b/tests/by-util/test_printenv.rs @@ -7,10 +7,11 @@ fn test_get_all() { env::set_var(key, "VALUE"); assert_eq!(env::var(key), Ok("VALUE".to_string())); - let result = TestScenario::new(util_name!()).ucmd_keepenv().run(); - assert!(result.success); - assert!(result.stdout.contains("HOME=")); - assert!(result.stdout.contains("KEY=VALUE")); + TestScenario::new(util_name!()) + .ucmd_keepenv() + .succeeds() + .stdout_contains("HOME=") + .stdout_contains("KEY=VALUE"); } #[test] @@ -22,9 +23,8 @@ fn test_get_var() { let result = TestScenario::new(util_name!()) .ucmd_keepenv() .arg("KEY") - .run(); + .succeeds(); - assert!(result.success); - assert!(!result.stdout.is_empty()); - assert!(result.stdout.trim() == "VALUE"); + assert!(!result.stdout_str().is_empty()); + assert!(result.stdout_str().trim() == "VALUE"); } diff --git a/tests/by-util/test_readlink.rs b/tests/by-util/test_readlink.rs index 84747b24c..cae5eafee 100644 --- a/tests/by-util/test_readlink.rs +++ b/tests/by-util/test_readlink.rs @@ -5,7 +5,7 @@ static GIBBERISH: &'static str = "supercalifragilisticexpialidocious"; #[test] fn test_canonicalize() { let (at, mut ucmd) = at_and_ucmd!(); - let actual = ucmd.arg("-f").arg(".").run().stdout; + let actual = ucmd.arg("-f").arg(".").run().stdout_move_str(); let expect = at.root_dir_resolved() + "\n"; println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -15,7 +15,7 @@ fn test_canonicalize() { #[test] fn test_canonicalize_existing() { let (at, mut ucmd) = at_and_ucmd!(); - let actual = ucmd.arg("-e").arg(".").run().stdout; + let actual = ucmd.arg("-e").arg(".").run().stdout_move_str(); let expect = at.root_dir_resolved() + "\n"; println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -25,7 +25,7 @@ fn test_canonicalize_existing() { #[test] fn test_canonicalize_missing() { let (at, mut ucmd) = at_and_ucmd!(); - let actual = ucmd.arg("-m").arg(GIBBERISH).run().stdout; + let actual = ucmd.arg("-m").arg(GIBBERISH).run().stdout_move_str(); let expect = path_concat!(at.root_dir_resolved(), GIBBERISH) + "\n"; println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -37,7 +37,7 @@ fn test_long_redirection_to_current_dir() { let (at, mut ucmd) = at_and_ucmd!(); // Create a 256-character path to current directory let dir = path_concat!(".", ..128); - let actual = ucmd.arg("-n").arg("-m").arg(dir).run().stdout; + let actual = ucmd.arg("-n").arg("-m").arg(dir).run().stdout_move_str(); let expect = at.root_dir_resolved(); println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -48,7 +48,12 @@ fn test_long_redirection_to_current_dir() { fn test_long_redirection_to_root() { // Create a 255-character path to root let dir = path_concat!("..", ..85); - let actual = new_ucmd!().arg("-n").arg("-m").arg(dir).run().stdout; + let actual = new_ucmd!() + .arg("-n") + .arg("-m") + .arg(dir) + .run() + .stdout_move_str(); let expect = get_root_path(); println!("actual: {:?}", actual); println!("expect: {:?}", expect); diff --git a/tests/by-util/test_shred.rs b/tests/by-util/test_shred.rs index de54fae5b..b29b9bfec 100644 --- a/tests/by-util/test_shred.rs +++ b/tests/by-util/test_shred.rs @@ -36,9 +36,7 @@ fn test_shred_force() { at.set_readonly(file); // Try shred -u. - let result = scene.ucmd().arg("-u").arg(file).run(); - println!("stderr = {:?}", result.stderr); - println!("stdout = {:?}", result.stdout); + scene.ucmd().arg("-u").arg(file).run(); // file_a was not deleted because it is readonly. assert!(at.file_exists(file)); diff --git a/tests/by-util/test_shuf.rs b/tests/by-util/test_shuf.rs index 717971bd4..f925f8357 100644 --- a/tests/by-util/test_shuf.rs +++ b/tests/by-util/test_shuf.rs @@ -9,35 +9,28 @@ fn test_output_is_random_permutation() { .collect::>() .join("\n"); - let result = new_ucmd!() - .pipe_in(input.as_bytes()) - .succeeds() - .no_stderr() - .stdout - .clone(); + let result = new_ucmd!().pipe_in(input.as_bytes()).succeeds(); + result.no_stderr(); let mut result_seq: Vec = 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!() - .arg("-z") - .arg("-i1-10") - .succeeds() - .no_stderr() - .stdout - .clone(); + let result = new_ucmd!().arg("-z").arg("-i1-10").succeeds(); + result.no_stderr(); let mut result_seq: Vec = 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::>(), ) - .succeeds() - .no_stderr() - .stdout - .clone(); + .succeeds(); + result.no_stderr(); let mut result_seq: Vec = 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() - .no_stderr() - .stdout - .clone(); + .succeeds(); + result.no_stderr(); let mut result_seq: Vec = 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() - .no_stderr() - .stdout - .clone(); + .succeeds(); + result.no_stderr(); let result_seq: Vec = 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!() - .arg("file_input.txt") - .succeeds() - .no_stderr() - .stdout - .clone(); + let result = new_ucmd!().arg("file_input.txt").succeeds(); + result.no_stderr(); let mut result_seq: Vec = 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(); - - assert!(!result.success); - assert!(result - .stderr - .contains("The argument '--input-range ' cannot be used with '--echo ...'")); + new_ucmd!() + .args(&["-e", "0", "-i", "0-2"]) + .fails() + .stderr_contains( + "The argument '--input-range ' cannot be used with '--echo ...'", + ); } #[test] fn test_shuf_input_range_and_file_not_allowed() { - let result = new_ucmd!().args(&["-i", "0-9", "file"]).run(); - - assert!(!result.success); - assert!(result - .stderr - .contains("The argument '' cannot be used with '--input-range '")); + new_ucmd!() + .args(&["-i", "0-9", "file"]) + .fails() + .stderr_contains("The argument '' cannot be used with '--input-range '"); } #[test] fn test_shuf_invalid_input_range_one() { - let result = new_ucmd!().args(&["-i", "0"]).run(); - - assert!(!result.success); - assert!(result.stderr.contains("invalid input range")); + new_ucmd!() + .args(&["-i", "0"]) + .fails() + .stderr_contains("invalid input range"); } #[test] fn test_shuf_invalid_input_range_two() { - let result = new_ucmd!().args(&["-i", "a-9"]).run(); - - assert!(!result.success); - assert!(result.stderr.contains("invalid input range: 'a'")); + new_ucmd!() + .args(&["-i", "a-9"]) + .fails() + .stderr_contains("invalid input range: 'a'"); } #[test] fn test_shuf_invalid_input_range_three() { - let result = new_ucmd!().args(&["-i", "0-b"]).run(); - - assert!(!result.success); - assert!(result.stderr.contains("invalid input range: 'b'")); + new_ucmd!() + .args(&["-i", "0-b"]) + .fails() + .stderr_contains("invalid input range: 'b'"); } #[test] fn test_shuf_invalid_input_line_count() { - let result = new_ucmd!().args(&["-n", "a"]).run(); - - assert!(!result.success); - assert!(result.stderr.contains("invalid line count: 'a'")); + new_ucmd!() + .args(&["-n", "a"]) + .fails() + .stderr_contains("invalid line count: 'a'"); } diff --git a/tests/by-util/test_sort.rs b/tests/by-util/test_sort.rs index 0f8020688..72d4f67fc 100644 --- a/tests/by-util/test_sort.rs +++ b/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("version-empty-lines.txt") - .succeeds() - .stdout_is("\n\n\n\n\n\n\n1.2.3-alpha\n1.2.3-alpha2\n\t\t\t1.12.4\n11.2.3\n"); + .arg("-V") + .arg("version-empty-lines.txt") + .succeeds() + .stdout_is("\n\n\n\n\n\n\n1.2.3-alpha\n1.2.3-alpha2\n\t\t\t1.12.4\n11.2.3\n"); } #[test] @@ -38,11 +38,7 @@ fn test_multiple_decimals_general() { #[test] fn test_multiple_decimals_numeric() { - new_ucmd!() - .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_helper("multiple_decimals_numeric", "-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"); +} \ No newline at end of file diff --git a/tests/by-util/test_stat.rs b/tests/by-util/test_stat.rs index 225ea52cd..60d735c51 100644 --- a/tests/by-util/test_stat.rs +++ b/tests/by-util/test_stat.rs @@ -194,7 +194,7 @@ fn test_terse_normal_format() { // note: contains birth/creation date which increases test fragility // * results may vary due to built-in `stat` limitations as well as linux kernel and rust version capability variations let args = ["-t", "/"]; - let actual = new_ucmd!().args(&args).run().stdout; + let actual = new_ucmd!().args(&args).succeeds().stdout_move_str(); let expect = expected_result(&args); println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -216,7 +216,7 @@ fn test_terse_normal_format() { #[cfg(target_os = "linux")] fn test_format_created_time() { let args = ["-c", "%w", "/boot"]; - let actual = new_ucmd!().args(&args).run().stdout; + let actual = new_ucmd!().args(&args).succeeds().stdout_move_str(); let expect = expected_result(&args); println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -240,7 +240,7 @@ fn test_format_created_time() { #[cfg(target_os = "linux")] fn test_format_created_seconds() { let args = ["-c", "%W", "/boot"]; - let actual = new_ucmd!().args(&args).run().stdout; + let actual = new_ucmd!().args(&args).succeeds().stdout_move_str(); let expect = expected_result(&args); println!("actual: {:?}", actual); println!("expect: {:?}", expect); @@ -337,5 +337,5 @@ fn expected_result(args: &[&str]) -> String { .env("LANGUAGE", "C") .args(args) .run() - .stdout + .stdout_move_str() } diff --git a/tests/by-util/test_sync.rs b/tests/by-util/test_sync.rs index ddd6969a3..436bfdef3 100644 --- a/tests/by-util/test_sync.rs +++ b/tests/by-util/test_sync.rs @@ -5,8 +5,7 @@ use tempfile::tempdir; #[test] fn test_sync_default() { - let result = new_ucmd!().run(); - assert!(result.success); + new_ucmd!().succeeds(); } #[test] @@ -18,8 +17,10 @@ fn test_sync_incorrect_arg() { fn test_sync_fs() { let temporary_directory = tempdir().unwrap(); let temporary_path = fs::canonicalize(temporary_directory.path()).unwrap(); - let result = new_ucmd!().arg("--file-system").arg(&temporary_path).run(); - assert!(result.success); + new_ucmd!() + .arg("--file-system") + .arg(&temporary_path) + .succeeds(); } #[test] @@ -27,12 +28,14 @@ fn test_sync_data() { // Todo add a second arg let temporary_directory = tempdir().unwrap(); let temporary_path = fs::canonicalize(temporary_directory.path()).unwrap(); - let result = new_ucmd!().arg("--data").arg(&temporary_path).run(); - assert!(result.success); + new_ucmd!().arg("--data").arg(&temporary_path).succeeds(); } #[test] fn test_sync_no_existing_files() { - let result = new_ucmd!().arg("--data").arg("do-no-exist").fails(); - assert!(result.stderr.contains("error: cannot stat")); + new_ucmd!() + .arg("--data") + .arg("do-no-exist") + .fails() + .stderr_contains("error: cannot stat"); } diff --git a/tests/by-util/test_tac.rs b/tests/by-util/test_tac.rs index 3733adbec..a8adbb28e 100644 --- a/tests/by-util/test_tac.rs +++ b/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") - .run() - .stderr - .contains("tac: error: failed to open 'b' for reading"); - - let (at, mut ucmd) = at_and_ucmd!(); + scene + .ucmd() + .arg("b") + .fails() + .stderr_contains("failed to open 'b' for reading: No such file or directory"); at.mkdir("a"); - ucmd.arg("a") - .run() - .stderr - .contains("tac: error: failed to read 'a'"); + scene + .ucmd() + .arg("a") + .fails() + .stderr_contains("dir: read error: Invalid argument"); } diff --git a/tests/by-util/test_tail.rs b/tests/by-util/test_tail.rs index 5edff4d55..1f74a3a98 100644 --- a/tests/by-util/test_tail.rs +++ b/tests/by-util/test_tail.rs @@ -226,8 +226,8 @@ fn test_bytes_big() { .arg(FILE) .arg("-c") .arg(format!("{}", N_ARG)) - .run() - .stdout; + .succeeds() + .stdout_move_str(); let expected = at.read(EXPECTED_FILE); assert_eq!(result.len(), expected.len()); @@ -340,6 +340,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_bytes_index.stdout, negative_bytes_index.stdout); + assert_eq!(positive_lines_index.stdout(), negative_lines_index.stdout()); + assert_eq!(positive_bytes_index.stdout(), negative_bytes_index.stdout()); +} + +#[test] +fn test_sleep_interval() { + new_ucmd!() + .arg("-s") + .arg("10") + .arg(FOOBAR_TXT) + .succeeds(); } diff --git a/tests/by-util/test_touch.rs b/tests/by-util/test_touch.rs index 9f2c079b0..40fbb8aa9 100644 --- a/tests/by-util/test_touch.rs +++ b/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 { + 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 => (), + } +} diff --git a/tests/by-util/test_tr.rs b/tests/by-util/test_tr.rs index a1500bcf6..630c305c6 100644 --- a/tests/by-util/test_tr.rs +++ b/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(); - - assert!(!result.success); - assert!(result.stderr.contains("missing operand")); + ucmd.fails().stderr_contains("missing operand"); } #[test] fn missing_required_second_arg_fails() { let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.args(&["foo"]).run(); - - assert!(!result.success); - assert!(result.stderr.contains("missing operand after")); + ucmd.args(&["foo"]) + .fails() + .stderr_contains("missing operand after"); } #[test] diff --git a/tests/by-util/test_truncate.rs b/tests/by-util/test_truncate.rs index ce7964d57..2a1f4429b 100644 --- a/tests/by-util/test_truncate.rs +++ b/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(); } + diff --git a/tests/by-util/test_tsort.rs b/tests/by-util/test_tsort.rs index 159b80025..0da6f44e4 100644 --- a/tests/by-util/test_tsort.rs +++ b/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(); - - assert_eq!(true, result.stderr.contains("No such file or directory")); + new_ucmd!() + .arg("invalid_file_txt") + .fails() + .stderr_contains("No such file or directory"); } #[test] fn test_version_flag() { - let version_short = new_ucmd!().arg("-V").run(); - let version_long = new_ucmd!().arg("--version").run(); + let version_short = new_ucmd!().arg("-V").succeeds(); + 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_long = new_ucmd!().arg("--help").run(); + let help_short = new_ucmd!().arg("-h").succeeds(); + 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") - .run(); - - assert_eq!(true, result.stderr.contains("error: Found argument 'invalid_file.txt' which wasn't expected, or isn't valid in this context")) + .arg("invalid_file") + .fails() + .stderr_contains( + "Found argument 'invalid_file' which wasn't expected, or isn't valid in this context", + ); } diff --git a/tests/by-util/test_uname.rs b/tests/by-util/test_uname.rs index f0e32b430..6b8d2d59d 100644 --- a/tests/by-util/test_uname.rs +++ b/tests/by-util/test_uname.rs @@ -2,60 +2,41 @@ use crate::common::util::*; #[test] fn test_uname_compatible() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-a").run(); - assert!(result.success); + new_ucmd!().arg("-a").succeeds(); } #[test] fn test_uname_name() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-n").run(); - assert!(result.success); + new_ucmd!().arg("-n").succeeds(); } #[test] fn test_uname_processor() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-p").run(); - assert!(result.success); - assert_eq!(result.stdout.trim_end(), "unknown"); + let result = new_ucmd!().arg("-p").succeeds(); + assert_eq!(result.stdout_str().trim_end(), "unknown"); } #[test] fn test_uname_hwplatform() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-i").run(); - assert!(result.success); - assert_eq!(result.stdout.trim_end(), "unknown"); + let result = new_ucmd!().arg("-i").succeeds(); + assert_eq!(result.stdout_str().trim_end(), "unknown"); } #[test] fn test_uname_machine() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-m").run(); - assert!(result.success); + new_ucmd!().arg("-m").succeeds(); } #[test] fn test_uname_kernel_version() { - let (_, mut ucmd) = at_and_ucmd!(); - - let result = ucmd.arg("-v").run(); - assert!(result.success); + new_ucmd!().arg("-v").succeeds(); } #[test] fn test_uname_kernel() { let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.arg("-o").run(); - assert!(result.success); + let result = ucmd.arg("-o").succeeds(); #[cfg(target_os = "linux")] - assert!(result.stdout.to_lowercase().contains("linux")); + assert!(result.stdout_str().to_lowercase().contains("linux")); } diff --git a/tests/by-util/test_uptime.rs b/tests/by-util/test_uptime.rs index c8f6a11d3..d20ad90c9 100644 --- a/tests/by-util/test_uptime.rs +++ b/tests/by-util/test_uptime.rs @@ -4,33 +4,23 @@ use crate::common::util::*; #[test] fn test_uptime() { - let result = TestScenario::new(util_name!()).ucmd_keepenv().run(); + TestScenario::new(util_name!()) + .ucmd_keepenv() + .succeeds() + .stdout_contains("load average:") + .stdout_contains(" up "); - println!("stdout = {}", result.stdout); - println!("stderr = {}", result.stderr); - - assert!(result.success); - assert!(result.stdout.contains("load average:")); - assert!(result.stdout.contains(" up ")); // Don't check for users as it doesn't show in some CI } #[test] fn test_uptime_since() { - let scene = TestScenario::new(util_name!()); - - let result = scene.ucmd().arg("--since").succeeds(); - - println!("stdout = {}", result.stdout); - println!("stderr = {}", result.stderr); - - assert!(result.success); let re = Regex::new(r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}").unwrap(); - assert!(re.is_match(&result.stdout.trim())); + + new_ucmd!().arg("--since").succeeds().stdout_matches(&re); } #[test] fn test_failed() { - let (_at, mut ucmd) = at_and_ucmd!(); - ucmd.arg("willfail").fails(); + new_ucmd!().arg("willfail").fails(); } diff --git a/tests/by-util/test_users.rs b/tests/by-util/test_users.rs index cb444b8be..3c5789820 100644 --- a/tests/by-util/test_users.rs +++ b/tests/by-util/test_users.rs @@ -3,14 +3,11 @@ use std::env; #[test] fn test_users_noarg() { - let (_, mut ucmd) = at_and_ucmd!(); - let result = ucmd.run(); - assert!(result.success); + new_ucmd!().succeeds(); } #[test] fn test_users_check_name() { - let result = TestScenario::new(util_name!()).ucmd_keepenv().run(); - assert!(result.success); + let result = TestScenario::new(util_name!()).ucmd_keepenv().succeeds(); // Expectation: USER is often set let key = "USER"; @@ -21,9 +18,9 @@ fn test_users_check_name() { // Check if "users" contains the name of the user { println!("username found {}", &username); - println!("result.stdout {}", &result.stdout); - if !&result.stdout.is_empty() { - assert!(result.stdout.contains(&username)) + // println!("result.stdout {}", &result.stdout); + if !result.stdout_str().is_empty() { + result.stdout_contains(&username); } } } diff --git a/tests/by-util/test_who.rs b/tests/by-util/test_who.rs index 89b7cec93..32d2427e0 100644 --- a/tests/by-util/test_who.rs +++ b/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() } diff --git a/tests/by-util/test_whoami.rs b/tests/by-util/test_whoami.rs index c6ec6990f..dc6a1ceed 100644 --- a/tests/by-util/test_whoami.rs +++ b/tests/by-util/test_whoami.rs @@ -1,50 +1,63 @@ use crate::common::util::*; -use std::env; + +// Apparently some CI environments have configuration issues, e.g. with 'whoami' and 'id'. +// If we are running inside the CI and "needle" is in "stderr" skipping this test is +// considered okay. If we are not inside the CI this calls assert!(result.success). +// +// From the Logs: "Build (ubuntu-18.04, x86_64-unknown-linux-gnu, feat_os_unix, use-cross)" +// stderr: "whoami: error: failed to get username" +// Maybe: "adduser --uid 1001 username" can put things right? +fn skipping_test_is_okay(result: &CmdResult, needle: &str) -> bool { + if !result.succeeded() { + println!("result.stdout = {}", result.stdout_str()); + println!("result.stderr = {}", result.stderr_str()); + if is_ci() && result.stderr_str().contains(needle) { + println!("test skipped:"); + return true; + } else { + result.success(); + } + } + false +} #[test] fn test_normal() { let (_, mut ucmd) = at_and_ucmd!(); let result = ucmd.run(); - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - println!("env::var(CI).is_ok() = {}", env::var("CI").is_ok()); - for (key, value) in env::vars() { - println!("{}: {}", key, value); - } - if is_ci() && result.stderr.contains("failed to get username") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + // use std::env; + // println!("env::var(CI).is_ok() = {}", env::var("CI").is_ok()); + // for (key, value) in env::vars() { + // println!("{}: {}", key, value); + // } + + if skipping_test_is_okay(&result, "failed to get username") { return; } - assert!(result.success); - assert!(!result.stdout.trim().is_empty()); + result.no_stderr(); + assert!(!result.stdout_str().trim().is_empty()); } #[test] #[cfg(not(windows))] fn test_normal_compare_id() { - let (_, mut ucmd) = at_and_ucmd!(); + let scene = TestScenario::new(util_name!()); - let result = ucmd.run(); - - println!("result.stdout = {}", result.stdout); - println!("result.stderr = {}", result.stderr); - if is_ci() && result.stderr.contains("failed to get username") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + let result_ucmd = scene.ucmd().run(); + if skipping_test_is_okay(&result_ucmd, "failed to get username") { return; } - assert!(result.success); - let ts = TestScenario::new("id"); - let id = ts.cmd("id").arg("-un").run(); - if is_ci() && id.stderr.contains("cannot find name for user ID") { - // In the CI, some server are failing to return whoami. - // As seems to be a configuration issue, ignoring it + let result_cmd = scene.cmd("id").arg("-un").run(); + if skipping_test_is_okay(&result_cmd, "cannot find name for user ID") { return; } - assert_eq!(result.stdout.trim(), id.stdout.trim()); + + assert_eq!( + result_ucmd.stdout_str().trim(), + result_cmd.stdout_str().trim() + ); } diff --git a/tests/common/util.rs b/tests/common/util.rs index 1915ac988..29b8a4633 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -55,14 +55,14 @@ pub struct CmdResult { //tmpd is used for convenience functions for asserts against fixtures tmpd: Option>, /// exit status for command (if there is one) - pub code: Option, + code: Option, /// zero-exit from running the Command? /// see [`success`] - pub success: bool, + 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>(&self, msg: T) -> &CmdResult { + let msg = msg.as_ref().replace("\r\n", "\n"); + assert_eq!(self.stdout.replace("\r\n", "\n"), msg); + self + } + /// asserts that the command resulted in stdout stream output, /// whose bytes equal those of the passed in slice pub fn stdout_is_bytes>(&self, msg: T) -> &CmdResult { @@ -306,14 +313,14 @@ impl CmdResult { } pub fn stdout_matches(&self, regex: ®ex::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: ®ex::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>, has_run: bool, - stdin: Option>, ignore_stdin_write_error: bool, + stdin: Option, + stdout: Option, + stderr: Option, + bytes_into_stdin: Option>, } 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>(&mut self, stdin: T) -> &mut UCommand { + self.stdin = Some(stdin.into()); + self + } + + pub fn set_stdout>(&mut self, stdout: T) -> &mut UCommand { + self.stdout = Some(stdout.into()); + self + } + + pub fn set_stderr>(&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>(&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>>(&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()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) + .stdin(self.stdin.take().unwrap_or_else(|| Stdio::piped())) + .stdout(self.stdout.take().unwrap_or_else(|| 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"); + } } diff --git a/tests/fixtures/sort/multiple_decimals_numeric.expected b/tests/fixtures/sort/multiple_decimals_numeric.expected new file mode 100644 index 000000000..3ef4d22e8 --- /dev/null +++ b/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