[PATCH 1/2] runtests: Small code cleanup

[PATCH 1/2] runtests: Small code cleanup

[Lukas Czerner]

Use uppercase for global valiable consistently.
Use lowercase local variables consistently.
Don't use single letter variable names.
Add some comments.

No functional changes.

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 test/runtests.sh | 78 +++++++++++++++++++++++-------------------------
 1 file changed, 38 insertions(+), 40 deletions(-)

diff --git a/test/runtests.sh b/test/runtests.sh
index 2cf1eb2..1eb3fda 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,68 +56,66 @@ _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
 
-if [ "${RET}" -ne 0 ]; then
+if [ ${RET} -ne 0 ]; then
 	echo "Tests $FAILED failed"
 	exit $RET
 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 2/2] runtests: add ability to exclude tests

[Lukas Czerner]

Signed-off-by: Lukas Czerner <lczerner@redhat.com>
---
 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 1eb3fda..9701339 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 2/2] runtests: add ability to exclude tests

[Jens Axboe]

On 9/7/20 7:22 AM, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

Is there a cover letter and/or 1/2 patch that didn't go out?

-- 
Jens Axboe

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

[Lukas Czerner]

On Mon, Sep 07, 2020 at 10:21:56AM -0600, Jens Axboe wrote:
> On 9/7/20 7:22 AM, Lukas Czerner wrote:
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> 
> Is there a cover letter and/or 1/2 patch that didn't go out?
> 

There is 1/2 patch. Not sure why it didn't go through. I'll recheck and
try again.

-Lukas

> -- 
> Jens Axboe
> 

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

[Jens Axboe]

On 9/7/20 11:13 AM, Lukas Czerner wrote:
> On Mon, Sep 07, 2020 at 10:21:56AM -0600, Jens Axboe wrote:
>> On 9/7/20 7:22 AM, Lukas Czerner wrote:
>>> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
>>
>> Is there a cover letter and/or 1/2 patch that didn't go out?
>>
> 
> There is 1/2 patch. Not sure why it didn't go through. I'll recheck and
> try again.

It's here now, just took many hours to get there. Weird!

-- 
Jens Axboe

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

[Jens Axboe]

On 9/7/20 7:22 AM, Lukas Czerner wrote:
> Signed-off-by: Lukas Czerner <lczerner@redhat.com>

This patch really needs some justification. I generally try to make sure
that older kernel skip tests appropriately, sometimes I miss some and I
fix them up when I find them.

So just curious what the use case is here for skipping tests? Not
adverse to doing it, just want to make sure it's for the right reasons.

-- 
Jens Axboe

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

[Lukas Czerner]

On Mon, Sep 07, 2020 at 01:34:38PM -0600, Jens Axboe wrote:
> On 9/7/20 7:22 AM, Lukas Czerner wrote:
> > Signed-off-by: Lukas Czerner <lczerner@redhat.com>
> 
> This patch really needs some justification. I generally try to make sure
> that older kernel skip tests appropriately, sometimes I miss some and I
> fix them up when I find them.
> 
> So just curious what the use case is here for skipping tests? Not
> adverse to doing it, just want to make sure it's for the right reasons.

I think this is very useful, at least for me, in situation where some
tests are causeing problems (like hangs, panics) that are not fixed yet,
but I would still like to run entire tests suite to make sure my changes
in didn't break anything else. I find it especially usefull in rapidly
evolving project like this.

I have a V2 patches with that justification in place and some minor
changes. I'll be sending that (hopefuly it won't take hours this time).

Thanks!
-Lukas

> 
> -- 
> Jens Axboe
> 

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

[Jens Axboe]

On 9/8/20 2:14 AM, Lukas Czerner wrote:
> On Mon, Sep 07, 2020 at 01:34:38PM -0600, Jens Axboe wrote:
>> On 9/7/20 7:22 AM, Lukas Czerner wrote:
>>> Signed-off-by: Lukas Czerner <lczerner@redhat.com>
>>
>> This patch really needs some justification. I generally try to make sure
>> that older kernel skip tests appropriately, sometimes I miss some and I
>> fix them up when I find them.
>>
>> So just curious what the use case is here for skipping tests? Not
>> adverse to doing it, just want to make sure it's for the right reasons.
> 
> I think this is very useful, at least for me, in situation where some
> tests are causeing problems (like hangs, panics) that are not fixed yet,
> but I would still like to run entire tests suite to make sure my changes
> in didn't break anything else. I find it especially usefull in rapidly
> evolving project like this.

Yeah agree, that can be useful.

> I have a V2 patches with that justification in place and some minor
> changes. I'll be sending that (hopefuly it won't take hours this time).

OK thanks, I'll take a look and get it applied if it looks good.

-- 
Jens Axboe