diff mbox series

[v2,5/9] tst_test.sh: Add $TST_ALL_FILESYSTEMS

Message ID 20220609214223.4608-6-pvorel@suse.cz (mailing list archive)
State New, archived
Headers show
Series shell: nfs: $TST_ALL_FILESYSTEMS (.all_filesystems) | expand

Commit Message

Petr Vorel June 9, 2022, 9:42 p.m. UTC
Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
 doc/shell-test-api.txt    |   1 +
 testcases/lib/tst_test.sh | 106 ++++++++++++++++++++++++--------------
 2 files changed, 69 insertions(+), 38 deletions(-)

Comments

Cyril Hrubis June 17, 2022, 1:52 p.m. UTC | #1
Hi!
> +_tst_run_iterations()
> +{
> +	local _tst_i=$TST_ITERATIONS
>  
> -	if [ "$TST_MOUNT_DEVICE" = 1 ]; then
> -		tst_mount
> -		TST_MOUNT_FLAG=1
> -	fi
> +	[ "$TST_NEEDS_TMPDIR" = 1 ] && cd "$TST_TMPDIR"
>  
> -	[ -n "$TST_NEEDS_CHECKPOINTS" ] && _tst_init_checkpoints
> +	_tst_setup_timer
>  
>  	if [ -n "$TST_SETUP" ]; then
>  		if command -v $TST_SETUP >/dev/null 2>/dev/null; then
> @@ -724,7 +741,7 @@ tst_run()
>  	fi
>  
>  	#TODO check that test reports some results for each test function call
> -	while [ $TST_ITERATIONS -gt 0 ]; do
> +	while [ $_tst_i -gt 0 ]; do
>  		if [ -n "$TST_TEST_DATA" ]; then
>  			tst_require_cmds cut tr wc
>  			_tst_max=$(( $(echo $TST_TEST_DATA | tr -cd "$TST_TEST_DATA_IFS" | wc -c) +1))
> @@ -735,9 +752,22 @@ tst_run()
>  		else
>  			_tst_run_tests
>  		fi
> -		TST_ITERATIONS=$((TST_ITERATIONS-1))
> +		_tst_i=$((_tst_i-1))
>  	done
> -	_tst_do_exit
> +
> +	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
> +
> +	if [ "$TST_MOUNT_FLAG" = 1 ]; then
> +		tst_umount
> +	fi
> +
> +	_tst_cleanup_timer

Generally the code looks good, there is still a minor difference between
C API and this changes though. As we do call the _tst_cleanup_timer at
the end of this function the script runs without a timeout for a short
while (which includes tst_mkfs call).

I guess that instead of stopping the timer at the end of the
_tst_run_iterations() we can simply reset it (on the top of this patch):

diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index d96ce3448..f5af4c214 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -50,6 +50,8 @@ _tst_do_exit()
                rm $LTP_IPC_PATH
        fi

+       _tst_cleanup_timer
+
        if [ $TST_FAIL -gt 0 ]; then
                ret=$((ret|1))
        fi
@@ -552,6 +554,12 @@ _tst_setup_timer()
        done
 }

+_tst_reset_timer()
+{
+       _tst_cleanup_timer
+       _tst_setup_timer
+}
+
 tst_require_root()
 {
        if [ "$(id -ru)" != 0 ]; then
@@ -665,6 +673,8 @@ tst_run()
                tst_brk TBROK "Number of iterations (-i) must be >= 0"
        fi

+       _tst_setup_timer
+
        [ "$TST_NEEDS_ROOT" = 1 ] && tst_require_root

        [ "$TST_DISABLE_APPARMOR" = 1 ] && tst_disable_apparmor
@@ -729,8 +739,6 @@ _tst_run_iterations()

        [ "$TST_NEEDS_TMPDIR" = 1 ] && cd "$TST_TMPDIR"

-       _tst_setup_timer
-
        if [ -n "$TST_SETUP" ]; then
                if command -v $TST_SETUP >/dev/null 2>/dev/null; then
                        TST_DO_CLEANUP=1
@@ -767,7 +775,7 @@ _tst_run_iterations()
                tst_umount
        fi

-       _tst_cleanup_timer
+       _tst_reset_timer
 }
Petr Vorel June 17, 2022, 2:07 p.m. UTC | #2
Hi Cyril,

> Generally the code looks good, there is still a minor difference between
> C API and this changes though. As we do call the _tst_cleanup_timer at
> the end of this function the script runs without a timeout for a short
> while (which includes tst_mkfs call).

