| Message ID | 20210202074753.31516-1-pvorel@suse.cz |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [RFC,1/1] tst_test.sh: Run cleanup also on timeout | expand |
Hi, On 2/2/2021 8:47 AM, Petr Vorel wrote: > Also timeout requires to run a test cleanup (e.g. zram01.sh). > Thus send first SIGINT, but keep also SIGKILL for safety reasons > (after 5 sec to give cleanup function some time. +1 > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh > index 69f007d89..35ad6d2d7 100644 > --- a/testcases/lib/tst_test.sh > +++ b/testcases/lib/tst_test.sh > @@ -21,7 +21,7 @@ export TST_LIB_LOADED=1 > . tst_security.sh > > # default trap function > -trap "tst_brk TBROK 'test interrupted'" INT > +trap "tst_brk TBROK 'test interrupted or timeout'" INT should be "timed out" for consistency > > _tst_do_exit() > { > @@ -459,7 +459,7 @@ _tst_setup_timer() > > tst_res TINFO "timeout per run is ${h}h ${m}m ${s}s" > > - sleep $sec && tst_res TBROK "test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill -9 -$pid & > + sleep $sec && tst_res TBROK "test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && { kill -2 -$pid; sleep 5; kill -9 -$pid; } & Can we move the timeout handling to a separate function? This is getting a tiny bit unreadable... Can this work?: _tst_handle_timeout() { tst_res TBROK ... kill .... } ... sleep $sec && _tst_handle_timeout & Jörg
Hi Jörg, > Hi, > On 2/2/2021 8:47 AM, Petr Vorel wrote: > > Also timeout requires to run a test cleanup (e.g. zram01.sh). > > Thus send first SIGINT, but keep also SIGKILL for safety reasons > > (after 5 sec to give cleanup function some time. > +1 > > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh > > index 69f007d89..35ad6d2d7 100644 > > --- a/testcases/lib/tst_test.sh > > +++ b/testcases/lib/tst_test.sh > > @@ -21,7 +21,7 @@ export TST_LIB_LOADED=1 > > . tst_security.sh > > # default trap function > > -trap "tst_brk TBROK 'test interrupted'" INT > > +trap "tst_brk TBROK 'test interrupted or timeout'" INT > should be "timed out" for consistency You probably mean consistency with TST_RETRY_FN_EXP_BACKOFF() I tried to be consistent with previous message: zram01 1 TBROK: test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1 That one should be changed as well. I slightly prefer "timeout", but I'm not a native speaker. > > _tst_do_exit() > > { > > @@ -459,7 +459,7 @@ _tst_setup_timer() > > tst_res TINFO "timeout per run is ${h}h ${m}m ${s}s" > > - sleep $sec && tst_res TBROK "test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill -9 -$pid & > > + sleep $sec && tst_res TBROK "test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && { kill -2 -$pid; sleep 5; kill -9 -$pid; } & > Can we move the timeout handling to a separate function? This is getting a > tiny bit unreadable... > Can this work?: > _tst_handle_timeout() > { > tst_res TBROK ... > kill .... > } > ... > sleep $sec && _tst_handle_timeout & +1. Thanks! Kind regards, Petr > Jörg
Hi Petr, >>> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh >>> index 69f007d89..35ad6d2d7 100644 >>> --- a/testcases/lib/tst_test.sh >>> +++ b/testcases/lib/tst_test.sh >>> @@ -21,7 +21,7 @@ export TST_LIB_LOADED=1 >>> . tst_security.sh >>> # default trap function >>> -trap "tst_brk TBROK 'test interrupted'" INT >>> +trap "tst_brk TBROK 'test interrupted or timeout'" INT >> should be "timed out" for consistency > You probably mean consistency with TST_RETRY_FN_EXP_BACKOFF() > I tried to be consistent with previous message: > zram01 1 TBROK: test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1 > That one should be changed as well. > I slightly prefer "timeout", but I'm not a native speaker. No I was meant the "test interrupted" part. "test interrupt or timeout" would be consistent. I am not a native speaker either, but "test interrupted or timed out" sound more correct to me. Jörg
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh index 69f007d89..35ad6d2d7 100644 --- a/testcases/lib/tst_test.sh +++ b/testcases/lib/tst_test.sh @@ -21,7 +21,7 @@ export TST_LIB_LOADED=1 . tst_security.sh # default trap function -trap "tst_brk TBROK 'test interrupted'" INT +trap "tst_brk TBROK 'test interrupted or timeout'" INT _tst_do_exit() { @@ -459,7 +459,7 @@ _tst_setup_timer() tst_res TINFO "timeout per run is ${h}h ${m}m ${s}s" - sleep $sec && tst_res TBROK "test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && kill -9 -$pid & + sleep $sec && tst_res TBROK "test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1" && { kill -2 -$pid; sleep 5; kill -9 -$pid; } & _tst_setup_timer_pid=$! }
Also timeout requires to run a test cleanup (e.g. zram01.sh). Thus send first SIGINT, but keep also SIGKILL for safety reasons (after 5 sec to give cleanup function some time. Signed-off-by: Petr Vorel <pvorel@suse.cz> --- Example of output on timeout: zram01 6 TINFO: filling zram0 (it can take long time) zram01 1 TBROK: test killed, timeout! If you are running on slow machine, try exporting LTP_TIMEOUT_MUL > 1 zram01 6 TBROK: test interrupted or timeout zram01 6 TINFO: AppArmor enabled, this may affect test results ... Kind regards, Petr testcases/lib/tst_test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)