testscript: add kill command #243
Conversation
testscript/cmd.go
Outdated
| if bg == nil { | ||
| ts.Fatalf("unknown background process %q", bgName) | ||
| } | ||
| signal := os.Interrupt |
There was a problem hiding this comment.
Might be more generic to be able to choose the signal? (hup vs int vs kill...)
There was a problem hiding this comment.
Yeah, I was thinking that. I was wondering about the API for that. Would you be happy with kill [-SIG] [name]? (SIG would be numeric only in the first instance on second thought, named signal would be better). With that, the default would probably be SIGKILL.
kortschak
force-pushed
the
kill_cmd
branch
3 times, most recently
from
89324db
to
f056824
Compare
|
ping As noted in the slack conversation about this PR, if adding a |
| ! exec sleep 5 & | ||
|
|
||
| kill -KILL test_sleep | ||
| wait test_sleep |
There was a problem hiding this comment.
can you please add failure test cases? in particular:
- trying to kill a running background process that was started with
&rather than&word&(i.e. using a background process name that doesn't exist) - trying to kill a background process that already stopped or was already killed
Please also cover the kill and kill name forms, as otherwise we can't be sure they will keep working in the future.
There was a problem hiding this comment.
These negated tests are not currently possible since the kill command is not negatable. This is consistent with wait.
The positive tests for without name are possible.
There was a problem hiding this comment.
Thanks for adding more tests!
I see your point about consistency with wait not being negatable. I personally think both should be, and both should fail if given an unknown/exited process, but we can always add that later - especially once we have ? to ignore the result of a command.
I would personally also be fine with this, but given that the majority of users would presumably use that to write their own |
This allows sending a termination signal to backgrounded commands.
This allows sending a termination signal to backgrounded commands.
Please take a look.
Closes #242