> I guess that instead of stopping the timer at the end of the
> _tst_run_iterations() we can simply reset it (on the top of this patch):

Good catch, thanks for providing a patch.
LGTM, I'll have a deeper look later.

Kind regards,
Petr
Petr Vorel Aug. 4, 2022, 7:07 a.m. UTC | #3
Hi all,

I wonder if we want to sort filesystems:

-for _tst_fs in $(tst_supported_fs); do
+for _tst_fs in $(tst_supported_fs | sort); do

or even -u (as uniq).

Martin used it in testcases/misc/lvm/prepare_lvm.sh, but IMHO it should not be
needed: looping over fs_type_whitelist() should be always the same.

FS_LIST=`tst_supported_fs | sort -u`

Therefore I'd remove it (don't use unnecessary dependencies - be nice for people
with minimal environment):

Kind regards,
Petr
diff mbox series

Patch

diff --git a/doc/shell-test-api.txt b/doc/shell-test-api.txt
index 65444541e..93073be13 100644
--- a/doc/shell-test-api.txt
+++ b/doc/shell-test-api.txt
@@ -199,6 +199,7 @@  simply by setting right '$TST_FOO'.
 [options="header"]
 |=============================================================================
 | Variable name            | Action done
+| 'TST_ALL_FILESYSTEMS'    | Testing on all available filesystems (tst_test.all_filesystems equivalent).
 | 'TST_DEV_EXTRA_OPTS'     | Pass extra 'mkfs' options _after_ device name,
                              to 'tst_mkfs', use with 'TST_FORMAT_DEVICE=1'.
 | 'TST_DEV_FS_OPTS'        | Pass 'mkfs' options _before_ the device name,
diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index f9ff9bcb4..d96ce3448 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -28,23 +28,12 @@  fi
 trap "tst_brk TBROK 'test interrupted'" INT
 trap "unset _tst_setup_timer_pid; tst_brk TBROK 'test terminated'" TERM
 
+# FIXME: debug called more times => check things moved out of it
 _tst_do_exit()
 {
 	local ret=0
 	TST_DO_EXIT=1
 
-	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
-
-	if [ "$TST_MOUNT_FLAG" = 1 ]; then
-		tst_umount
-	fi
-
 	if [ "$TST_NEEDS_DEVICE" = 1 -a "$TST_DEVICE_FLAG" = 1 ]; then
 		if ! tst_device release "$TST_DEVICE"; then
 			tst_res TWARN "Failed to release device '$TST_DEVICE'"
@@ -61,8 +50,6 @@  _tst_do_exit()
 		rm $LTP_IPC_PATH
 	fi
 
-	_tst_cleanup_timer
-
 	if [ $TST_FAIL -gt 0 ]; then
 		ret=$((ret|1))
 	fi
@@ -613,17 +600,41 @@  _tst_init_checkpoints()
 	export LTP_IPC_PATH
 }
 
+_prepare_device()
+{
+	if [ "$TST_FORMAT_DEVICE" = 1 ]; then
+		tst_clear_device $TST_DEVICE
+		tst_mkfs $TST_FS_TYPE $TST_DEV_FS_OPTS $TST_DEVICE $TST_DEV_EXTRA_OPTS
+	fi
+
+	if [ "$TST_MOUNT_DEVICE" = 1 ]; then
+		tst_mount
+		TST_MOUNT_FLAG=1
+	fi
+}
+
+_tst_run_tcases_per_fs()
+{
+	for _tst_fs in $(tst_supported_fs); do
+		tst_res TINFO "Testing on $_tst_fs"
+		TST_FS_TYPE="$_tst_fs"
+		_prepare_device
+		_tst_run_iterations
+	done
+}
+
 tst_run()
 {
 	local _tst_i
 	local _tst_data
+	local _tst_fs
 	local _tst_max
 	local _tst_name
 
 	if [ -n "$TST_TEST_PATH" ]; then
 		for _tst_i in $(grep '^[^#]*\bTST_' "$TST_TEST_PATH" | sed 's/.*TST_//; s/[='\''"} \t\/:`].*//'); do
 			case "$_tst_i" in
-			DISABLE_APPARMOR|DISABLE_SELINUX);;
+			ALL_FILESYSTEMS|DISABLE_APPARMOR|DISABLE_SELINUX);;
 			SETUP|CLEANUP|TESTFUNC|ID|CNT|MIN_KVER);;
 			OPTS|USAGE|PARSE_ARGS|POS_ARGS);;
 			NEEDS_ROOT|NEEDS_TMPDIR|TMPDIR|NEEDS_DEVICE|DEVICE);;
