[PATCH nft 0/3] tests/shell: minor improvements to "run-tests.sh"

[PATCH nft 0/3] tests/shell: minor improvements to "run-tests.sh"

[Thomas Haller]

These patches out from another branch. Three minor improvements.

Thomas Haller (3):
  tests/shell: set C locale in "run-tests.sh"
  tests/shell: don't show the exit status for failed tests
  tests/shell: colorize NFT_TEST_HAS_SOCKET_LIMITS

 tests/shell/run-tests.sh | 46 +++++++++++++++++++++++-----------------
 1 file changed, 27 insertions(+), 19 deletions(-)

-- 
2.41.0

[PATCH nft 1/3] tests/shell: set C locale in "run-tests.sh"

[Thomas Haller]

The tests should run always the same, regardless of the user's language
settings. Set LANG=C and LC_ALL=C and unset LANGUAGE. If some part wants
to test a different language, it would set it explicitly. They anyway
wouldn't want to depend on something from the user's environment.

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

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 528646f57eca..79c818cb0f11 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -1,5 +1,9 @@
 #!/bin/bash
 
+unset LANGUAGE
+export LANG=C
+export LC_ALL=C
+
 GREEN=""
 YELLOW=""
 RED=""
@@ -235,7 +239,7 @@ for file in "${F[@]}"; do
 		msg_warn "Ignore feature file \"$file\""
 	fi
 done
-_HAVE_OPTS=( $(printf '%s\n' "${_HAVE_OPTS[@]}" | LANG=C sort) )
+_HAVE_OPTS=( $(printf '%s\n' "${_HAVE_OPTS[@]}" | sort) )
 
 for KEY in $(compgen -v | grep '^NFT_TEST_HAVE_' | sort) ; do
 	if ! array_contains "${KEY#NFT_TEST_HAVE_}" "${_HAVE_OPTS[@]}" ; then
