Skip to content

Conversation

@ChrisDryden
Copy link
Collaborator

@ChrisDryden ChrisDryden commented Dec 16, 2025

This bug sent me caused me a bunch of difficulty while working on the locale GNU integration tests, since non utf-8 values were being piped into nl in the GNU integration tests and then coming out as UTF-8 breaking the tests. I am curious to see if this was the root cause for some more integration tests failing in the CI.

Its a bit rough to see that there were integration tests specifically made for this scenario but the tests were implemented with the same "to_string_lossy" which made the tests actually check nothing. Ideally we should add the "matches_gnu()" to all of these integration tests: #9660 so that we can detect these issues preemptively.

@codspeed-hq
Copy link

codspeed-hq bot commented Dec 16, 2025

CodSpeed Performance Report

Merging #9673 will improve performances by 75.86%

Comparing ChrisDryden:fix-nl-preserve-bytes (93c8d54) with main (ae3c51f)

Summary

⚡ 2 improvements
✅ 125 untouched
⏩ 6 skipped1

Benchmarks breakdown

Benchmark BASE HEAD Change
nl_large_file[10] 99.9 ms 57.1 ms +74.82%
nl_many_lines[100000] 79 ms 44.9 ms +75.86%

Footnotes

  1. 6 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
Copy link
Collaborator Author

Nice, didn't realize this was also the bottleneck in performance

@ChrisDryden ChrisDryden marked this pull request as ready for review December 16, 2025 13:21
@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/tail/overlay-headers (fails in this run but passes in the 'main' branch)

@cakebaker cakebaker merged commit 6b23e6f into uutils:main Dec 16, 2025
126 of 127 checks passed
@cakebaker
Copy link
Contributor

Thanks!

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