[PATCH nft 00/11] tests/shell: colorize output, fix VALGRIND mode

[PATCH nft 00/11] tests/shell: colorize output, fix VALGRIND mode

[Thomas Haller]

- colorize the test output.
- fix the valgrind mode (-V, VALGRIND=y).
- various minor improvements(?).

Thomas Haller (11):
  tests/shell: cleanup result handling in "test-wrapper.sh"
  tests/shell: cleanup print_test_result() and show TAINTED error code
  tests/shell: colorize terminal output with test result
  tests/shell: fix handling failures with VALGRIND=y
  tests/shell: print the NFT setting with the VALGRIND=y wrapper
  tests/shell: don't redirect error/warning messages to stderr
  tests/shell: redirect output of test script to file too
  tests/shell: print "kernel is tainted" separate from test result
  tests/shell: no longer enable verbose output when selecting a test
  tests/shell: record wall time of test run in result data
  tests/shell: set NFT_TEST_JOBS based on $(nproc)

 tests/shell/helpers/nft-valgrind-wrapper.sh |  15 ++-
 tests/shell/helpers/test-wrapper.sh         |  70 ++++++----
 tests/shell/run-tests.sh                    | 134 +++++++++++++++-----
 3 files changed, 165 insertions(+), 54 deletions(-)

-- 
2.41.0

[PATCH nft 01/11] tests/shell: cleanup result handling in "test-wrapper.sh"

[Thomas Haller]

The previous code was mostly correct, but hard to understand.
Rework it.

Also, on failure now always write "rc-failed-exit", which is the exit
code that "test-wrapper.sh" reports to "run-test.sh". Note that this
error code may not be the same as the one returned by the TEST binary.
The latter you can find in one of the files "rc-{ok,skipped,failed}".

In general, you can search the directory with test results for those
"rc-*" files. If you find a "rc-failed" file, it was counted as failure.
There might be other "rc-failed-*" files, depending on whether the diff
failed or kernel got tainted.

Also, reserve all the error codes 118 - 124 for the "test-wrapper.sh".
For example, 124 means a dump difference and 123 means kernel got
tainted. In the future, 122 will mean a valgrind error. Other numbers
are not reserved. If a test command fails with such an reserved code,
"test-wrapper.sh" modifies it to 125, so that "run-test.sh" does not get
the wrong idea about the failure reason.  This is not new in this patch,
except that the reserved range was extended for future additions.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/helpers/test-wrapper.sh | 65 ++++++++++++++++++-----------
 1 file changed, 41 insertions(+), 24 deletions(-)

diff --git a/tests/shell/helpers/test-wrapper.sh b/tests/shell/helpers/test-wrapper.sh
index 43b3aa09ef26..f8b27b1e9291 100755
--- a/tests/shell/helpers/test-wrapper.sh
+++ b/tests/shell/helpers/test-wrapper.sh
@@ -51,7 +51,6 @@ DUMPPATH="$TESTDIR/dumps"
 DUMPFILE="$DUMPPATH/$TESTBASE.nft"
 
 dump_written=
-rc_dump=
 
 # The caller can request a re-geneating of the dumps, by setting
 # DUMPGEN=y.
@@ -66,42 +65,60 @@ if [ "$rc_test" -eq 0 -a "$DUMPGEN" = y -a -d "$DUMPPATH" ] ; then
 	cat "$NFT_TEST_TESTTMPDIR/ruleset-after" > "$DUMPFILE"
 fi
 
+rc_dump=0
 if [ "$rc_test" -ne 77 -a -f "$DUMPFILE" ] ; then
-	rc_dump=0
 	if [ "$dump_written" != y ] ; then
-		$DIFF -u "$DUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff" || rc_dump=$?
-		if [ "$rc_dump" -eq 0 ] ; then
+		if ! $DIFF -u "$DUMPFILE" "$NFT_TEST_TESTTMPDIR/ruleset-after" &> "$NFT_TEST_TESTTMPDIR/ruleset-diff" ; then
+			rc_dump=124
 			rm -f "$NFT_TEST_TESTTMPDIR/ruleset-diff"
 		fi
 	fi
 fi
+if [ "$rc_dump" -ne 0 ] ; then
+	echo "$DUMPFILE" > "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
+fi
 
+rc_tainted=0
 if [ "$tainted_before" != "$tainted_after" ] ; then
 	echo "$tainted_after" > "$NFT_TEST_TESTTMPDIR/rc-failed-tainted"
