Skip to content

Conversation

naoNao89
Copy link
Contributor

This PR adds test coverage to improve alignment with GNU coreutils for ls, tee, and cat as part of addressing regressions tracked in #4627.

Summary of changes

  • ls: Add focused tests for TIME_STYLE behavior and precedence
    • TIME_STYLE from environment (e.g., TIME_STYLE=full-iso)
    • --time-style=iso formatting for recent vs older timestamps
    • Precedence and interaction between --full-time and --time-style (last one wins)
    • Behavior under LC_ALL=POSIX (posix-* fallbacks / locale-style formatting)
    • Sorting effects without -l when using time keys (-u/-c), to reflect GNU behavior
  • tee: Add tests for --output-error
    • Presence-only flag defaults to warn-nopipe
    • Broken-pipe robustness (no crash; exit status and stderr tolerant to platform differences)
    • Error reporting on invalid output targets
  • cat: Add broken pipe test
    • Verifies robustness in the face of SIGPIPE-like conditions without crashing
    • Removed an invalid test that used an unsupported -o flag

Issue linkage

  • Refs Regressions compared to GNU coreutils 9.2, 9.3 and 9.4 #4627
  • The added tests directly map to the remaining GNU test cases highlighted in the issue:
    • tests/misc/tee.sh
    • tests/misc/write-errors.sh (tee and cat broken pipe behavior)
    • tests/ls/ls-time.sh (TIME_STYLE, precedence, locale behavior, and time-sorting without -l)

Local validation

  • cargo test -p uu_ls -p uu_tee -p uu_cat --tests succeeds locally.
  • cargo fmt and targeted cargo clippy (uu_ls, uu_tee, uu_cat) pass locally.

Implementation notes

  • Tests are written to be robust across platforms. For broken pipe scenarios, assertions avoid brittle, platform-specific exit codes and focus on non-crash behavior and presence of diagnostics where applicable. Some tests are guarded with #[cfg(unix)] to reflect SIGPIPE semantics.
  • If CI reports failures, they may indicate platform differences or real implementation gaps. We’re happy to follow up with additional fixes in separate PRs where needed (e.g., refining tee’s --output-error behavior, ls TIME_STYLE edge cases, or cat’s SIGPIPE handling) to further close the gap with GNU coreutils.

Copy link

codspeed-hq bot commented Sep 20, 2025

CodSpeed Performance Report

Merging #8684 will not alter performance

Comparing naoNao89:test/gnu-compat_write-errors_and_ls-time-style_4627 (82646c7) with main (32eef06)

Summary

✅ 93 untouched
⏩ 1 skipped1

Footnotes

  1. 1 benchmark was skipped, so the baseline result was used instead. If it was deleted from the codebase, click here and archive it to remove it from the performance reports.

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

@naoNao89 naoNao89 force-pushed the test/gnu-compat_write-errors_and_ls-time-style_4627 branch from 042f6a4 to cdaf2be Compare September 21, 2025 14:19
@naoNao89 naoNao89 force-pushed the test/gnu-compat_write-errors_and_ls-time-style_4627 branch from cdaf2be to c72abe0 Compare September 21, 2025 14:24
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)

@naoNao89 naoNao89 force-pushed the test/gnu-compat_write-errors_and_ls-time-style_4627 branch from f502f9b to 17d4da3 Compare September 21, 2025 20:11
- chcon, runcon: replace crate-level cfg with entrypoint guard and add a no-op main on non-Linux to avoid build failures.

- ls: enable SELinux support only when both feature="selinux" and target_os="linux"; default to false otherwise.

- tests(by-util/test_cat.rs): correctly drop the read_end handle to ensure writers see EPIPE (rename _read_end -> read_end and drop it).

Refs: uutils#4627
@naoNao89 naoNao89 force-pushed the test/gnu-compat_write-errors_and_ls-time-style_4627 branch from 17d4da3 to 6ac5031 Compare September 21, 2025 20:28
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)

…id CI hangs (PR uutils#8684)\n\n- Guard test with #[cfg(all(unix, not(target_os = "freebsd")))]\n- Rationale: FreeBSD workflow repeatedly times out on this test in vmactions/freebsd-vm\n- Keeps coverage on other Unix platforms while unblocking FreeBSD CI
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/usage_vs_getopt (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

naoNao89 added a commit to naoNao89/coreutils that referenced this pull request Sep 22, 2025
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@naoNao89 naoNao89 force-pushed the test/gnu-compat_write-errors_and_ls-time-style_4627 branch from 2cce039 to e777d86 Compare September 22, 2025 11:50
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

1 similar comment
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

@naoNao89 naoNao89 force-pushed the test/gnu-compat_write-errors_and_ls-time-style_4627 branch from 9d4f820 to 30835b7 Compare September 22, 2025 18:40
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/timeout/timeout (passes in this run but fails in the 'main' branch)

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)

Copy link

GNU testsuite comparison:

GNU test failed: tests/tail/overlay-headers. tests/tail/overlay-headers is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/misc/tee (fails in this run but passes in the 'main' branch)


#[cfg(feature = "feat_external_libstdbuf")]
#[test]
fn test_permission_external_missing_lib() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ecordonnier wdyt ? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be written differently long-term:

  • one corner-case is that this will fail in case the user ran "make install" before running this test
  • testing "Permission denied" while building with feat_external_libstdbuf adds more value than skipping "test_permission" and testing for the "missing libstdbuf" error message
  • one way to fix this is by implementing the same logic than in GNU coreutils: stdbuf should search for libstdbuf.so first in the path where stdbuf is currently located, and then in LIBSTDBUF_DIR. that way it would be easy to provide a symlink to libstdbuf.so for testing purpose (without requiring make install / root access)
  • if you need to merge it now please add a big TODO

Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/misc/stdbuf (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

…x gating for ls

- chcon/runcon: add rationale comments for item-level cfg with stub main() on non-Linux
  to keep binary targets present across platforms (fixes reviewer request)
- cat tests: remove top-of-file banner; add scoped comment and replace tautology
  with assert on process exit to ensure no hang/crash in broken pipe scenario
- tee tests: replace tautological assertion with explicit exit presence check
  in broken pipe scenario (Unix, non-FreeBSD)
- ls: gate SecurityContext lookup under all(feature=selinux, target_os=linux) to
  avoid unresolved symbols when building with all features; maintain String
  return path across cfg branches

These changes address @sylvestre’s comments and improve cross-platform stability
while preserving GNU-compat coverage for TIME_STYLE, broken pipes, and stdbuf.
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/misc/tee (passes in this run but fails in the 'main' branch)

EOPNOTSUPP
EPERM
EROFS
EPIPE
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this doesn't respect the existing alphabetical order


#[cfg(feature = "feat_external_libstdbuf")]
#[test]
fn test_permission_external_missing_lib() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs to be written differently long-term:

  • one corner-case is that this will fail in case the user ran "make install" before running this test
  • testing "Permission denied" while building with feat_external_libstdbuf adds more value than skipping "test_permission" and testing for the "missing libstdbuf" error message
  • one way to fix this is by implementing the same logic than in GNU coreutils: stdbuf should search for libstdbuf.so first in the path where stdbuf is currently located, and then in LIBSTDBUF_DIR. that way it would be easy to provide a symlink to libstdbuf.so for testing purpose (without requiring make install / root access)
  • if you need to merge it now please add a big TODO

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants