[PATCH v2 1/2] runtests: Clean up code in runtests.sh

[PATCH v2 1/2] runtests: Clean up code in runtests.sh

[Lukas Czerner]

Use uppercase for global and lowecase for local valiable consistently.
Don't use single letter variable names and add some comments.

No functional changes.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
v2: Update description and subject, don't remove quotes around RET variable

 test/runtests.sh | 76 +++++++++++++++++++++++-------------------------
 1 file changed, 37 insertions(+), 39 deletions(-)

diff --git a/test/runtests.sh b/test/runtests.sh
index 2cf1eb2..f860766 100755
--- a/test/runtests.sh
+++ b/test/runtests.sh
@@ -2,18 +2,18 @@
 
 TESTS="$@"
 RET=0
-
 TIMEOUT=60
+DMESG_FILTER="cat"
+TEST_DIR=$(dirname $0)
+TEST_FILES=""
 FAILED=""
 MAYBE_FAILED=""
 
-do_kmsg="1"
-if ! [ $(id -u) = 0 ]; then
-	do_kmsg="0"
-fi
+# Only use /dev/kmsg if running as root
+DO_KMSG="1"
+[ "$(id -u)" != "0" ] && DO_KMSG="0"
 
-TEST_DIR=$(dirname $0)
-TEST_FILES=""
+# Include config.local if exists and check TEST_FILES for valid devices
 if [ -f "$TEST_DIR/config.local" ]; then
 	. $TEST_DIR/config.local
 	for dev in $TEST_FILES; do
@@ -29,7 +29,7 @@ _check_dmesg()
 	local dmesg_marker="$1"
 	local seqres="$2.seqres"
 
-	if [[ $do_kmsg -eq 0 ]]; then
+	if [ $DO_KMSG -eq 0 ]; then
 		return 0
 	fi
 
@@ -56,56 +56,55 @@ _check_dmesg()
 
 run_test()
 {
-	T="$1"
-	D="$2"
-	DMESG_FILTER="cat"
+	local test_name="$1"
+	local dev="$2"
 
-	if [ "$do_kmsg" -eq 1 ]; then
-		if [ -z "$D" ]; then
-			local dmesg_marker="Running test $T:"
+	if [ "$DO_KMSG" -eq 1 ]; then
+		if [ -z "$dev" ]; then
+			local dmesg_marker="Running test $test_name:"
 		else
-			local dmesg_marker="Running test $T $D:"
+			local dmesg_marker="Running test $test_name $dev:"
 		fi
 		echo $dmesg_marker | tee /dev/kmsg
 	else
 		local dmesg_marker=""
-		echo Running test $T $D
+		echo Running test $test_name $dev
 	fi
-	timeout --preserve-status -s INT -k $TIMEOUT $TIMEOUT ./$T $D
-	r=$?
-	if [ "${r}" -eq 124 ]; then
-		echo "Test $T timed out (may not be a failure)"
-	elif [ "${r}" -ne 0 ]; then
-		echo "Test $T failed with ret ${r}"
-		if [ -z "$D" ]; then
-			FAILED="$FAILED <$T>"
+	timeout --preserve-status -s INT -k $TIMEOUT $TIMEOUT ./$test_name $dev
+	local status=$?
+	if [ "$status" -eq 124 ]; then
+		echo "Test $test_name timed out (may not be a failure)"
+	elif [ "$status" -ne 0 ]; then
+		echo "Test $test_name failed with ret $status"
+		if [ -z "$dev" ]; then
+			FAILED="$FAILED <$test_name>"
 		else
-			FAILED="$FAILED <$T $D>"
+			FAILED="$FAILED <$test_name $dev>"
 		fi
 		RET=1
-	elif ! _check_dmesg "$dmesg_marker" "$T"; then
-		echo "Test $T failed dmesg check"
-		if [ -z "$D" ]; then
-			FAILED="$FAILED <$T>"
+	elif ! _check_dmesg "$dmesg_marker" "$test_name"; then
+		echo "Test $test_name failed dmesg check"
+		if [ -z "$dev" ]; then
+			FAILED="$FAILED <$test_name>"
 		else
-			FAILED="$FAILED <$T $D>"
+			FAILED="$FAILED <$test_name $dev>"
 		fi
 		RET=1
-	elif [ ! -z "$D" ]; then
+	elif [ ! -z "$dev" ]; then
 		sleep .1
 		ps aux | grep "\[io_wq_manager\]" > /dev/null
-		R="$?"
-		if [ "$R" -eq 0 ]; then
-			MAYBE_FAILED="$MAYBE_FAILED $T"
+		if [ $? -eq 0 ]; then
+			MAYBE_FAILED="$MAYBE_FAILED $test_name"
 		fi
 	fi
 }
 
-for t in $TESTS; do
-	run_test $t
+# Run all specified tests
+for tst in $TESTS; do
+	run_test $tst
 	if [ ! -z "$TEST_FILES" ]; then
 		for dev in $TEST_FILES; do
-			run_test $t $dev
+			run_test $tst $dev
 		done
 	fi
 done
