From 9d40a129ee8299381cb631f86b4d64c3a99593f8 Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Wed, 15 Feb 2023 18:21:27 +0100 Subject: [PATCH] update tail tests for more compatibility --- tests/coreutils/tail.rs | 305 +++++++++++++++++++++++++++++----------- 1 file changed, 219 insertions(+), 86 deletions(-) diff --git a/tests/coreutils/tail.rs b/tests/coreutils/tail.rs index b8797f8..02c99b3 100644 --- a/tests/coreutils/tail.rs +++ b/tests/coreutils/tail.rs @@ -2,97 +2,111 @@ use std::{ffi::OsString, path::PathBuf}; use uutils_args::{Arguments, Initial, Options, Value}; -#[derive(Arguments)] -enum DeprecatedArg { - #[option("{N}")] - Shorthand(Shorthand), - #[positional] - File(PathBuf), -} +// This format is way to specific to implement using a library. Basically, any +// deviation should be return `None` to indicate that we're not using the +// this format. If this fails, we fall back on the normal parsing, so errors +// from this function are not relevant, so we can just return an `Option`. +// Once this gets into uutils, I highly recommend that we make this format +// optional at compile time. As the GNU docs explain, it's very error-prone. +fn parse_deprecated(iter: I) -> Option +where + I: IntoIterator + Clone + 'static, + I::Item: Into, +{ + let mut iter = iter.into_iter(); -impl Options for Settings { - fn apply(&mut self, arg: DeprecatedArg) { - match arg { - DeprecatedArg::Shorthand(Shorthand { num, mode, follow }) => { - self.number = num; - self.mode = mode; - self.follow = follow.then_some(FollowMode::Descriptor); - } - DeprecatedArg::File(file) => { - self.inputs.push(file); - } - } + // We don't use it, but the first argument is the binary name. + let _ = iter.next(); + + let shorthand = iter.next()?; + let input = iter.next()?; + + // We can only have a maximum of 2 arguments in this format + // The error doesn't really matter because we'll ignore any errors + // from this format. + if iter.next().is_some() { + return None; } -} -struct Shorthand { - num: SigNum, - mode: Mode, - follow: bool, -} + // Parse the shorthand by turning it into a String (via OsString) + // The format we're parsing is `{+/-}[NUM][bcl][f]`. + let os_string = shorthand.into(); -// This is not technically 100% compatible with GNU, because the shorthand can -// appear as any argument, not just the first. -impl Value for Shorthand { - fn from_value(value: &std::ffi::OsStr) -> uutils_args::ValueResult { - let s = String::from_value(value)?; + // The part of the string that is not parsed yet + let mut rest = os_string.to_str()?; - let mut rest: &str = &s; - - let sig = if let Some(r) = rest.strip_prefix('-') { - rest = r; - SigNum::Negative - } else if let Some(r) = rest.strip_prefix('+') { - rest = r; - SigNum::Positive - } else { - return Err("Invalid shorthand".into()); - }; - - // Find and parse the number part of the string - let end_num = rest - .find(|c: char| !c.is_ascii_digit()) - .unwrap_or(rest.len()); - let num = rest[..end_num].parse().unwrap_or(10); - rest = &rest[end_num..]; - - let mode = if let Some(r) = rest.strip_prefix('l') { - rest = r; - Mode::Lines - } else if let Some(r) = rest.strip_prefix('c') { - rest = r; - Mode::Bytes - } else if let Some(r) = rest.strip_prefix('b') { - rest = r; - Mode::Blocks - } else { - Mode::Lines - }; - - let follow = if let Some(r) = rest.strip_prefix('f') { - rest = r; - true - } else { - false - }; - - if !rest.is_empty() { - return Err("Invalid shorthand!".into()); - } - - Ok(Self { - num: sig(num), - mode, - follow, - }) + // Corner case: If it's just `-` then it needs to be parsed like + // the nondeprecated syntax, because `-` represents standard input. + // Curiously, GNU parses `tail + a.txt` as the deprecated syntax. + if rest == "-" { + return None; } + + // Corner case: `tail -c 10` is ambiguous and should be interpreted as + // `tail -c10 -`, not as `tail -c10 10`. All other things in this syntax + // do not create problems. For example, `tail -f a` has the same effect in + // this syntax and normal parsing. + if rest == "-c" { + return None; + } + + // Parse the sign + let sig = if let Some(r) = rest.strip_prefix('-') { + rest = r; + SigNum::Negative + } else if let Some(r) = rest.strip_prefix('+') { + rest = r; + SigNum::Positive + } else { + return None; + }; + + // Find and parse the number part of the string + let end_num = rest + .find(|c: char| !c.is_ascii_digit()) + .unwrap_or(rest.len()); + let num = rest[..end_num].parse().unwrap_or(10); + rest = &rest[end_num..]; + + // Parse the mode, one of `b`, `c`, 'l`. + let mode = if let Some(r) = rest.strip_prefix('l') { + rest = r; + Mode::Lines + } else if let Some(r) = rest.strip_prefix('c') { + rest = r; + Mode::Bytes + } else if let Some(r) = rest.strip_prefix('b') { + rest = r; + Mode::Blocks + } else { + Mode::Lines + }; + + // Parse the `f` + let follow = if let Some(r) = rest.strip_prefix('f') { + rest = r; + Some(FollowMode::Descriptor) + } else { + None + }; + + if !rest.is_empty() { + return None; + } + + Some(Settings { + number: sig(num), + mode, + follow, + inputs: vec![input.into().into()], + ..Settings::initial() + }) } #[derive(Arguments)] enum Arg { - // TODO: Bytes and Lines should take a `SigNum` #[option("-c NUM", "--bytes=NUM")] - Bytes(u64), + Bytes(SigNum), #[option("-f", "--follow[=HOW]", default=FollowMode::Descriptor)] Follow(FollowMode), @@ -104,7 +118,7 @@ enum Arg { MaxUnchangedStats(u32), #[option("-n NUM", "--lines=NUM")] - Lines(u64), + Lines(SigNum), #[option("--pid=PID")] Pid(u64), @@ -138,6 +152,45 @@ enum SigNum { Negative(u64), } +impl Value for SigNum { + fn from_value(value: &std::ffi::OsStr) -> uutils_args::ValueResult { + let s = String::from_value(value)?; + let mut rest: &str = &s; + + let sign = if let Some(r) = s.strip_prefix('+') { + rest = r; + Self::Positive + } else if let Some(r) = s.strip_prefix('-') { + rest = r; + Self::Negative + } else { + Self::Negative + }; + + // Get the number from the front of the string + let end_num = rest + .find(|c: char| !c.is_ascii_digit()) + .unwrap_or(rest.len()); + let num = rest[..end_num].parse().unwrap_or(1); + rest = &rest[end_num..]; + + // Determine the multiplier + let multiplier = match rest { + "" => 1, + "b" => 512, // blocks + "K" => 1024, // kibibytes + "M" => 1024 * 1024, // mebibytes + "G" => 1024 * 1024 * 1024, // gibibytes + "KB" => 1000, // kilobytes + "MB" => 1000 * 1000, // megabytes + "GB" => 1000 * 1000 * 1000, // gigabytes + _ => return Err(format!("Invalid number of lines: {s}").into()), + }; + + Ok(sign(num * multiplier)) + } +} + impl Default for SigNum { fn default() -> Self { Self::Negative(10) @@ -166,7 +219,7 @@ struct Settings { max_unchanged_stats: u32, mode: Mode, number: SigNum, - // Should be a dedicated PID type + // TODO: Should be a dedicated PID type pid: u64, retry: bool, sleep_sec: u64, @@ -181,11 +234,11 @@ impl Options for Settings { match arg { Arg::Bytes(n) => { self.mode = Mode::Bytes; - self.number = SigNum::Negative(n); + self.number = n; } Arg::Lines(n) => { self.mode = Mode::Lines; - self.number = SigNum::Negative(n); + self.number = n; } Arg::Follow(mode) => self.follow = Some(mode), Arg::FollowRetry => { @@ -210,9 +263,12 @@ where I: IntoIterator + Clone + 'static, I::Item: Into, { - >::try_parse(iter.clone()) - .or_else(|_| >::try_parse(iter)) + match parse_deprecated(iter.clone()) { + Some(s) => Ok(s), + None => Settings::try_parse(iter), + } } + #[test] fn shorthand() { let s = parse_tail(["tail", "-20", "somefile"]).unwrap(); @@ -229,4 +285,81 @@ fn shorthand() { assert_eq!(s.number, SigNum::Negative(100)); assert_eq!(s.mode, Mode::Bytes); assert_eq!(s.follow, Some(FollowMode::Descriptor)); + + // Corner case where the shorthand does not apply + let s = parse_tail(["tail", "-c", "42"]).unwrap(); + assert_eq!(s.number, SigNum::Negative(42)); + assert_eq!(s.mode, Mode::Bytes); + assert_eq!(s.inputs, Vec::::new()); +} + +#[test] +fn standard_input() { + let s = parse_tail(["tail", "-"]).unwrap(); + assert_eq!(s.inputs, vec![PathBuf::from("-")]) +} + +#[test] +fn normal_format() { + let s = parse_tail(["tail", "-c", "20", "somefile"]).unwrap(); + assert_eq!(s.number, SigNum::Negative(20)); + assert_eq!(s.mode, Mode::Bytes); +} + +#[test] +fn signum() { + let s = parse_tail(["tail", "-n", "20"]).unwrap(); + assert_eq!(s.number, SigNum::Negative(20)); + let s = parse_tail(["tail", "-n", "-20"]).unwrap(); + assert_eq!(s.number, SigNum::Negative(20)); + let s = parse_tail(["tail", "-n", "+20"]).unwrap(); + assert_eq!(s.number, SigNum::Positive(20)); + + let s = parse_tail(["tail", "-n", "20b"]).unwrap(); + assert_eq!(s.number, SigNum::Negative(20 * 512)); + let s = parse_tail(["tail", "-n", "+20b"]).unwrap(); + assert_eq!(s.number, SigNum::Positive(20 * 512)); + + let s = parse_tail(["tail", "-n", "b"]).unwrap(); + assert_eq!(s.number, SigNum::Negative(512)); + let s = parse_tail(["tail", "-n", "+b"]).unwrap(); + assert_eq!(s.number, SigNum::Positive(512)); + + assert!(parse_tail(["tail", "-n", "20invalid_suffix"]).is_err()); +} + +#[test] +fn followmode() { + // Sanity check: should be None initially + let s = parse_tail(["tail"]).unwrap(); + assert_eq!(s.follow, None); + + let s = parse_tail(["tail", "--follow"]).unwrap(); + assert_eq!(s.follow, Some(FollowMode::Descriptor)); + + let s = parse_tail(["tail", "-f"]).unwrap(); + assert_eq!(s.follow, Some(FollowMode::Descriptor)); + + let s = parse_tail(["tail", "--follow=descriptor"]).unwrap(); + assert_eq!(s.follow, Some(FollowMode::Descriptor)); + + let s = parse_tail(["tail", "--follow=des"]).unwrap(); + assert_eq!(s.follow, Some(FollowMode::Descriptor)); + + let s = parse_tail(["tail", "--follow=d"]).unwrap(); + assert_eq!(s.follow, Some(FollowMode::Descriptor)); + + let s = parse_tail(["tail", "--follow=name"]).unwrap(); + assert_eq!(s.follow, Some(FollowMode::Name)); + + let s = parse_tail(["tail", "--follow=na"]).unwrap(); + assert_eq!(s.follow, Some(FollowMode::Name)); + + let s = parse_tail(["tail", "--follow=n"]).unwrap(); + assert_eq!(s.follow, Some(FollowMode::Name)); + + assert!(parse_tail(["tail", "--follow="]).is_err()); + + let s = parse_tail(["tail", "-F"]).unwrap(); + assert_eq!(s.follow, Some(FollowMode::Name)); }