Summary
executor::tests::conformance::landing::a_directory_at_the_destination_fails_the_shell_write fails intermittently on CI, and the failure is in the test harness rather than in anything it is testing:
thread '...a_directory_at_the_destination_fails_the_shell_write' panicked at src/executor.rs:2995:22:
write contents: Os { code: 32, kind: BrokenPipe, message: "Broken pipe" }
Seen on run 31291172351 (371 passed, 1 failed); the same commit's code passed the same job on earlier runs, which is what a race looks like.
PUT_FILE_SCRIPT refuses a directory at the destination in its first four lines:
if [ -d "$dest" ]; then
echo "destination $dest is a directory" >&2
exit 1
fi
That is before cat > "$tmp", so on this path the shell never reads its stdin and exits with the read end closed. Meanwhile run_landing_script writes the contents and .expect("write contents")s the result. Two outcomes race: the eight bytes reach the pipe buffer before the shell is gone, and the write succeeds; or the shell exits first, and the write gets EPIPE. A loaded runner makes the second more likely, and nothing about the behaviour under test decides which happens.
The refusal being tested is the exit status and the stderr the script produces. Whether the parent managed to hand over bytes nobody was going to read is not part of it.
Scope
In run_landing_script, accept ErrorKind::BrokenPipe from the write_all as one of the script's legitimate outcomes and let the existing assertions on Output deliver the verdict; keep panicking on every other write error. The pipe must still be closed explicitly before wait_with_output, or cat on the paths that do read stdin never sees EOF.
PUT_FILE_SCRIPT itself does not change: refusing before staging anything is the correct order, and reading stdin to be polite about a write that is being refused would be the wrong repair.
Non-goals
Every other test in mod landing. Only the helper they share is at fault, and only on the paths that refuse early.
Acceptance criteria
Note
Found by #41 (issue #37), whose branch touches no part of src/executor.rs. It lands separately for the same reason #42 did: that pull request's diff is fixed at the four files its issue names.
Summary
executor::tests::conformance::landing::a_directory_at_the_destination_fails_the_shell_writefails intermittently on CI, and the failure is in the test harness rather than in anything it is testing:Seen on run 31291172351 (371 passed, 1 failed); the same commit's code passed the same job on earlier runs, which is what a race looks like.
PUT_FILE_SCRIPTrefuses a directory at the destination in its first four lines:That is before
cat > "$tmp", so on this path the shell never reads its stdin and exits with the read end closed. Meanwhilerun_landing_scriptwrites the contents and.expect("write contents")s the result. Two outcomes race: the eight bytes reach the pipe buffer before the shell is gone, and the write succeeds; or the shell exits first, and the write getsEPIPE. A loaded runner makes the second more likely, and nothing about the behaviour under test decides which happens.The refusal being tested is the exit status and the stderr the script produces. Whether the parent managed to hand over bytes nobody was going to read is not part of it.
Scope
In
run_landing_script, acceptErrorKind::BrokenPipefrom thewrite_allas one of the script's legitimate outcomes and let the existing assertions onOutputdeliver the verdict; keep panicking on every other write error. The pipe must still be closed explicitly beforewait_with_output, orcaton the paths that do read stdin never sees EOF.PUT_FILE_SCRIPTitself does not change: refusing before staging anything is the correct order, and reading stdin to be polite about a write that is being refused would be the wrong repair.Non-goals
Every other test in
mod landing. Only the helper they share is at fault, and only on the paths that refuse early.Acceptance criteria
run_landing_scripttreats aBrokenPipewrite as a non-failure and panics on any other write error.cargo fmt -- --check --config group_imports=StdExternalCrate,cargo clippy --all-targets -- -D warnings,cargo clippy --all-targets --features test-support -- -D warnings,cargo testandcargo test --features test-supportall pass.Note
Found by #41 (issue #37), whose branch touches no part of
src/executor.rs. It lands separately for the same reason #42 did: that pull request's diff is fixed at the four files its issue names.