@@ -116,8 +115,7 @@ if [ "${RET}" -ne 0 ]; then
 else
 	sleep 1
 	ps aux | grep "\[io_wq_manager\]" > /dev/null
-	R="$?"
-	if [ "$R" -ne 0 ]; then
+	if [ $? -ne 0 ]; then
 		MAYBE_FAILED=""
 	fi
 	if [ ! -z "$MAYBE_FAILED" ]; then
-- 
2.26.2

[PATCH v2 2/2] runtests: add ability to exclude tests

[Lukas Czerner]

Add TEST_EXCLUDE configuration option to be able to skip specified
tests. This is usefull in situations when certain tests are causing
problems that are not yet fixed, but I'd still want to see if any of the
changes made didn't break anything else.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
v2: Add proper description

 test/config      |  7 +++++--
 test/runtests.sh | 48 ++++++++++++++++++++++++++++++------------------
 2 files changed, 35 insertions(+), 20 deletions(-)

diff --git a/test/config b/test/config
index 80a5f46..cab2703 100644
--- a/test/config
+++ b/test/config
@@ -1,4 +1,7 @@
-# Define raw test devices (or files) for test cases, if any
-# Copy this to config.local, and uncomment + define test files
+# Copy this to config.local, uncomment and define values
+#
+# Define tests to exclude from running
+# TEST_EXCLUDE=""
 #
+# Define raw test devices (or files) for test cases, if any
 # TEST_FILES="/dev/nvme0n1p2 /data/file"
diff --git a/test/runtests.sh b/test/runtests.sh
index f860766..fa240f2 100755
--- a/test/runtests.sh
+++ b/test/runtests.sh
@@ -7,6 +7,7 @@ DMESG_FILTER="cat"
 TEST_DIR=$(dirname $0)
 TEST_FILES=""
 FAILED=""
+SKIPPED=""
 MAYBE_FAILED=""
 
 # Only use /dev/kmsg if running as root
@@ -58,43 +59,50 @@ run_test()
 {
 	local test_name="$1"
 	local dev="$2"
+	local test_string=$test_name
 
+	# Specify test string to print
+	if [ -n "$dev" ]; then
+		test_string="$test_name $dev"
+	fi
+
+	# Log start of the test
 	if [ "$DO_KMSG" -eq 1 ]; then
-		if [ -z "$dev" ]; then
-			local dmesg_marker="Running test $test_name:"
-		else
-			local dmesg_marker="Running test $test_name $dev:"
-		fi
+		local dmesg_marker="Running test $test_string:"
 		echo $dmesg_marker | tee /dev/kmsg
 	else
 		local dmesg_marker=""
 		echo Running test $test_name $dev
 	fi
+
+	# Do we have to exclude the test ?
+	echo $TEST_EXCLUDE | grep -w "$test_name" > /dev/null 2>&1
+	if [ $? -eq 0 ]; then
+		echo "Test skipped"
+		SKIPPED="$SKIPPED <$test_string>"
+		return
+	fi
+
+	# Run the test
 	timeout --preserve-status -s INT -k $TIMEOUT $TIMEOUT ./$test_name $dev
 	local status=$?
+
+	# Check test status
 	if [ "$status" -eq 124 ]; then
 		echo "Test $test_name timed out (may not be a failure)"
 	elif [ "$status" -ne 0 ]; then
 		echo "Test $test_name failed with ret $status"
-		if [ -z "$dev" ]; then
-			FAILED="$FAILED <$test_name>"
-		else
-			FAILED="$FAILED <$test_name $dev>"
-		fi
+		FAILED="$FAILED <$test_string>"
 		RET=1
 	elif ! _check_dmesg "$dmesg_marker" "$test_name"; then
 		echo "Test $test_name failed dmesg check"
-		if [ -z "$dev" ]; then
-			FAILED="$FAILED <$test_name>"
-		else
-			FAILED="$FAILED <$test_name $dev>"
-		fi
+		FAILED="$FAILED <$test_string>"
 		RET=1
-	elif [ ! -z "$dev" ]; then
+	elif [ -n "$dev" ]; then
 		sleep .1
 		ps aux | grep "\[io_wq_manager\]" > /dev/null
 		if [ $? -eq 0 ]; then
-			MAYBE_FAILED="$MAYBE_FAILED $test_name"
+			MAYBE_FAILED="$MAYBE_FAILED $test_string"
 		fi
 	fi
 }
@@ -109,8 +117,12 @@ for tst in $TESTS; do
 	fi
 done
 
+if [ -n "$SKIPPED" ]; then
+	echo "Tests skipped: $SKIPPED"
+fi
+
 if [ "${RET}" -ne 0 ]; then
-	echo "Tests $FAILED failed"
+	echo "Tests failed: $FAILED"
 	exit $RET
 else
 	sleep 1
-- 
2.26.2

Re: [PATCH v2 1/2] runtests: Clean up code in runtests.sh

[Jens Axboe]

On 9/8/20 2:17 AM, Lukas Czerner wrote:
> Use uppercase for global and lowecase for local valiable consistently.
> Don't use single letter variable names and add some comments.
> 
> No functional changes.

Applied 1-2, thanks.

-- 
Jens Axboe