@@ -336,7 +340,7 @@ while [ $# -gt 0 ] ; do
 done
 
 find_tests() {
-	find "$1" -type f -executable | LANG=C sort
+	find "$1" -type f -executable | sort
 }
 
 if [ "${#TESTS[@]}" -eq 0 ] ; then
-- 
2.41.0

[PATCH nft 2/3] tests/shell: don't show the exit status for failed tests

[Thomas Haller]

Previously, for failed tests we would print the exit code

  W: [FAILED]     2/2 tests/shell/testcases/listing/0013objects_0: got 1

This doesn't seem very useful. For one, we have special exit codes like
0 (OK), 77 (SKIPPED), 124 (DUMP FAIL), 123 (TAINTED), 122 (VALGRIND).
Any other exit code is just an arbitrary failure. We don't define any
special codes, and printing them is not useful.

Note that further exit codes (118 - 121) are reserved, and could be
special purposed, when there is a use.

You can find the real exit code from the test in the result data in the
"rc-failed" file.

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

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 79c818cb0f11..418fab95da94 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -704,7 +704,6 @@ print_test_header() {
 	local testfile="$2"
 	local testidx_completed="$3"
 	local status="$4"
-	local suffix="$5"
 	local text
 	local s_idx
 
@@ -713,7 +712,7 @@ print_test_header() {
 	s_idx="$text/${#TESTS[@]}"
 
 	align_text text left 12 "[$status]"
-	_msg "$msglevel" "$text $s_idx $testfile${suffix:+: $suffix}"
+	_msg "$msglevel" "$text $s_idx $testfile"
 }
 
 print_test_result() {
@@ -722,7 +721,6 @@ print_test_result() {
 	local rc_got="$3"
 
 	local result_msg_level="I"
-	local result_msg_suffix=""
 	local result_msg_files=( "$NFT_TEST_TESTTMPDIR/testout.log" "$NFT_TEST_TESTTMPDIR/ruleset-diff" )
 	local result_msg_status
 
@@ -743,13 +741,12 @@ print_test_result() {
 			result_msg_status="DUMP FAIL"
 		else
 			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
 
-	print_test_header "$result_msg_level" "$testfile" "$((ok + skipped + failed))" "$result_msg_status" "$result_msg_suffix"
+	print_test_header "$result_msg_level" "$testfile" "$((ok + skipped + failed))" "$result_msg_status"
 
 	if [ "$VERBOSE" = "y" ] ; then
 		local f
@@ -773,7 +770,7 @@ job_start() {
 	local testidx="$2"
 
 	if [ "$NFT_TEST_JOBS" -le 1 ] ; then
-		print_test_header I "$testfile" "$testidx" "EXECUTING" ""
+		print_test_header I "$testfile" "$testidx" "EXECUTING"
 	fi
 
 	NFT_TEST_TESTTMPDIR="${JOBS_TEMPDIR["$testfile"]}" \
-- 
2.41.0

[PATCH nft 3/3] tests/shell: colorize NFT_TEST_HAS_SOCKET_LIMITS

[Thomas Haller]

NFT_TEST_HAS_SOCKET_LIMITS= is similar to NFT_TEST_HAVE_* variables and
indicates a feature (or lack thereof), except that it's inverted.  Maybe
this should be consolidated, however, NFT_TEST_HAS_SOCKET_LIMITS= is
detected in the root namespace, unlike the shell scripts from features.
So it's unclear how to consolidate them best.

Anyway. Still highlight a lack of the capability, as it can cause tests
to be skipped and we should see that easily.

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

diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 418fab95da94..03021085e0e7 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -30,6 +30,18 @@ array_contains() {
 	return 1
 }
 
+colorize_keywords() {
+	local out_variable="$1"
+	local color="$2"
+	local val="$3"
+	local val2
+	shift 3
+
+	printf -v val2 '%q' "$val"
+	array_contains "$val" "$@" && val2="$color$val2$RESET"
+	printf -v "$out_variable" '%s' "$val2"
+}
+
 strtonum() {
 	local s="$1"
 	local n
@@ -571,7 +583,8 @@ msg_info "conf: DUMPGEN=$(printf '%q' "$DUMPGEN")"
 msg_info "conf: VALGRIND=$(printf '%q' "$VALGRIND")"
 msg_info "conf: KMEMLEAK=$(printf '%q' "$KMEMLEAK")"
 msg_info "conf: NFT_TEST_HAS_REALROOT=$(printf '%q' "$NFT_TEST_HAS_REALROOT")"
-msg_info "conf: NFT_TEST_HAS_SOCKET_LIMITS=$(printf '%q' "$NFT_TEST_HAS_SOCKET_LIMITS")"
+colorize_keywords value "$YELLOW" "$NFT_TEST_HAS_SOCKET_LIMITS" y
+msg_info "conf: NFT_TEST_HAS_SOCKET_LIMITS=$value"
 msg_info "conf: NFT_TEST_UNSHARE_CMD=$(printf '%q' "$NFT_TEST_UNSHARE_CMD")"
 msg_info "conf: NFT_TEST_HAS_UNSHARED=$(printf '%q' "$NFT_TEST_HAS_UNSHARED")"
 msg_info "conf: NFT_TEST_HAS_UNSHARED_MOUNT=$(printf '%q' "$NFT_TEST_HAS_UNSHARED_MOUNT")"
@@ -582,19 +595,13 @@ msg_info "conf: NFT_TEST_SHUFFLE_TESTS=$NFT_TEST_SHUFFLE_TESTS"
 msg_info "conf: TMPDIR=$(printf '%q' "$_TMPDIR")"
 echo
 for KEY in $(compgen -v | grep '^NFT_TEST_SKIP_' | sort) ; do
-	v="${!KEY}"
-	if [ "$v" = y ] ; then
-		v="$YELLOW$v$RESET"
-	fi
-	msg_info "conf: $KEY=$v"
+	colorize_keywords value "$YELLOW" "${!KEY}" y
+	msg_info "conf: $KEY=$value"
 	export "$KEY"
 done
 for KEY in $(compgen -v | grep '^NFT_TEST_HAVE_' | sort) ; do
-	v="${!KEY}"
-	if [ "$v" = n ] ; then
-		v="$YELLOW$v$RESET"
-	fi
-	msg_info "conf: $KEY=$v"
+	colorize_keywords value "$YELLOW" "${!KEY}" n
+	msg_info "conf: $KEY=$value"
 	export "$KEY"
 done
 
-- 
2.41.0

Re: [PATCH nft 0/3] tests/shell: minor improvements to "run-tests.sh"

[Pablo Neira Ayuso]

Hi Thomas,

Not precisely related to this series in this email thread, but...

I occasionally see errors like this when running tests in a loop:

./run-tests.sh: line 673: echo: write error: Interrupted system call

it is happening since the recent updates, do you have any idea what
could be wrong?

Thanks.

Re: [PATCH nft 0/3] tests/shell: minor improvements to "run-tests.sh"

[Thomas Haller]

On Wed, 2023-09-27 at 22:27 +0200, Pablo Neira Ayuso wrote:
> Hi Thomas,
> 
> Not precisely related to this series in this email thread, but...
> 
> I occasionally see errors like this when running tests in a loop:
> 
> ./run-tests.sh: line 673: echo: write error: Interrupted system call
> 
> it is happening since the recent updates, do you have any idea what
> could be wrong?
> 
> Thanks.
> 

Hi,


I think that line is

»·······echo scan > /sys/kernel/debug/kmemleak

Are you running with KMEMLEAK=y? But also without that, it gets called
after every 30th test returning -- since usually the fast tests run
quickly in parallel, we would call it more frequently at the beginning
of the tests.


Note that the "check_kmemleak_force" function in the run-tests.sh
script does NOT run in parallel. 


I am not familiar with kmemleak, but seems not serious. 
Maybe just ignore the error output?

  ( echo scan > /sys/kernel/debug/kmemleak ) &>/dev/null


Thomas