| Message ID | 20241211001418.392890-1-pvorel@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] tst_test.sh: Fix TBROK => TWARN evaluation | expand |
Hi!
This I suppose got broken in:
commit 55bfa08e179de16773f19b703de70262896383ea
Author: Petr Vorel <pvorel@suse.cz>
Date: Thu Dec 14 15:00:10 2023 +0100
tst_test.sh/tst_brk(): Convert only TBROK to TWARN in cleanup
The original code only only depended on TST_DO_EXIT being set. I guess
that the easiest fix here would be actually to revert that patch. That
is because we mostly call the cleanup from _tst_do_exit() which sets the
TST_DO_EXIT before it calls _tst_do_cleanup(). The only place where we
call _tst_do_cleanup() wihout the TST_DO_EXIT is inside of the
_tst_run_iterations(), if we wanted to convert TBROK to TWARN in that
case we can simply do:
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index cfdae0230..ac1caebcb 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -820,6 +820,7 @@ _tst_run_iterations()
_tst_i=$((_tst_i-1))
done
+ TST_DO_EXIT=1
_tst_do_cleanup
if [ "$TST_MOUNT_FLAG" = 1 ]; then
And possibly change the TST_DO_EXIT to TST_TBROK_TO_TWARN as well.
Hi! > commit 55bfa08e179de16773f19b703de70262896383ea > Author: Petr Vorel <pvorel@suse.cz> > Date: Thu Dec 14 15:00:10 2023 +0100 > > tst_test.sh/tst_brk(): Convert only TBROK to TWARN in cleanup > > > The original code only only depended on TST_DO_EXIT being set. I guess > that the easiest fix here would be actually to revert that patch. That > is because we mostly call the cleanup from _tst_do_exit() which sets the > TST_DO_EXIT before it calls _tst_do_cleanup(). The only place where we > call _tst_do_cleanup() wihout the TST_DO_EXIT is inside of the > _tst_run_iterations(), if we wanted to convert TBROK to TWARN in that > case we can simply do: > > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh > index cfdae0230..ac1caebcb 100644 > --- a/testcases/lib/tst_test.sh > +++ b/testcases/lib/tst_test.sh > @@ -820,6 +820,7 @@ _tst_run_iterations() > _tst_i=$((_tst_i-1)) > done > > + TST_DO_EXIT=1 > _tst_do_cleanup And of course add: unset TST_DO_EXIT after the cleanup. > if [ "$TST_MOUNT_FLAG" = 1 ]; then > > > And possibly change the TST_DO_EXIT to TST_TBROK_TO_TWARN as well. > > -- > Cyril Hrubis > chrubis@suse.cz > > -- > Mailing list info: https://lists.linux.it/listinfo/ltp
Hi Cyril, > Hi! > This I suppose got broken in: > commit 55bfa08e179de16773f19b703de70262896383ea > Author: Petr Vorel <pvorel@suse.cz> > Date: Thu Dec 14 15:00:10 2023 +0100 FYI it was broken by 5c36ae3e30 ("tst_test.sh: Call cleanup function only after test start") (much earlier than 55bfa08e17). > tst_test.sh/tst_brk(): Convert only TBROK to TWARN in cleanup > The original code only only depended on TST_DO_EXIT being set. I guess > that the easiest fix here would be actually to revert that patch. That > is because we mostly call the cleanup from _tst_do_exit() which sets the > TST_DO_EXIT before it calls _tst_do_cleanup(). The only place where we > call _tst_do_cleanup() wihout the TST_DO_EXIT is inside of the > _tst_run_iterations(), if we wanted to convert TBROK to TWARN in that > case we can simply do: > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh > index cfdae0230..ac1caebcb 100644 > --- a/testcases/lib/tst_test.sh > +++ b/testcases/lib/tst_test.sh > @@ -820,6 +820,7 @@ _tst_run_iterations() > _tst_i=$((_tst_i-1)) > done > + TST_DO_EXIT=1 > _tst_do_cleanup OK, you would revert 55bfa08e179de16773f19b703de70262896383ea + use variable as guard here (TST_TBROK_TO_TWARN or whatever name it uses). I'll try to test it. BTW I remember in the past there were problems when setup got tst_brk TCONF, which calls the cleanup. Kind regards, Petr > if [ "$TST_MOUNT_FLAG" = 1 ]; then > And possibly change the TST_DO_EXIT to TST_TBROK_TO_TWARN as well.
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh index cfdae02300..eddb38a80b 100644 --- a/testcases/lib/tst_test.sh +++ b/testcases/lib/tst_test.sh @@ -1,6 +1,6 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0-or-later -# Copyright (c) Linux Test Project, 2014-2022 +# Copyright (c) Linux Test Project, 2014-2024 # Author: Cyril Hrubis <chrubis@suse.cz> # # LTP test library for shell. @@ -26,15 +26,20 @@ trap "unset _tst_setup_timer_pid; tst_brk TBROK 'test terminated'" TERM _tst_do_cleanup() { - if [ -n "$TST_DO_CLEANUP" -a -n "$TST_CLEANUP" -a -z "$LTP_NO_CLEANUP" ]; then - if command -v $TST_CLEANUP >/dev/null 2>/dev/null; then - TST_DO_CLEANUP= - $TST_CLEANUP - else - tst_res TWARN "TST_CLEANUP=$TST_CLEANUP declared, but function not defined (or cmd not found)" - fi + # run cleanup only once, when not requested by user to skip + if [ -n "$TST_DO_CLEANUP" ] || [ -z "$LTP_NO_CLEANUP" ]; then + return 0 + fi + + TST_DO_CLEANUP=1 + + if command -v $TST_CLEANUP >/dev/null 2>/dev/null; then + $TST_CLEANUP + elif [ -z "$TST_TEST_STARTED" ]; then + tst_res TWARN "Attempt to run cleanup function before test has started => '. tst_test.sh' should be at the end of the file" + else + tst_res TWARN "TST_CLEANUP=$TST_CLEANUP declared, but function not defined (or command not found)" fi - TST_DO_CLEANUP= } _tst_do_exit() @@ -128,9 +133,8 @@ tst_brk() shift # TBROK => TWARN on cleanup or exit - if [ "$res" = TBROK ] && [ "$TST_DO_EXIT" = 1 -o -z "$TST_DO_CLEANUP" -a -n "$TST_CLEANUP" ]; then + if [ "$res" = TBROK ] && [ "$TST_DO_EXIT" = 1 -o "$TST_DO_CLEANUP" = 1 ]; then tst_res TWARN "$@" - TST_DO_CLEANUP= return fi @@ -798,7 +802,7 @@ _tst_run_iterations() if [ -n "$TST_SETUP" ]; then if command -v $TST_SETUP >/dev/null 2>/dev/null; then - TST_DO_CLEANUP=1 + TST_TEST_STARTED=1 $TST_SETUP else tst_brk TBROK "TST_SETUP=$TST_SETUP declared, but function not defined (or cmd not found)" @@ -834,7 +838,7 @@ _tst_run_tests() local _tst_data="$1" local _tst_i - TST_DO_CLEANUP=1 + TST_TEST_STARTED=1 for _tst_i in $(seq ${TST_CNT:-1}); do if command -v ${TST_TESTFUNC}1 > /dev/null 2>&1; then _tst_run_test "$TST_TESTFUNC$_tst_i" $_tst_i "$_tst_data"
./nfs10.sh -v 3 -t udp nfs10 1 TINFO: Running: nfs10.devel.sh -v 3 -t udp nfs10 1 TINFO: Tested kernel: Linux ts 6.13.0-rc1-1.g492f944-default #1 SMP PREEMPT_DYNAMIC Mon Dec 2 08:55:00 UTC 2024 (492f944) x86_64 x86_64 x86_64 GNU/Linux nfs10 1 TINFO: initialize 'lhost' 'ltp_ns_veth2' interface nfs10 1 TINFO: add local addr 10.0.0.2/24 nfs10 1 TINFO: add local addr fd00:1:1:1::2/64 nfs10 1 TINFO: initialize 'rhost' 'ltp_ns_veth1' interface nfs10 1 TINFO: add remote addr 10.0.0.1/24 nfs10 1 TINFO: add remote addr fd00:1:1:1::1/64 nfs10 1 TINFO: Network config (local -- remote): nfs10 1 TINFO: ltp_ns_veth2 -- ltp_ns_veth1 nfs10 1 TINFO: 10.0.0.2/24 -- 10.0.0.1/24 nfs10 1 TINFO: fd00:1:1:1::2/64 -- fd00:1:1:1::1/64 nfs10 1 TINFO: Using /tmp/LTP_nfs10.fMhZnmFim0 as tmpdir (tmpfs filesystem) tst_device.c:299: TWARN: Failed to create test_dev.img: ENOSPC (28) Usage: tst_device acquire [size [filename]] tst_device release /path/to/device tst_device clear /path/to/device nfs10 1 TWARN: Failed to acquire device => This should be TBROK, but it wasn't due TST_CLEANUP being defined in tst_brk() (any test with TST_CLEANUP=1 did not exit when tst_brk TBROK was called from tst_test.sh due failure in early phase). Fixing it by splitting $TST_DO_CLEANUP variable into two functions: * $TST_DO_CLEANUP is a guarder for running cleanup function only once (similar to $TST_DO_EXIT). * Introduce new variable $TST_TEST_STARTED to indicate that test was started. Previously $TST_DO_CLEANUP was misused for this because the name suggests it (regression in 5c36ae3e30). Also print TWARN when cleanup function is not found due '. tst_test.sh' (or other shell library which loads it) is not at the end of the test (before tst_run). After 04021637f4 all tests load library late enough, but it's better to keep this check. Fixes: 5c36ae3e30 ("tst_test.sh: Call cleanup function only after test start") Signed-off-by: Petr Vorel <pvorel@suse.cz> --- testcases/lib/tst_test.sh | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-)