| Message ID | 20220930112434.13038-1-pvorel@suse.cz |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/1] tst_test.sh: Fix missing cleanup run from setup | expand |
Hi,
NOTE: we could add cleanup function call to at least one test
to have at least visual check that it's run and only once.
Kind regards,
Petr
diff --git lib/newlib_tests/shell/tst_mount_device.sh lib/newlib_tests/shell/tst_mount_device.sh
index 70f80f84a..4f387014f 100755
--- lib/newlib_tests/shell/tst_mount_device.sh
+++ lib/newlib_tests/shell/tst_mount_device.sh
@@ -6,8 +6,14 @@ TST_MOUNT_DEVICE=1
TST_NEEDS_ROOT=1
TST_FS_TYPE=ext4
TST_TESTFUNC=test
+TST_CLEANUP=do_cleanup
TST_CNT=3
+do_cleanup()
+{
+ tst_res TINFO "run cleanup"
+}
+
test1()
{
EXPECT_PASS "cd $TST_MNTPOINT"
Hi!
Looks like an obvious regression that should be fixed.
But we should at least re-run all shell tests to make sure that this
does not break anything. I would vote for this to go in before the
release but after you did enough testing.
And also it would be better if anyone else had a look.
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
Hi, > Hi! > Looks like an obvious regression that should be fixed. +1 > But we should at least re-run all shell tests to make sure that this > does not break anything. I would vote for this to go in before the > release but after you did enough testing. I'm testing with these 2 patches (this and zram01.sh) in my fork: https://github.com/pevik/ltp/commits/ltp-202209.2022-09-30.pre-release I rerun whole LTP tests on 3 SLES versions, most of the shell tests on Tumbleweed, I'll also try some shell tests on some Debian version. Also CI passed (which runs make test-shell), + I run make test-shell manually (covers more than CI), but that's not that relevant as we don't cover much. > And also it would be better if anyone else had a look. That'd be great, but not sure if anybody jumps in. Kind regards, Petr
Hi all, > > But we should at least re-run all shell tests to make sure that this > > does not break anything. I would vote for this to go in before the > > release but after you did enough testing. > I'm testing with these 2 patches (this and zram01.sh) in my fork: > https://github.com/pevik/ltp/commits/ltp-202209.2022-09-30.pre-release > I rerun whole LTP tests on 3 SLES versions, most of the shell tests on > Tumbleweed, I'll also try some shell tests on some Debian version. > Also CI passed (which runs make test-shell), + I run make test-shell manually > (covers more than CI), but that's not that relevant as we don't cover much. Tests results looks ok, looked into the output (not just the exit code), thus I merged this. This should be the last commit, we can tag the release and the rest of the release process. Kind regards, Petr > > And also it would be better if anyone else had a look. > That'd be great, but not sure if anybody jumps in. > Kind regards, > Petr
Hi! > > I rerun whole LTP tests on 3 SLES versions, most of the shell tests on > > Tumbleweed, I'll also try some shell tests on some Debian version. > > Also CI passed (which runs make test-shell), + I run make test-shell manually > > (covers more than CI), but that's not that relevant as we don't cover much. > > Tests results looks ok, looked into the output (not just the exit code), > thus I merged this. This should be the last commit, we can tag the release and > the rest of the release process. Sounds good to me. Feel free to tag the git, you signed for generating and uploading the tarballs anyway. I'm preparing release notes and send them once tarballs are uploaded on github.
> Hi! > > > I rerun whole LTP tests on 3 SLES versions, most of the shell tests on > > > Tumbleweed, I'll also try some shell tests on some Debian version. > > > Also CI passed (which runs make test-shell), + I run make test-shell manually > > > (covers more than CI), but that's not that relevant as we don't cover much. > > Tests results looks ok, looked into the output (not just the exit code), > > thus I merged this. This should be the last commit, we can tag the release and > > the rest of the release process. > Sounds good to me. Feel free to tag the git, you signed for generating > and uploading the tarballs anyway. I'm preparing release notes and send > them once tarballs are uploaded on github. Hi Cyril, Right, LTP version updated and tagged, both with my GPG sign. https://github.com/linux-test-project/ltp/commit/b763f81998f19f783982d3937d1fd05bcf649c16 I generated LTP 20220930 release: https://github.com/linux-test-project/ltp/releases/tag/20220930 Please send release notes, I'll add them to the releases page in github. Kind regards, Petr
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh index 7ec744cac..033491b08 100644 --- a/testcases/lib/tst_test.sh +++ b/testcases/lib/tst_test.sh @@ -24,11 +24,25 @@ export TST_LIB_LOADED=1 trap "tst_brk TBROK 'test interrupted'" INT 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 "$TST_NO_CLEANUP" ]; then + if command -v $TST_CLEANUP >/dev/null 2>/dev/null; then + $TST_CLEANUP + else + tst_res TWARN "TST_CLEANUP=$TST_CLEANUP declared, but function not defined (or cmd not found)" + fi + fi + TST_DO_CLEANUP= +} + _tst_do_exit() { local ret=0 TST_DO_EXIT=1 + _tst_do_cleanup + cd "$LTPROOT" [ "$TST_MOUNT_FLAG" = 1 ] && tst_umount @@ -785,13 +799,7 @@ _tst_run_iterations() _tst_i=$((_tst_i-1)) done - if [ -n "$TST_DO_CLEANUP" -a -n "$TST_CLEANUP" -a -z "$TST_NO_CLEANUP" ]; then - if command -v $TST_CLEANUP >/dev/null 2>/dev/null; then - $TST_CLEANUP - else - tst_res TWARN "TST_CLEANUP=$TST_CLEANUP declared, but function not defined (or cmd not found)" - fi - fi + _tst_do_cleanup if [ "$TST_MOUNT_FLAG" = 1 ]; then cd "$LTPROOT"
There was a regression on tests which don't use TST_ALL_FILESYSTEMS=1 when the cleanup function was not run when test called tst_brk in the setup function. This broke DCCP tests on kernels without dccp_ipv6 module: ./dccp_ipsec_vti.sh; ltp_file -p esp -m tunnel -s 100:500:1000:R1000 dccp_ipsec_vti 1 TINFO: Test vti + IPsec[esp/tunnel] ... netstress.c:970: TCONF: Failed to load dccp_ipv6 module Summary: passed 0 failed 0 broken 0 skipped 1 warnings 0 netstress.c:970: TCONF: Failed to load dccp_ipv6 module dccp_ipsec_vti 1 TCONF: server failed ./dccp4_ipsec02 dccp_ipsec.sh -p ah -m transport -s 100:500:1000:R1000 ... dccp_ipsec_vti 1 TINFO: Test vti + IPsec[esp/tunnel] dccp_ipsec_vti 1 TBROK: ip link add ltp_vti0 type vti local 10.0.0.2 remote 10.0.0.1 key 10 dev ltp_ns_veth2 failed: RTNETLINK answers: File exists The reason was that cleanup function call was moved from _tst_do_exit() to _tst_run_iterations() created for TST_ALL_FILESYSTEMS. But cleanup function still needs to be run in _tst_do_exit() (but only) if it weren't called before. Fixes: 1f6bd6e66 ("tst_test.sh: Add $TST_ALL_FILESYSTEMS") Reported-by: Martin Doucha <mdoucha@suse.cz> Signed-off-by: Petr Vorel <pvorel@suse.cz> --- Hi, It'd be nice to get this to the release. FYI tested on following tests (probably not worth to add to git) + other tests in the shell API. Kind regards, Petr === /tmp/xx/tst_all_filesystems_cleanup.sh === #!/bin/sh # SPDX-License-Identifier: GPL-2.0-or-later # Copyright (c) 2022 Petr Vorel <pvorel@suse.cz> TST_ALL_FILESYSTEMS=1 TST_MOUNT_DEVICE=1 TST_NEEDS_ROOT=1 TST_TESTFUNC=test TST_SETUP=do_setup TST_CLEANUP=do_cleanup TST_CNT=2 do_setup() { tst_brk TCONF "required TCONF" } do_cleanup() { tst_res TINFO "run cleanup" } test1() { tst_res TPASS "device using filesystem" } test2() { EXPECT_PASS "grep -E '$TST_MNTPOINT ($TST_FS_TYPE|fuseblk)' /proc/mounts" } . tst_test.sh tst_run === /tmp/xx/tst_net_cleanup.sh === #!/bin/sh # SPDX-License-Identifier: GPL-2.0-or-later # Copyright (c) 2022 Petr Vorel <pvorel@suse.cz> TST_CLEANUP=do_cleanup TST_TESTFUNC=test do_cleanup() { tst_res TINFO "run cleanup" } test() { tst_res TPASS "pass" } . tst_net.sh tst_run === /tmp/xx/tst_net_tconf_cleanup.sh === #!/bin/sh # SPDX-License-Identifier: GPL-2.0-or-later # Copyright (c) 2022 Petr Vorel <pvorel@suse.cz> TST_SETUP=do_setup TST_CLEANUP=do_cleanup TST_TESTFUNC=test do_setup() { tst_brk TCONF "required TCONF" } do_cleanup() { tst_res TINFO "run cleanup" } test() { tst_res TPASS "pass" } . tst_net.sh tst_run testcases/lib/tst_test.sh | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-)