From 71bbebdc760fccc342a0e9f55be0486ab76d8205 Mon Sep 17 00:00:00 2001 From: Joining7943 <111500881+Joining7943@users.noreply.github.com> Date: Tue, 28 Feb 2023 07:21:18 +0100 Subject: [PATCH] sleep: Fix whitespace issues. Don't panic when adding durations. Refactor error propagation and use show_error! instead of USimpleError. --- src/uu/sleep/src/sleep.rs | 22 ++++++++++--------- tests/by-util/test_sleep.rs | 43 +++++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/src/uu/sleep/src/sleep.rs b/src/uu/sleep/src/sleep.rs index dde252204..4751d89b2 100644 --- a/src/uu/sleep/src/sleep.rs +++ b/src/uu/sleep/src/sleep.rs @@ -10,7 +10,7 @@ use std::time::Duration; use uucore::{ error::{UResult, USimpleError, UUsageError}, - format_usage, help_about, help_section, help_usage, show, + format_usage, help_about, help_section, help_usage, show_error, }; use clap::{crate_version, Arg, ArgAction, Command}; @@ -61,15 +61,17 @@ pub fn uu_app() -> Command { fn sleep(args: &[&str]) -> UResult<()> { let mut arg_error = false; - let intervals = args.iter().map(|s| match uucore::parse_time::from_str(s) { - Ok(result) => result, - Err(err) => { - arg_error = true; - show!(USimpleError::new(1, err)); - Duration::new(0, 0) - } - }); - let sleep_dur = intervals.fold(Duration::ZERO, |acc, n| acc.saturating_add(n)); + let sleep_dur = args + .iter() + .filter_map(|input| { + uucore::parse_time::from_str(input.trim()).ok().or_else(|| { + arg_error = true; + show_error!("invalid time interval '{input}'"); + None + }) + }) + .fold(Duration::ZERO, |acc, n| acc.saturating_add(n)); + if arg_error { return Err(UUsageError::new(1, "")); }; diff --git a/tests/by-util/test_sleep.rs b/tests/by-util/test_sleep.rs index 5f538900f..c525cc0d2 100644 --- a/tests/by-util/test_sleep.rs +++ b/tests/by-util/test_sleep.rs @@ -1,3 +1,5 @@ +use rstest::rstest; + // spell-checker:ignore dont use crate::common::util::*; @@ -180,6 +182,47 @@ fn test_sleep_when_multiple_inputs_exceed_max_duration_then_no_error() { .no_output(); } +#[rstest] +#[case::whitespace_prefix(" 0.1s")] +#[case::multiple_whitespace_prefix(" 0.1s")] +#[case::whitespace_suffix("0.1s ")] +#[case::mixed_newlines_spaces_tabs("\n\t0.1s \n ")] +fn test_sleep_when_input_has_whitespace_then_no_error(#[case] input: &str) { + new_ucmd!() + .arg(input) + .timeout(Duration::from_secs(10)) + .succeeds() + .no_output(); +} + +#[rstest] +#[case::only_space(" ")] +#[case::only_tab("\t")] +#[case::only_newline("\n")] +fn test_sleep_when_input_has_only_whitespace_then_error(#[case] input: &str) { + new_ucmd!() + .arg(input) + .timeout(Duration::from_secs(10)) + .fails() + .usage_error(format!("invalid time interval '{input}'")); +} + +#[test] +fn test_sleep_when_multiple_input_some_with_error_then_shows_all_errors() { + let expected = "invalid time interval 'abc'\n\ + sleep: invalid time interval '1years'\n\ + sleep: invalid time interval ' '"; + + // Even if one of the arguments is valid, but the rest isn't, we should still fail and exit early. + // So, the timeout of 10 seconds ensures we haven't executed `thread::sleep` with the only valid + // interval of `100000.0` seconds. + new_ucmd!() + .args(&["abc", "100000.0", "1years", " "]) + .timeout(Duration::from_secs(10)) + .fails() + .usage_error(expected); +} + #[test] fn test_negative_interval() { new_ucmd!()