Skip to content

Conversation

@ChrisDryden
Copy link
Collaborator

This PR is based on the investigation of the read-error gnu test failures. Even though we correctly identify that the test is trying to use a directory we are failing the test because the error coming from the read function is different. This led me down the path to discover that we actually do not need to do additional stat checks to check if we are trying to read a directory because that is handled by read() and it is the expectation that we rely on the read() checks to provide the error message.

Every utility in this PR already had integration tests for validating that the error message matches the GNU error message and I have made them more exact by also validating error code and making it stderr_is instead of stderr_contains.

There will probably be two more PR's basically the same as this one for the other utilities

@ChrisDryden
Copy link
Collaborator Author

Hmm I have to figure out what to do for windows here, this is not defined behavior compared to GNU

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 12, 2026

Merging this PR will degrade performance by 4.75%

❌ 2 regressed benchmarks
✅ 282 untouched benchmarks
⏩ 38 skipped benchmarks1

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

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation unexpand_many_lines[100000] 189.7 ms 199.2 ms -4.75%
Simulation unexpand_large_file[10] 397.7 ms 417.6 ms -4.75%

Comparing ChrisDryden:fix-read-errors-simple (7509656) with main (8e59663)

Open in CodSpeed

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.

@github-actions
Copy link

GNU testsuite comparison:

Skipping an intermittent issue tests/shuf/shuf-reservoir (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/sort/sort-stale-thread-mem (passes in this run but fails in the 'main' branch)

@ChrisDryden ChrisDryden marked this pull request as draft February 12, 2026 15:18
@ChrisDryden
Copy link
Collaborator Author

It looks like the fix here is to make the is_dir checks a windows only thing instead of removing it entirely

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.

1 participant