-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
tail: fix issue #6543 (--pid when reading from stdin) #6582
base: main
Are you sure you want to change the base?
Changes from 5 commits
addf7b5
03b3bf1
05262b6
aa8f2af
bbefeba
cbbef09
520e8b8
b242461
465ef45
731e4e1
fed2705
a4cc3a7
e7d1c0b
1ade9ac
0811fea
73bbd9a
bcc144b
b8c5bc8
b120a56
8c5097a
1f0d9d5
98c546f
09e2610
59aea7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4824,3 +4824,38 @@ fn test_obsolete_encoding_windows() { | |
.stderr_is("tail: bad argument encoding: '-�b'\n") | ||
.code_is(1); | ||
} | ||
|
||
#[test] | ||
fn test_following_with_pid() { | ||
use std::process::Command; | ||
|
||
let scene = TestScenario::new(util_name!()); | ||
|
||
let sleep_command = Command::new("sleep") | ||
.arg("999d") | ||
.spawn() | ||
.expect("failed to start sleep command"); | ||
|
||
let sleep_pid = sleep_command.id(); | ||
|
||
// when -f is specified, tail should die after | ||
// the pid from --pid also dies | ||
let mut child = scene | ||
.ucmd() | ||
.args(&["--pid", &sleep_pid.to_string(), "-f"]) | ||
.set_stdin(Stdio::null()) | ||
.stderr_to_stdout() | ||
.run_no_wait(); | ||
|
||
child.make_assertion_with_delay(1000).is_alive(); | ||
|
||
Command::new("kill") | ||
.arg("-9") | ||
.arg(sleep_pid.to_string()) | ||
.output() | ||
.expect("failed to kill sleep command"); | ||
|
||
child.make_assertion_with_delay(100).is_alive(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 0.1 seconds is a bit very short. Again, some CI runners are really slow, that's why I'm worried that this would result in a flaky test (we already have too many of them). Maybe increase it to 1 second, i.e. 1000 milliseconds? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright. I'll go ahead and do that, then push. I'm usually of the mindset of trying to make sure any one test doesn't take too long, because it begins to add up over a large test suite. But I'd hate to make a weak or flaky test |
||
|
||
child.kill(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After killing
sleep
, we have a perfect opportunity to quickly check thattail
really does terminate by itself. Maybe insert something like this?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, for checking if it's
is_alive
, why notis_not_alive
. Running the commands manually, I confirm that the tail exits after sleep exits, and after it's killed. But, I changed it tois_not_alive
, and it failed on Linux. Although it passed on Windows. I need to switch back over to Windows and confirm it's breaking nowThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To answer my previous question, I believe it should be
is_not_alive
, and the cause for failure was because sleep's parent was the Rust test program, and without exiting, and without reaping, it became a zombie. However, the libc kill function inis_dead
saw it as still alive (returning0
), and the reason the program exited before was because ofrecv_timeout(settings.sleep_sec)
in watch.rs (currently line 532 in this commit), which I assume errored out, leading to the program ending, and leading to the appearance that it detected the pid specified dying and following suite, when it wasn't actually the case.