+	rc_tainted=123
 fi
 
-rc_exit="$rc_test"
-if [ -n "$rc_dump" ] && [ "$rc_dump" -ne 0 ] ; then
-	echo "$DUMPFILE" > "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
-	echo "$rc_test" > "$NFT_TEST_TESTTMPDIR/rc-failed"
-	if [ "$rc_exit" -eq 0 ] ; then
-		# Special exit code to indicate dump diff.
-		rc_exit=124
-	fi
-elif [ "$rc_test" -eq 77 ] ; then
-	echo "$rc_test" > "$NFT_TEST_TESTTMPDIR/rc-skipped"
-elif [ "$rc_test" -eq 0 -a "$tainted_before" = "$tainted_after" ] ; then
-	echo "$rc_test" > "$NFT_TEST_TESTTMPDIR/rc-ok"
+if [ "$rc_tainted" -ne 0 ] ; then
+	rc_exit="$rc_tainted"
+elif [ "$rc_test" -ge 118 -a "$rc_test" -le 124 ] ; then
+	# Special exit codes are reserved. Coerce them.
+	rc_exit="125"
+elif [ "$rc_test" -ne 0 ] ; then
+	rc_exit="$rc_test"
+elif [ "$rc_dump" -ne 0 ] ; then
+	rc_exit="$rc_dump"
 else
-	echo "$rc_test" > "$NFT_TEST_TESTTMPDIR/rc-failed"
-	if [ "$rc_test" -eq 0 -a "$tainted_before" != "$tainted_after" ] ; then
-		# Special exit code to indicate tainted.
-		rc_exit=123
-	elif [ "$rc_test" -eq 124 -o "$rc_test" -eq 123 ] ; then
-		# These exit codes are reserved
-		rc_exit=125
-	fi
+	rc_exit="0"
+fi
+
+
+# We always write the real exit code of the test ($rc_test) to one of the files
+# rc-{ok,skipped,failed}, depending on which it is.
+#
+# Note that there might be other rc-failed-{dump,tainted} files with additional
+# errors. Note that if such files exist, the overall state will always be
+# failed too (and an "rc-failed" file exists).
+#
+# On failure, we also write the combined "$rc_exit" code from "test-wrapper.sh"
+# to "rc-failed-exit" file.
+#
+# This means, failed tests will have a "rc-failed" file, and additional
+# "rc-failed-*" files exist for further information.
+if [ "$rc_exit" -eq 0 ] ; then
+	RC_FILENAME="rc-ok"
+elif [ "$rc_exit" -eq 77 ] ; then
+	RC_FILENAME="rc-skipped"
+else
+	RC_FILENAME="rc-failed"
+	echo "$rc_exit" > "$NFT_TEST_TESTTMPDIR/rc-failed-exit"
 fi
+echo "$rc_test" > "$NFT_TEST_TESTTMPDIR/$RC_FILENAME"
 
 END_TIME="$(cut -d ' ' -f1 /proc/uptime)"
 WALL_TIME="$(awk -v start="$START_TIME" -v end="$END_TIME" "BEGIN { print(end - start) }")"
-- 
2.41.0

[PATCH nft 02/11] tests/shell: cleanup print_test_result() and show TAINTED error code

[Thomas Haller]

We will add more special error codes (122 for VALGRIND). Minor refactor
of print_test_result() to make it easier to extend for that.