@@ -668,12 +679,23 @@  tst_run()
 			tst_brk TCONF "test requires kernel $TST_MIN_KVER+"
 	fi
 
-	_tst_setup_timer
+	[ -n "$TST_NEEDS_MODULE" ] && tst_require_module "$TST_NEEDS_MODULE"
 
+	[ "$TST_ALL_FILESYSTEMS" = 1 ] && TST_MOUNT_DEVICE=1
 	[ "$TST_MOUNT_DEVICE" = 1 ] && TST_FORMAT_DEVICE=1
 	[ "$TST_FORMAT_DEVICE" = 1 ] && TST_NEEDS_DEVICE=1
 	[ "$TST_NEEDS_DEVICE" = 1 ] && TST_NEEDS_TMPDIR=1
 
+	if [ "$TST_NEEDS_DEVICE" = 1 ]; then
+		TST_DEVICE=$(tst_device acquire)
+
+		if [ ! -b "$TST_DEVICE" -o $? -ne 0 ]; then
+			unset TST_DEVICE
+			tst_brk TBROK "Failed to acquire device"
+		fi
+		TST_DEVICE_FLAG=1
+	fi
+
 	if [ "$TST_NEEDS_TMPDIR" = 1 ]; then
 		if [ -z "$TMPDIR" ]; then
 			export TMPDIR="/tmp"
@@ -684,35 +706,30 @@  tst_run()
 		chmod 777 "$TST_TMPDIR"
 
 		TST_STARTWD=$(pwd)
-
 		cd "$TST_TMPDIR"
 	fi
 
-	TST_MNTPOINT="${TST_MNTPOINT:-$PWD/mntpoint}"
-	if [ "$TST_NEEDS_DEVICE" = 1 ]; then
-
-		TST_DEVICE=$(tst_device acquire)
+	[ -n "$TST_NEEDS_CHECKPOINTS" ] && _tst_init_checkpoints
 
-		if [ ! -b "$TST_DEVICE" -o $? -ne 0 ]; then
-			unset TST_DEVICE
-			tst_brk TBROK "Failed to acquire device"
-		fi
+	TST_MNTPOINT="${TST_MNTPOINT:-$PWD/mntpoint}"
+	[ -z "$TST_ALL_FILESYSTEMS" ] && _prepare_device
 
-		TST_DEVICE_FLAG=1
+	if [ -n "$TST_ALL_FILESYSTEMS" ]; then
+		_tst_run_tcases_per_fs
+	else
+		_tst_run_iterations
 	fi
 
-	[ -n "$TST_NEEDS_MODULE" ] && tst_require_module "$TST_NEEDS_MODULE"
+	_tst_do_exit
+}
 
-	if [ "$TST_FORMAT_DEVICE" = 1 ]; then
-		tst_mkfs $TST_FS_TYPE $TST_DEV_FS_OPTS $TST_DEVICE $TST_DEV_EXTRA_OPTS
-	fi
+_tst_run_iterations()
+{
+	local _tst_i=$TST_ITERATIONS
 
-	if [ "$TST_MOUNT_DEVICE" = 1 ]; then
-		tst_mount
-		TST_MOUNT_FLAG=1
-	fi
+	[ "$TST_NEEDS_TMPDIR" = 1 ] && cd "$TST_TMPDIR"
 
-	[ -n "$TST_NEEDS_CHECKPOINTS" ] && _tst_init_checkpoints
+	_tst_setup_timer
 
 	if [ -n "$TST_SETUP" ]; then
 		if command -v $TST_SETUP >/dev/null 2>/dev/null; then
@@ -724,7 +741,7 @@  tst_run()
 	fi
 
 	#TODO check that test reports some results for each test function call
-	while [ $TST_ITERATIONS -gt 0 ]; do
+	while [ $_tst_i -gt 0 ]; do
 		if [ -n "$TST_TEST_DATA" ]; then
 			tst_require_cmds cut tr wc
 			_tst_max=$(( $(echo $TST_TEST_DATA | tr -cd "$TST_TEST_DATA_IFS" | wc -c) +1))
@@ -735,9 +752,22 @@  tst_run()
 		else
 			_tst_run_tests
 		fi
-		TST_ITERATIONS=$((TST_ITERATIONS-1))
+		_tst_i=$((_tst_i-1))
 	done
-	_tst_do_exit
+
+	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
+
+	if [ "$TST_MOUNT_FLAG" = 1 ]; then
+		tst_umount
+	fi
+
+	_tst_cleanup_timer
 }
 
 _tst_run_tests()