From 59cd6e5e4141e367ce2a2721cd3b154473e3d0e5 Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Sat, 22 Mar 2025 14:18:38 +0100 Subject: [PATCH 1/2] tests: Move seq/yes run function to Ucommand::run_stdout_starts_with Tests for both `seq` and `yes` run a command that never terminates, and check the beggining of their output in stdout, move the copied parts of the wrapper function to common/util. We still need to use slightly different logic to parse exit value as `seq` returns success if stdout gets closed, while `yes` fails. --- tests/by-util/test_seq.rs | 48 +++++++++++++++++++-------------------- tests/by-util/test_yes.rs | 13 ++++------- tests/common/util.rs | 12 ++++++++++ 3 files changed, 40 insertions(+), 33 deletions(-) diff --git a/tests/by-util/test_seq.rs b/tests/by-util/test_seq.rs index 1fae070f4..25b400864 100644 --- a/tests/by-util/test_seq.rs +++ b/tests/by-util/test_seq.rs @@ -4,7 +4,6 @@ // file that was distributed with this source code. // spell-checker:ignore lmnop xlmnop use crate::common::util::TestScenario; -use std::process::Stdio; #[test] fn test_invalid_arg() { @@ -566,51 +565,52 @@ fn test_width_floats() { .stdout_only("09.0\n10.0\n"); } -// TODO This is duplicated from `test_yes.rs`; refactor them. -/// Run `seq`, capture some of the output, close the pipe, and verify it. -fn run(args: &[&str], expected: &[u8]) { - let mut cmd = new_ucmd!(); - let mut child = cmd.args(args).set_stdout(Stdio::piped()).run_no_wait(); - let buf = child.stdout_exact_bytes(expected.len()); - child.close_stdout(); - child.wait().unwrap().success(); - assert_eq!(buf.as_slice(), expected); -} - #[test] fn test_neg_inf() { - run(&["--", "-inf", "0"], b"-inf\n-inf\n-inf\n"); + new_ucmd!() + .args(&["--", "-inf", "0"]) + .run_stdout_starts_with(b"-inf\n-inf\n-inf\n") + .success(); } #[test] fn test_neg_infinity() { - run(&["--", "-infinity", "0"], b"-inf\n-inf\n-inf\n"); + new_ucmd!() + .args(&["--", "-infinity", "0"]) + .run_stdout_starts_with(b"-inf\n-inf\n-inf\n") + .success(); } #[test] fn test_inf() { - run(&["inf"], b"1\n2\n3\n"); + new_ucmd!() + .args(&["inf"]) + .run_stdout_starts_with(b"1\n2\n3\n") + .success(); } #[test] fn test_infinity() { - run(&["infinity"], b"1\n2\n3\n"); + new_ucmd!() + .args(&["infinity"]) + .run_stdout_starts_with(b"1\n2\n3\n") + .success(); } #[test] fn test_inf_width() { - run( - &["-w", "1.000", "inf", "inf"], - b"1.000\n inf\n inf\n inf\n", - ); + new_ucmd!() + .args(&["-w", "1.000", "inf", "inf"]) + .run_stdout_starts_with(b"1.000\n inf\n inf\n inf\n") + .success(); } #[test] fn test_neg_inf_width() { - run( - &["-w", "1.000", "-inf", "-inf"], - b"1.000\n -inf\n -inf\n -inf\n", - ); + new_ucmd!() + .args(&["-w", "1.000", "-inf", "-inf"]) + .run_stdout_starts_with(b"1.000\n -inf\n -inf\n -inf\n") + .success(); } #[test] diff --git a/tests/by-util/test_yes.rs b/tests/by-util/test_yes.rs index 26a7b14ac..9f5f84ed8 100644 --- a/tests/by-util/test_yes.rs +++ b/tests/by-util/test_yes.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. use std::ffi::OsStr; -use std::process::{ExitStatus, Stdio}; +use std::process::ExitStatus; #[cfg(unix)] use std::os::unix::process::ExitStatusExt; @@ -22,15 +22,10 @@ fn check_termination(result: ExitStatus) { const NO_ARGS: &[&str] = &[]; -/// Run `yes`, capture some of the output, close the pipe, and verify it. +/// Run `yes`, capture some of the output, then check exit status. fn run(args: &[impl AsRef], expected: &[u8]) { - let mut cmd = new_ucmd!(); - let mut child = cmd.args(args).set_stdout(Stdio::piped()).run_no_wait(); - let buf = child.stdout_exact_bytes(expected.len()); - child.close_stdout(); - - check_termination(child.wait().unwrap().exit_status()); - assert_eq!(buf.as_slice(), expected); + let result = new_ucmd!().args(args).run_stdout_starts_with(expected); + check_termination(result.exit_status()); } #[test] diff --git a/tests/common/util.rs b/tests/common/util.rs index d6352b993..ec332c046 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -1846,6 +1846,18 @@ impl UCommand { let tmpdir_path = self.tmpd.as_ref().unwrap().path(); format!("{}/{file_rel_path}", tmpdir_path.to_str().unwrap()) } + + /// Runs the command, checks that the stdout starts with "expected", + /// then terminates the command. + #[track_caller] + pub fn run_stdout_starts_with(&mut self, expected: &[u8]) -> CmdResult { + let mut child = self.set_stdout(Stdio::piped()).run_no_wait(); + let buf = child.stdout_exact_bytes(expected.len()); + child.close_stdout(); + + assert_eq!(buf.as_slice(), expected); + child.wait().unwrap() + } } impl std::fmt::Display for UCommand { From 596ea0a6947bc56d6f12ca552b4395ec47772e0d Mon Sep 17 00:00:00 2001 From: Nicolas Boichat Date: Sat, 22 Mar 2025 13:44:09 +0100 Subject: [PATCH 2/2] test_seq: Add a few more tests for corner cases Some of these tests are not completely defined behavior, but in many cases they make sense (or at least one can find some consistent logic to it). However, there are 2 edge cases that are more dubious IMHO. One of them has been reported on list a while back, and I just reported another. --- tests/by-util/test_seq.rs | 108 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/tests/by-util/test_seq.rs b/tests/by-util/test_seq.rs index 25b400864..95caa0ccb 100644 --- a/tests/by-util/test_seq.rs +++ b/tests/by-util/test_seq.rs @@ -936,3 +936,111 @@ fn test_parse_valid_hexadecimal_float_format_issues() { .succeeds() .stdout_only("9.92804e-09\n1\n"); } + +// GNU `seq` manual states that, when the parameters "all use a fixed point +// decimal representation", the format should be `%.pf`, where the precision +// is inferred from parameters. Else, `%g` is used. +// +// This is understandable, as translating hexadecimal precision to decimal precision +// is not straightforward or intuitive to the user. There are some exceptions though, +// if a mix of hexadecimal _integers_ and decimal floats are provided. +// +// The way precision is inferred is said to be able to "represent the output numbers +// exactly". In practice, this means that trailing zeros in first/increment number are +// considered, but not in the last number. This makes sense if we take that last number +// as a "bound", and not really part of input/output. +#[test] +fn test_precision_corner_cases() { + // Mixed input with integer hex still uses precision in decimal float + new_ucmd!() + .args(&["0x1", "0.90", "3"]) + .succeeds() + .stdout_is("1.00\n1.90\n2.80\n"); + + // Mixed input with hex float reverts to %g + new_ucmd!() + .args(&["0x1.00", "0.90", "3"]) + .succeeds() + .stdout_is("1\n1.9\n2.8\n"); + + // Even if it's the last number. + new_ucmd!() + .args(&["1", "1.20", "0x3.000000"]) + .succeeds() + .stdout_is("1\n2.2\n"); + + // Otherwise, precision in last number is ignored. + new_ucmd!() + .args(&["1", "1.20", "3.000000"]) + .succeeds() + .stdout_is("1.00\n2.20\n"); + + // Infinity is ignored + new_ucmd!() + .args(&["1", "1.2", "inf"]) + .run_stdout_starts_with(b"1.0\n2.2\n3.4\n") + .success(); +} + +// GNU `seq` manual only makes guarantees about `-w` working if the +// provided numbers "all use a fixed point decimal representation", +// and guides the user to use `-f` for other cases. +#[test] +fn test_equalize_widths_corner_cases() { + // Mixed input with hexadecimal does behave as expected + new_ucmd!() + .args(&["-w", "0x1", "5.2", "9"]) + .succeeds() + .stdout_is("1.0\n6.2\n"); + + // Confusingly, the number of integral digits in the last number is + // used to pad the output numbers, while it is ignored for precision + // computation. + // + // This problem has been reported on list here: + // "bug#77179: seq incorrectly(?) pads output when last parameter magnitude" + // https://lists.gnu.org/archive/html/bug-coreutils/2025-03/msg00028.html + // + // TODO: This case could be handled correctly, consider fixing this in + // `uutils` implementation. Output should probably be "1.0\n6.2\n". + new_ucmd!() + .args(&["-w", "0x1", "5.2", "10.0000"]) + .succeeds() + .stdout_is("01.0\n06.2\n"); + + // But if we fixed the case above, we need to make sure we still pad + // if the last number in the output requires an extra digit. + new_ucmd!() + .args(&["-w", "0x1", "5.2", "15.0000"]) + .succeeds() + .stdout_is("01.0\n06.2\n11.4\n"); + + // GNU `seq` bails out if any hex float is in the output. + // Unlike the precision corner cases above, it is harder to justify + // this behavior for hexadecimal float inputs, as it is always be + // possible to output numbers with a fixed width. + // + // This problem has been reported on list here: + // "bug#76070: Subject: seq, hexadecimal args and equal width" + // https://lists.gnu.org/archive/html/bug-coreutils/2025-02/msg00007.html + // + // TODO: These cases could be handled correctly, consider fixing this in + // `uutils` implementation. + // If we ignore hexadecimal precision, the output should be "1.0\n6.2\n". + new_ucmd!() + .args(&["-w", "0x1.0000", "5.2", "10"]) + .succeeds() + .stdout_is("1\n6.2\n"); + // The equivalent `seq -w 1.0625 1.00002 3` correctly pads the first number: "1.06250\n2.06252\n" + new_ucmd!() + .args(&["-w", "0x1.1", "1.00002", "3"]) + .succeeds() + .stdout_is("1.0625\n2.06252\n"); + + // We can't really pad with infinite number of zeros, so `-w` is ignored. + // (there is another test with infinity as an increment above) + new_ucmd!() + .args(&["-w", "1", "1.2", "inf"]) + .run_stdout_starts_with(b"1.0\n2.2\n3.4\n") + .success(); +}