Also, we will soon colorize the output. This preparation patch makes
that easier too.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 423c5465c4d4..e0adb27ad104 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -471,25 +471,27 @@ print_test_result() {
 	shift 3
 
 	local result_msg_level="I"
-	local result_msg_status="OK"
 	local result_msg_suffix=""
 	local result_msg_files=( "$NFT_TEST_TESTTMPDIR/testout.log" "$NFT_TEST_TESTTMPDIR/ruleset-diff" )
+	local result_msg_status
 
 	if [ "$rc_got" -eq 0 ] ; then
 		((ok++))
-	elif [ "$rc_got" -eq 124 ] ; then
-		((failed++))
-		result_msg_level="W"
-		result_msg_status="DUMP FAIL"
+		result_msg_status="OK"
 	elif [ "$rc_got" -eq 77 ] ; then
 		((skipped++))
-		result_msg_level="I"
 		result_msg_status="SKIPPED"
 	else
 		((failed++))
 		result_msg_level="W"
-		result_msg_status="FAILED"
-		result_msg_suffix="got $rc_got"
+		if [ "$rc_got" -eq 123 ] ; then
+			result_msg_status="TAINTED"
+		elif [ "$rc_got" -eq 124 ] ; then
+			result_msg_status="DUMP FAIL"
+		else
+			result_msg_status="FAILED"
+			result_msg_suffix="got $rc_got"
+		fi
 		result_msg_files=( "$NFT_TEST_TESTTMPDIR/testout.log" )
 	fi
 
-- 
2.41.0

[PATCH nft 03/11] tests/shell: colorize terminal output with test result

[Thomas Haller]

Colors help to see what is important.

It honors the common NO_COLOR=<anything> to disable coloring. It also
does not colorize, if [ -t 1 ] indicates that stdout is not a terminal.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 71 ++++++++++++++++++++++++++++++++++------
 1 file changed, 61 insertions(+), 10 deletions(-)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index e0adb27ad104..c8688587bbc4 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -1,15 +1,26 @@
 #!/bin/bash
 
+GREEN=""
+YELLOW=""
+RED=""
+RESET=""
+if [[ -t 1 && -z "$NO_COLOR" ]] ; then
+	GREEN=$'\e[32m'
+	YELLOW=$'\e[33m'
+	RED=$'\e[31m'
+	RESET=$'\e[0m'
+fi
+
 _msg() {
 	local level="$1"
 	shift
-	local msg
 
-	msg="$level: $*"
-	if [ "$level" = E -o "$level" = W ] ; then
-		printf '%s\n' "$msg" >&2
+	if [ "$level" = E ] ; then
+		printf '%s\n' "$RED$level$RESET: $*" >&2
+	elif [ "$level" = W ] ; then
+		printf '%s\n' "$YELLOW$level$RESET: $*" >&2
 	else
-		printf '%s\n' "$msg"
+		printf '%s\n' "$level: $*"
 	fi
 	if [ "$level" = E ] ; then
 		exit 1
@@ -28,6 +39,39 @@ msg_info() {
 	_msg I "$@"
 }
 
+align_text() {
+	local _OUT_VARNAME="$1"
+	local _LEFT_OR_RIGHT="$2"
+	local _INDENT="$3"
+	shift 3
+	local _text="$*"
+	local _text_plain
+	local _text_align
+	local _text_result
+	local _i
+
+	# This function is needed, because "$text" might contain color escape
+	# sequences. A plain `printf '%12s' "$text"` will not align properly.
+
+	# strip escape sequences
+	_text_plain="${_text//$'\e['[0-9]m/}"
+	_text_plain="${_text_plain//$'\e['[0-9][0-9]m/}"
+
+	_text_align=""
+	for (( _i = "${#_text_plain}" ; "$_i" < "$_INDENT" ; _i++ )) ; do
+		_text_align="$_text_align "
+	done
+
+	if [ "$_LEFT_OR_RIGHT" = left ] ; then
+		_text_result="$(printf "%s$_text_align-" "$_text")"
+	else
+		_text_result="$(printf "$_text_align%s-" "$_text")"
+	fi
+	_text_result="${_text_result%-}"
+
+	eval "$_OUT_VARNAME=\"\$_text_result\""
+}
+
 bool_n() {
 	case "$1" in
 		n|N|no|No|NO|0|false|False|FALSE)
@@ -459,8 +503,7 @@ print_test_header() {
 	local suffix="$4"
 	local text
 
-	text="[$status]"
-	text="$(printf '%-12s' "$text")"
+	align_text text left 12 "[$status]"
 	_msg "$msglevel" "$text $testfile${suffix:+: $suffix}"
 }
 
@@ -477,10 +520,10 @@ print_test_result() {
 
 	if [ "$rc_got" -eq 0 ] ; then
 		((ok++))
-		result_msg_status="OK"
+		result_msg_status="${GREEN}OK$RESET"
 	elif [ "$rc_got" -eq 77 ] ; then
 		((skipped++))
-		result_msg_status="SKIPPED"
+		result_msg_status="${YELLOW}SKIPPED$RESET"
 	else
 		((failed++))
 		result_msg_level="W"
@@ -492,6 +535,7 @@ print_test_result() {
 			result_msg_status="FAILED"
 			result_msg_suffix="got $rc_got"
 		fi
+		result_msg_status="$RED$result_msg_status$RESET"
 		result_msg_files=( "$NFT_TEST_TESTTMPDIR/testout.log" )
 	fi
 
@@ -578,7 +622,14 @@ echo ""
 kmemleak_found=0
 check_kmemleak_force
 
-msg_info "results: [OK] $ok [SKIPPED] $skipped [FAILED] $failed [TOTAL] $((ok+skipped+failed))"
+if [ "$failed" -gt 0 ] ; then
+	RR="$RED"
+elif [ "$skipped" -gt 0 ] ; then
+	RR="$YELLOW"
+else
+	RR="$GREEN"
+fi
+msg_info "${RR}results$RESET: [OK] $GREEN$ok$RESET [SKIPPED] $YELLOW$skipped$RESET [FAILED] $RED$failed$RESET [TOTAL] $((ok+skipped+failed))"
 
 kernel_cleanup
 
-- 
2.41.0

[PATCH nft 04/11] tests/shell: fix handling failures with VALGRIND=y

[Thomas Haller]

With VALGRIND=y, on memleaks the tests did not fail. Fix that by passing
"--error-exitcode=122" to valgrind.

But just returning 122 from $NFT command may not correctly fail the test.
Instead, ensure to write a "rc-failed-valrind" file, which is picked up
by "test-wrapper.sh" to properly handle the valgrind failure (and fail
with error code 122 itself).

Also, accept NFT_TEST_VALGRIND_OPTS variable to a pass additional
arguments to valgrind. For example a "--suppressions" file.

Also show the special error code [VALGRIND] in "run-test.sh".

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/helpers/nft-valgrind-wrapper.sh | 15 ++++++++++++++-
 tests/shell/helpers/test-wrapper.sh         | 13 +++++++++----
 tests/shell/run-tests.sh                    |  4 +++-
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/tests/shell/helpers/nft-valgrind-wrapper.sh b/tests/shell/helpers/nft-valgrind-wrapper.sh
index 9da50d4d9d1d..ad8cc74bc781 100755
--- a/tests/shell/helpers/nft-valgrind-wrapper.sh
+++ b/tests/shell/helpers/nft-valgrind-wrapper.sh
@@ -1,6 +1,6 @@
 #!/bin/bash -e
 
-SUFFIX="$(date '+%Y%m%d-%H%M%S.%6N')"
+SUFFIX="$(date "+%Y%m%d-%H%M%S.%6N.$$")"
 
 rc=0
 libtool \
@@ -10,8 +10,21 @@ libtool \
 		--trace-children=yes \
 		--leak-check=full \
 		--show-leak-kinds=all \
+		--num-callers=100 \
+		--error-exitcode=122 \
+		$NFT_TEST_VALGRIND_OPTS \
 		"$NFT_REAL" \
 		"$@" \
 	|| rc=$?
 
+if [ "$rc" -eq 122 ] ; then
+	shopt -s nullglob
+	FILES=( "$NFT_TEST_TESTTMPDIR/valgrind.$SUFFIX."*".log" )
+	shopt -u nullglob
+	(
+		printf '%s\n' "args: $*"
+		printf '%s\n' "${FILES[*]}"
+	) >> "$NFT_TEST_TESTTMPDIR/rc-failed-valgrind"
+fi
+
 exit $rc
diff --git a/tests/shell/helpers/test-wrapper.sh b/tests/shell/helpers/test-wrapper.sh
index f8b27b1e9291..405e70c86751 100755
--- a/tests/shell/helpers/test-wrapper.sh
+++ b/tests/shell/helpers/test-wrapper.sh
@@ -78,13 +78,18 @@ if [ "$rc_dump" -ne 0 ] ; then
 	echo "$DUMPFILE" > "$NFT_TEST_TESTTMPDIR/rc-failed-dump"
 fi
 
+rc_valgrind=0
+[ -f "$NFT_TEST_TESTTMPDIR/rc-failed-valgrind" ] && rc_valgrind=122
+
 rc_tainted=0
 if [ "$tainted_before" != "$tainted_after" ] ; then
 	echo "$tainted_after" > "$NFT_TEST_TESTTMPDIR/rc-failed-tainted"
 	rc_tainted=123
 fi
 
-if [ "$rc_tainted" -ne 0 ] ; then
+if [ "$rc_valgrind" -ne 0 ] ; then
+	rc_exit="$rc_valgrind"
+elif [ "$rc_tainted" -ne 0 ] ; then
 	rc_exit="$rc_tainted"
 elif [ "$rc_test" -ge 118 -a "$rc_test" -le 124 ] ; then
 	# Special exit codes are reserved. Coerce them.
@@ -101,9 +106,9 @@ fi
 # We always write the real exit code of the test ($rc_test) to one of the files
 # rc-{ok,skipped,failed}, depending on which it is.
 #
-# Note that there might be other rc-failed-{dump,tainted} files with additional
-# errors. Note that if such files exist, the overall state will always be
-# failed too (and an "rc-failed" file exists).
+# Note that there might be other rc-failed-{dump,tainted,valgrind} files with
+# additional errors. Note that if such files exist, the overall state will
+# always be failed too (and an "rc-failed" file exists).
 #
 # On failure, we also write the combined "$rc_exit" code from "test-wrapper.sh"
 # to "rc-failed-exit" file.
diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index c8688587bbc4..ab91fd4d9053 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -527,7 +527,9 @@ print_test_result() {
 	else
 		((failed++))
 		result_msg_level="W"
-		if [ "$rc_got" -eq 123 ] ; then
+		if [ "$rc_got" -eq 122 ] ; then
+			result_msg_status="VALGRIND"
+		elif [ "$rc_got" -eq 123 ] ; then
 			result_msg_status="TAINTED"
 		elif [ "$rc_got" -eq 124 ] ; then
 			result_msg_status="DUMP FAIL"
-- 
2.41.0

[PATCH nft 05/11] tests/shell: print the NFT setting with the VALGRIND=y wrapper

[Thomas Haller]

With this we see in the info output

  I: info: NFT=./tests/shell/helpers/nft-valgrind-wrapper.sh

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index ab91fd4d9053..4f0df3217b76 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -411,6 +411,11 @@ echo
 msg_info "info: NFT_TEST_BASEDIR=$(printf '%q' "$NFT_TEST_BASEDIR")"
 msg_info "info: NFT_TEST_TMPDIR=$(printf '%q' "$NFT_TEST_TMPDIR")"
 
+if [ "$VALGRIND" == "y" ]; then
+	NFT="$NFT_TEST_BASEDIR/helpers/nft-valgrind-wrapper.sh"
+	msg_info "info: NFT=$(printf '%q' "$NFT")"
+fi
+
 kernel_cleanup() {
 	if [ "$NFT_TEST_JOBS" -ne 0 ] ; then
 		# When we run jobs in parallel (even with only one "parallel"
@@ -442,10 +447,6 @@ kernel_cleanup() {
 	nft_xfrm
 }
 
-if [ "$VALGRIND" == "y" ]; then
-	NFT="$NFT_TEST_BASEDIR/helpers/nft-valgrind-wrapper.sh"
-fi
-
 echo ""
 ok=0
 skipped=0
-- 
2.41.0

[PATCH nft 06/11] tests/shell: don't redirect error/warning messages to stderr

[Thomas Haller]

Writing some messages to stderr and some to stdout is not helpful.
Once they are written to separate streams, it's hard to be sure about
their relative order.

Use grep to filter messages.

Also, next we will redirect the entire output also to a file. There the
output is also not split in two files.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 4f0df3217b76..e4efbb2de540 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -16,9 +16,9 @@ _msg() {
 	shift
 
 	if [ "$level" = E ] ; then
-		printf '%s\n' "$RED$level$RESET: $*" >&2
+		printf '%s\n' "$RED$level$RESET: $*"
 	elif [ "$level" = W ] ; then
-		printf '%s\n' "$YELLOW$level$RESET: $*" >&2
+		printf '%s\n' "$YELLOW$level$RESET: $*"
 	else
 		printf '%s\n' "$level: $*"
 	fi
-- 
2.41.0

[PATCH nft 07/11] tests/shell: redirect output of test script to file too

[Thomas Haller]

It's useful to keep around for later. Redirect to the temporary
directory.

Note that the file content may be colorized too. `less -R` helps with
that.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index e4efbb2de540..4c1ab29b8536 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -382,6 +382,8 @@ NFT_TEST_TMPDIR="$(mktemp --tmpdir="$_TMPDIR" -d "nft-test.$(date '+%Y%m%d-%H%M%
 	msg_error "Failure to create temp directory in \"$_TMPDIR\""
 chmod 755 "$NFT_TEST_TMPDIR"
 
+exec &> >(tee "$NFT_TEST_TMPDIR/test.log")
+
 NFT_REAL="${NFT_REAL-$NFT}"
 
 msg_info "conf: NFT=$(printf '%q' "$NFT")"
-- 
2.41.0

[PATCH nft 08/11] tests/shell: print "kernel is tainted" separate from test result

[Thomas Haller]

Once the kernel is tainted, it stays until reboot. It would not be
useful to fail the entire test run based on that (and we don't do that).

But then, it seems odd to print this in the same style as the test
results, because a [FAILED] of a test counts as an overall failure.
Instead, print this warning in a different style.

Previously:

    $ ./tests/shell/run-tests.sh -- /usr/bin/true
    ...

    W: [FAILED]     kernel is tainted
    I: [OK]         /usr/bin/true

    I: results: [OK] 1 [SKIPPED] 0 [FAILED] 0 [TOTAL] 1

Now:

    $ ./tests/shell/run-tests.sh -- /usr/bin/true
    ...

    W: kernel is tainted

    I: [OK]         /usr/bin/true

    I: results: [OK] 1 [SKIPPED] 0 [FAILED] 0 [TOTAL] 1

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 4c1ab29b8536..6abb6c0c73a0 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -496,7 +496,8 @@ check_kmemleak()
 
 read kernel_tainted < /proc/sys/kernel/tainted
 if [ "$kernel_tainted" -ne 0 ] ; then
-	msg_warn "[FAILED]	kernel is tainted"
+	msg_warn "kernel is tainted"
+	echo
 fi
 
 print_test_header() {
-- 
2.41.0

[PATCH nft 09/11] tests/shell: no longer enable verbose output when selecting a test

[Thomas Haller]

Previously, when selecting a test on the command line, it would also
enable verbose output (except if the "--" separator was used).

This convenience feature seems not great because the output from the
test badly clutters the "run-test.sh" output.

Now that the test results are all on disk, you can search them after the
run with great flexibility (grep).

Additionally, in previous versions, command line argument parsing was
more restrictive, requiring that "-v" always be placed first. Now, the
order does not matter, so it's easy to edit the command prompt and
append a "-v", if that is what you want. Or if you really like verbose
output, then `export VERBOSE=y`.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 6abb6c0c73a0..bb73a771dfdc 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -100,7 +100,7 @@ usage() {
 	echo "OPTIONS:"
 	echo " -h|--help       : Print usage."
 	echo " -L|--list-tests : List test names and quit."
-	echo " -v              : Sets VERBOSE=y. Specifying tests without \"--\" enables verbose mode."
+	echo " -v              : Sets VERBOSE=y."
 	echo " -g              : Sets DUMPGEN=y."
 	echo " -V              : Sets VALGRIND=y."
 	echo " -K              : Sets KMEMLEAK=y."
@@ -218,10 +218,7 @@ while [ $# -gt 0 ] ; do
 			shift $#
 			;;
 		*)
-			# Any unrecognized option is treated as a test name, and also
-			# enable verbose tests.
 			TESTS+=( "$A" )
-			VERBOSE=y
 			;;
 	esac
 done
-- 
2.41.0

[PATCH nft 10/11] tests/shell: record wall time of test run in result data

[Thomas Haller]

When running tests, it's useful to see how long it took. Keep track if
the timestamps in a "times" file.

Try:

    ( \
        for d in /tmp/nft-test.latest.*/test-*/ ; do \
               printf '%10.2f  %s\n' \
                      "$(sed '1!d' "$d/times")" \
                      "$(cat "$d/name")" ; \
        done \
        | sort -n \
        | awk '{print $0; s+=$1} END{printf("%10.2f\n", s)}' ; \
        printf '%10.2f wall time\n' "$(sed '1!d' /tmp/nft-test.latest.*/times)" \
    )

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index bb73a771dfdc..35621bac569d 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -249,6 +249,8 @@ if [ "$DO_LIST_TESTS" = y ] ; then
 	exit 0
 fi
 
+START_TIME="$(cut -d ' ' -f1 /proc/uptime)"
+
 _TMPDIR="${TMPDIR:-/tmp}"
 
 if [ "$NFT_TEST_HAS_REALROOT" = "" ] ; then
@@ -636,6 +638,20 @@ msg_info "${RR}results$RESET: [OK] $GREEN$ok$RESET [SKIPPED] $YELLOW$skipped$RES
 
 kernel_cleanup
 
+#    ( \
+#        for d in /tmp/nft-test.latest.*/test-*/ ; do \
+#               printf '%10.2f  %s\n' \
+#                      "$(sed '1!d' "$d/times")" \
+#                      "$(cat "$d/name")" ; \
+#        done \
+#        | sort -n \
+#        | awk '{print $0; s+=$1} END{printf("%10.2f\n", s)}' ; \
+#        printf '%10.2f wall time\n' "$(sed '1!d' /tmp/nft-test.latest.*/times)" \
+#    )
+END_TIME="$(cut -d ' ' -f1 /proc/uptime)"
+WALL_TIME="$(awk -v start="$START_TIME" -v end="$END_TIME" "BEGIN { print(end - start) }")"
+printf "%s\n" "$WALL_TIME" "$START_TIME" "$END_TIME" > "$NFT_TEST_TMPDIR/times"
+
 if [ "$failed" -gt 0 -o "$NFT_TEST_KEEP_LOGS" = y ] ; then
 	msg_info "check the temp directory \"$NFT_TEST_TMPDIR\" (\"$NFT_TEST_LATEST\")"
 	msg_info "   ls -lad \"$NFT_TEST_LATEST\"/*/*"
-- 
2.41.0

[PATCH nft 11/11] tests/shell: set NFT_TEST_JOBS based on $(nproc)

[Thomas Haller]

Choose 150% of $(nproc) for the default vlaue of NFT_TEST_JOBS
(rounded up). The minimal value chosen by default is 2.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/run-tests.sh | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 35621bac569d..c622c1509500 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -154,7 +154,7 @@ usage() {
 	echo "                 mount namespace."
 	echo "                 This is only honored when \$NFT_TEST_UNSHARE_CMD= is set. Otherwise it's detected."
 	echo " NFT_TEST_KEEP_LOGS=*|y: Keep the temp directory. On success, it will be deleted by default."
-	echo " NFT_TEST_JOBS=<NUM}>: number of jobs for parallel execution. Defaults to \"12\" for parallel run."
+	echo " NFT_TEST_JOBS=<NUM}>: number of jobs for parallel execution. Defaults to \"\$(nproc)*1.5\" for parallel run."
 	echo "                 Setting this to \"0\" or \"1\", means to run jobs sequentially."
 	echo "                 Setting this to \"0\" means also to perform global cleanups between tests (remove"
 	echo "                 kernel modules)."
@@ -167,13 +167,17 @@ NFT_TEST_BASEDIR="$(dirname "$0")"
 # Export the base directory. It may be used by tests.
 export NFT_TEST_BASEDIR
 
+_NFT_TEST_JOBS_DEFAULT="$(nproc)"
+[ "$_NFT_TEST_JOBS_DEFAULT" -gt 0 ] 2>/dev/null || _NFT_TEST_JOBS_DEFAULT=1
+_NFT_TEST_JOBS_DEFAULT="$(( _NFT_TEST_JOBS_DEFAULT + (_NFT_TEST_JOBS_DEFAULT + 1) / 2 ))"
+
 VERBOSE="$(bool_y "$VERBOSE")"
 DUMPGEN="$(bool_y "$DUMPGEN")"
 VALGRIND="$(bool_y "$VALGRIND")"
 KMEMLEAK="$(bool_y "$KMEMLEAK")"
 NFT_TEST_KEEP_LOGS="$(bool_y "$NFT_TEST_KEEP_LOGS")"
 NFT_TEST_HAS_REALROOT="$NFT_TEST_HAS_REALROOT"
-NFT_TEST_JOBS="${NFT_TEST_JOBS:-12}"
+NFT_TEST_JOBS="${NFT_TEST_JOBS:-$_NFT_TEST_JOBS_DEFAULT}"
 DO_LIST_TESTS=
 
 TESTS=()
@@ -345,7 +349,7 @@ export NFT_TEST_HAS_UNSHARED_MOUNT
 
 # normalize the jobs number to be an integer.
 case "$NFT_TEST_JOBS" in
-	''|*[!0-9]*) NFT_TEST_JOBS=12 ;;
+	''|*[!0-9]*) NFT_TEST_JOBS=_NFT_TEST_JOBS_DEFAULT ;;
 esac
 if [ -z "$NFT_TEST_UNSHARE_CMD" -a "$NFT_TEST_JOBS" -gt 1 ] ; then
 	NFT_TEST_JOBS=1
-- 
2.41.0