t: Make process handling more robust with IPC::Run #3123
Conversation
| my ($h, $forced) = @_; | ||
| return unless $h; | ||
| if ($forced) { | ||
| $h->kill_kill(grace => 3); |
There was a problem hiding this comment.
Should we handle the exception and print a warning? Otherwise it presumably breaks the test plan in that case.
There was a problem hiding this comment.
I don't think it does. This is basically still the same we were doing in before, we send TERM and then KILL if the parameter was given
Whenever processes are spawned they need to be tracked properly for cleanup. This posed problems of sometimes leaking orphans when tests are aborted or crashed. As we already use IPC::Run we can use the same for other cases where we spawn processes with fork by using IPC::Run::start instead. Additional benefits are that we can debug the process handling and IPC going with https://metacpan.org/pod/IPC::Run#Debugging-Tip as well as be able to catch output of the processes. One example of a recent problem that should be fixed by this is t/05-scheduler-full.t failing to stop completely when individual test steps fail. This prevents the RETRY on the level of tools/retry and the Makefile to work as the former test never completely finishes and is stuck until the CI aborts the complete test run without further retries. https://progress.opensuse.org/issues/59043
|
Test fails with but I could not reproduce this locally |
|
Also works for me locally. Maybe run prove with -v to get more info? |
|
We do have all artifacts available. In https://app.circleci.com/pipelines/github/os-autoinst/openQA/3049/workflows/51f1299a-215e-47e1-85e1-baaf4dde7d56/jobs/29008 I reran also with ssh and both that run succeeded as well as my run within the ssh session |
Codecov Report
@@ Coverage Diff @@
## master #3123 +/- ##
=======================================
Coverage 92.05% 92.05%
=======================================
Files 211 211
Lines 12932 12932
=======================================
Hits 11904 11904
Misses 1028 1028 Continue to review full report at Codecov.
|
|
All problems fixed. Somehow I have the feeling the observed problems where somewhat temporary in circle CI |
Whenever processes are spawned they need to be tracked properly for
cleanup. This posed problems of sometimes leaking orphans when tests are
aborted or crashed. As we already use IPC::Run we can use the same for
other cases where we spawn processes with fork by using IPC::Run::start
instead. Additional benefits are that we can debug the process handling
and IPC going with https://metacpan.org/pod/IPC::Run#Debugging-Tip as
well as be able to catch output of the processes.
One example of a recent problem that should be fixed by this is
t/05-scheduler-full.t failing to stop completely when individual test
steps fail. This prevents the RETRY on the level of tools/retry and the
Makefile to work as the former test never completely finishes and is
stuck until the CI aborts the complete test run without further retries.
https://progress.opensuse.org/issues/59043