Skip to content

Conversation

@sylvestre
Copy link
Contributor

instead of #10219

The test assumed that the process always has exactly one TTY in its
file descriptors. In CI environments or when running tests without a
terminal attached, there is no controlling TTY, causing the test to
fail with `assert_eq!(result.len(), 1)` when `result.len()` is 0.
- Add `set -o pipefail` to ensure test failures from cargo nextest are
  detected even when output is piped through grep
- Look for llvm-profdata in the nightly toolchain (nightly-gnu) since
  that's what the script uses for coverage builds
- Add helpful error message if llvm-profdata is not found
Add LANG=C environment variable to the FIFO tests alongside LC_ALL=C
and LANGUAGE=C. The Fluent localization system checks multiple locale
environment variables, and all three are needed to ensure English
output in the test assertions.
The previous implementation checked if Errno::last() was UnknownErrno
to determine success, but this is incorrect because errno is not
cleared on success. A previous syscall could have set errno to a value
like ENOENT, causing getsid to incorrectly report an error.

The correct pattern is to check if the result is -1, which is what
getsid returns on error per POSIX.
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 17, 2026

CodSpeed Performance Report

Merging this PR will degrade performance by 4.25%

Comparing sylvestre:fix-pid (cf2806f) with main (2a044db)

Summary

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 280 untouched benchmarks
⏩ 38 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Memory du_wide_tree[(5000, 500)] 1.2 MB 1.3 MB -4.25%
Memory cp_large_file[16] 118.9 KB 112.9 KB +5.25%

Footnotes

  1. 38 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@ChrisDryden ChrisDryden merged commit cd288fc into uutils:main Jan 17, 2026
158 of 159 checks passed
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.

2 participants