Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests: Harden two tests, prevent 4 flaky tests (cat, numfmt, sort, split, tee) #6000

Merged
merged 5 commits into from
Feb 28, 2024

Conversation

BenWiederhake
Copy link
Collaborator

This PR improves the tests:

  • Two tests were unnecessarily lax (test_tee_no_more_writeable_1 ignored memory problems, the new test_check_unique_combined exercises an input that wasn't possible before clap#2624)
  • Four tests had the potential to flake: They were writing to the stdin of a process that might have already terminated.

My goal was to find any flaky tests like 9995c63: When we write a large buffer to the stdin of a process that will fail at some point, then a race happens:

  • Perhaps the write happens first, and the kernel buffer is large enough to accept it entirely. In that case, the write is successful, and the writer thread terminates normally. This is likely to happen on a developer machine.
  • Perhaps the write happens later, and the child process executes first. This will close the pipe. If we then try to write to it, the write will fail. This is unlikely to happen, but not impossible on a busy CI machine.

Arguably, these tests are unlikely to be the cause of any flakiness, but let's fix them anyway: Either it improves the situation, or makes no difference.

Now that clap#2624 has been resolved, we can and should test both variants.
See also 9995c63.

There is a race condition between the writing thread and the command.
It is easily possible that on the developer's machine, the writing
thread is always faster, filling the kernel's buffer of the stdin pipe,
thus succeeding the write. It is also easily possible that on the busy
CI machines, the child command runs first for whatever reason, and exits
early, thus killing the pipe, which causes the later write to fail. This
results in a flaky test. Let's prevent flaky tests.
@BenWiederhake
Copy link
Collaborator Author

Oh look! Other tests which have exactly the same setup (writing to stdin of a process that is failing due to cmdline args) cause real problems.

Let's merge this PR and avoid these problems with cat, numfmt, sort, split, and tee! :)

@BenWiederhake
Copy link
Collaborator Author

Oh look! Yet another test has exactly the same setup (writing to stdin of a process that is exiting before reading anything) and causes real problems.

Let's merge this PR and avoid these problems with cat, numfmt, sort, split, and tee! :)

@sylvestre sylvestre merged commit 769c5ca into uutils:main Feb 28, 2024
62 checks passed
@BenWiederhake BenWiederhake deleted the dev-pipein-fails-flake branch February 28, 2024 11:22
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