netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH nft 1/2] tests/shell: use bash instead of /bin/sh for tests
@ 2023-10-16 13:30 Thomas Haller
  2023-10-16 13:30 ` [PATCH nft 2/2] tests/shell: honor NFT_TEST_VERBOSE_TEST variable to debug tests via `bash -x` Thomas Haller
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Haller @ 2023-10-16 13:30 UTC (permalink / raw)
  To: NetFilter; +Cc: Thomas Haller

All tests under "tests/shell" are shell scripts with shebang /bin/bash
or /bin/sh. This may seem expected, since these tests are under
"tests/shell" directory, but any executable file would work.

Anyway. The vast majority of the tests has "#!/bin/bash" as shebang.
A few tests had "#!/bin/sh" or "#!/bin/sh -e". Unify this and always use bash.
Since we anyway require bash, this is not a limitation.

Also, if we know that this is a bash script (by parsing the shebang), we
can let the test wrapper pass "-x" to the script. The next commit will
do that, and it is nicer if the shebangs are all uniform.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/testcases/chains/0014rename_0            | 2 +-
 tests/shell/testcases/chains/0044chain_destroy_0     | 2 +-
 tests/shell/testcases/flowtable/0015destroy_0        | 2 +-
 tests/shell/testcases/maps/0014destroy_0             | 2 +-
 tests/shell/testcases/rule_management/0010replace_0  | 2 +-
 tests/shell/testcases/rule_management/0012destroy_0  | 2 +-
 tests/shell/testcases/sets/0043concatenated_ranges_0 | 2 +-
 tests/shell/testcases/sets/0043concatenated_ranges_1 | 2 +-
 tests/shell/testcases/sets/0044interval_overlap_0    | 2 +-
 tests/shell/testcases/sets/0044interval_overlap_1    | 2 +-
 tests/shell/testcases/sets/0072destroy_0             | 2 +-
 tests/shell/testcases/transactions/bad_expression    | 2 +-
 12 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tests/shell/testcases/chains/0014rename_0 b/tests/shell/testcases/chains/0014rename_0
index bebe48d67af9..bd84e95784a7 100755
--- a/tests/shell/testcases/chains/0014rename_0
+++ b/tests/shell/testcases/chains/0014rename_0
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 $NFT add table t || exit 1
 $NFT add chain t c1 || exit 1
diff --git a/tests/shell/testcases/chains/0044chain_destroy_0 b/tests/shell/testcases/chains/0044chain_destroy_0
index 1763d802c1dd..5c5a10a7b9c8 100755
--- a/tests/shell/testcases/chains/0044chain_destroy_0
+++ b/tests/shell/testcases/chains/0044chain_destroy_0
@@ -1,4 +1,4 @@
-#!/bin/sh -e
+#!/bin/bash -e
 
 # NFT_TEST_REQUIRES(NFT_TEST_HAVE_destroy)
 
diff --git a/tests/shell/testcases/flowtable/0015destroy_0 b/tests/shell/testcases/flowtable/0015destroy_0
index 9e91ef5036a2..d2a87da080fb 100755
--- a/tests/shell/testcases/flowtable/0015destroy_0
+++ b/tests/shell/testcases/flowtable/0015destroy_0
@@ -1,4 +1,4 @@
-#!/bin/sh -e
+#!/bin/bash -e
 
 # NFT_TEST_REQUIRES(NFT_TEST_HAVE_destroy)
 
diff --git a/tests/shell/testcases/maps/0014destroy_0 b/tests/shell/testcases/maps/0014destroy_0
index b17d0021d926..ee81e3cdcca9 100755
--- a/tests/shell/testcases/maps/0014destroy_0
+++ b/tests/shell/testcases/maps/0014destroy_0
@@ -1,4 +1,4 @@
-#!/bin/sh -e
+#!/bin/bash -e
 
 # NFT_TEST_REQUIRES(NFT_TEST_HAVE_destroy)
 
diff --git a/tests/shell/testcases/rule_management/0010replace_0 b/tests/shell/testcases/rule_management/0010replace_0
index 251cebb26ec0..cd69a89d9861 100755
--- a/tests/shell/testcases/rule_management/0010replace_0
+++ b/tests/shell/testcases/rule_management/0010replace_0
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 # test for kernel commit ca08987885a147643817d02bf260bc4756ce8cd4
 # ("netfilter: nf_tables: deactivate expressions in rule replecement routine")
diff --git a/tests/shell/testcases/rule_management/0012destroy_0 b/tests/shell/testcases/rule_management/0012destroy_0
index 46a906cf36b8..a058150fed14 100755
--- a/tests/shell/testcases/rule_management/0012destroy_0
+++ b/tests/shell/testcases/rule_management/0012destroy_0
@@ -1,4 +1,4 @@
-#!/bin/sh -e
+#!/bin/bash -e
 
 # NFT_TEST_REQUIRES(NFT_TEST_HAVE_destroy)
 
diff --git a/tests/shell/testcases/sets/0043concatenated_ranges_0 b/tests/shell/testcases/sets/0043concatenated_ranges_0
index 4165b2f5f711..83d743503c7b 100755
--- a/tests/shell/testcases/sets/0043concatenated_ranges_0
+++ b/tests/shell/testcases/sets/0043concatenated_ranges_0
@@ -1,4 +1,4 @@
-#!/bin/sh -e
+#!/bin/bash -e
 #
 # NFT_TEST_SKIP(NFT_TEST_SKIP_slow)
 #
diff --git a/tests/shell/testcases/sets/0043concatenated_ranges_1 b/tests/shell/testcases/sets/0043concatenated_ranges_1
index bab189c56d8c..1be2889352c9 100755
--- a/tests/shell/testcases/sets/0043concatenated_ranges_1
+++ b/tests/shell/testcases/sets/0043concatenated_ranges_1
@@ -1,4 +1,4 @@
-#!/bin/sh -e
+#!/bin/bash -e
 #
 # 0043concatenated_ranges_1 - Insert and list subnets of different sizes
 
diff --git a/tests/shell/testcases/sets/0044interval_overlap_0 b/tests/shell/testcases/sets/0044interval_overlap_0
index 19aa6f5ed081..71bf3345a558 100755
--- a/tests/shell/testcases/sets/0044interval_overlap_0
+++ b/tests/shell/testcases/sets/0044interval_overlap_0
@@ -1,4 +1,4 @@
-#!/bin/sh -e
+#!/bin/bash -e
 #
 # NFT_TEST_SKIP(NFT_TEST_SKIP_slow)
 #
diff --git a/tests/shell/testcases/sets/0044interval_overlap_1 b/tests/shell/testcases/sets/0044interval_overlap_1
index 905e6d5a0348..cdd0c8446f1b 100755
--- a/tests/shell/testcases/sets/0044interval_overlap_1
+++ b/tests/shell/testcases/sets/0044interval_overlap_1
@@ -1,4 +1,4 @@
-#!/bin/sh -e
+#!/bin/bash -e
 #
 # NFT_TEST_SKIP(NFT_TEST_SKIP_slow)
 #
diff --git a/tests/shell/testcases/sets/0072destroy_0 b/tests/shell/testcases/sets/0072destroy_0
index 6399dd0ff4c8..9886a9b04463 100755
--- a/tests/shell/testcases/sets/0072destroy_0
+++ b/tests/shell/testcases/sets/0072destroy_0
@@ -1,4 +1,4 @@
-#!/bin/sh -e
+#!/bin/bash -e
 
 # NFT_TEST_REQUIRES(NFT_TEST_HAVE_destroy)
 
diff --git a/tests/shell/testcases/transactions/bad_expression b/tests/shell/testcases/transactions/bad_expression
index a820c2b98c39..794b62581b62 100755
--- a/tests/shell/testcases/transactions/bad_expression
+++ b/tests/shell/testcases/transactions/bad_expression
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 
 # table with invalid expression (masquerade called from filter table).
 # nft must return an error.  Also catch nfnetlink retry loops that
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH nft 2/2] tests/shell: honor NFT_TEST_VERBOSE_TEST variable to debug tests via `bash -x`
  2023-10-16 13:30 [PATCH nft 1/2] tests/shell: use bash instead of /bin/sh for tests Thomas Haller
@ 2023-10-16 13:30 ` Thomas Haller
  2023-10-16 19:43   ` [PATCH nft v2 " Thomas Haller
  0 siblings, 1 reply; 3+ messages in thread
From: Thomas Haller @ 2023-10-16 13:30 UTC (permalink / raw)
  To: NetFilter; +Cc: Thomas Haller

It can be cumbersome to debug why a test fails. Our tests are just shell
scripts, which for the most part don't print much. That is good, but for
debugging, it can be useful to run the test via `bash -x`. Previously,
we would just patch the source file while debugging.

Add an option "-x" and NFT_TEST_VERBOSE_TEST=y environment variable. If set,
"test-wrapper.sh" will check whether the shebang is "#!/bin/bash" and add
"-x" to the command line.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 tests/shell/helpers/test-wrapper.sh |  9 ++++++++-
 tests/shell/run-tests.sh            | 15 ++++++++++++++-
 2 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/tests/shell/helpers/test-wrapper.sh b/tests/shell/helpers/test-wrapper.sh
index 13b918f8b8e1..832bd89a19bc 100755
--- a/tests/shell/helpers/test-wrapper.sh
+++ b/tests/shell/helpers/test-wrapper.sh
@@ -93,7 +93,14 @@ if [ "$rc_test" -eq 0 ] ; then
 fi
 
 if [ "$rc_test" -eq 0 ] ; then
-	"$TEST" &>> "$NFT_TEST_TESTTMPDIR/testout.log" || rc_test=$?
+	CMD=( "$TEST" )
+	if [ "$NFT_TEST_VERBOSE_TEST" = y ] ; then
+		X="$(sed -n '1 s/^#!\(\/bin\/bash\>.*$\)/\1/p' "$TEST" 2>/dev/null)"
+		if [ -n "$X" ] ; then
+			CMD=( $X -x "$TEST" )
+		fi
+	fi
+	"${CMD[@]}" &>> "$NFT_TEST_TESTTMPDIR/testout.log" || rc_test=$?
 fi
 
 $NFT list ruleset > "$NFT_TEST_TESTTMPDIR/ruleset-after"
diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 22105c2e90e2..27a0ec43042a 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -163,6 +163,7 @@ usage() {
 	echo " -R|--without-realroot : Sets NFT_TEST_HAS_REALROOT=n."
 	echo " -U|--no-unshare : Sets NFT_TEST_UNSHARE_CMD=\"\"."
 	echo " -k|--keep-logs  : Sets NFT_TEST_KEEP_LOGS=y."
+	echo " -x              : Sets NFT_TEST_VERBOSE_TEST=y."
 	echo " -s|--sequential : Sets NFT_TEST_JOBS=0, which also enables global cleanups."
 	echo "                   Also sets NFT_TEST_SHUFFLE_TESTS=n if left unspecified."
 	echo " -Q|--quick      : Sets NFT_TEST_SKIP_slow=y."
@@ -181,6 +182,8 @@ usage() {
 	echo " NFT_REAL=<CMD> : Real nft comand. Usually this is just the same as \$NFT,"
 	echo "                 however, you may set NFT='valgrind nft' and NFT_REAL to the real command."
 	echo " VERBOSE=*|y   : Enable verbose output."
+	echo " NFT_TEST_VERBOSE_TEST=*|y: if true, enable verbose output for tests. For bash scripts, this means"
+	echo "                 to pass \"-x\" to the interpreter."
 	echo " DUMPGEN=*|y   : Regenerate dump files. Dump files are only recreated if the"
 	echo "                 test completes successfully and the \"dumps\" directory for the"
 	echo "                 test exits."
@@ -275,6 +278,7 @@ _NFT_TEST_JOBS_DEFAULT="$(nproc)"
 _NFT_TEST_JOBS_DEFAULT="$(( _NFT_TEST_JOBS_DEFAULT + (_NFT_TEST_JOBS_DEFAULT + 1) / 2 ))"
 
 VERBOSE="$(bool_y "$VERBOSE")"
+NFT_TEST_VERBOSE_TEST="$(bool_y "$NFT_TEST_VERBOSE_TEST")"
 DUMPGEN="$(bool_y "$DUMPGEN")"
 VALGRIND="$(bool_y "$VALGRIND")"
 KMEMLEAK="$(bool_y "$KMEMLEAK")"
@@ -327,6 +331,9 @@ while [ $# -gt 0 ] ; do
 		-v)
 			VERBOSE=y
 			;;
+		-x)
+			NFT_TEST_VERBOSE_TEST=y
+			;;
 		-g)
 			DUMPGEN=y
 			;;
@@ -627,6 +634,7 @@ exec &> >(tee "$NFT_TEST_TMPDIR/test.log")
 msg_info "conf: NFT=$(printf '%q' "$NFT")"
 msg_info "conf: NFT_REAL=$(printf '%q' "$NFT_REAL")"
 msg_info "conf: VERBOSE=$(printf '%q' "$VERBOSE")"
+msg_info "conf: NFT_TEST_VERBOSE_TEST=$(printf '%q' "$NFT_TEST_VERBOSE_TEST")"
 msg_info "conf: DUMPGEN=$(printf '%q' "$DUMPGEN")"
 msg_info "conf: VALGRIND=$(printf '%q' "$VALGRIND")"
 msg_info "conf: KMEMLEAK=$(printf '%q' "$KMEMLEAK")"
@@ -832,7 +840,12 @@ job_start() {
 	fi
 
 	NFT_TEST_TESTTMPDIR="${JOBS_TEMPDIR["$testfile"]}" \
-	NFT="$NFT" NFT_REAL="$NFT_REAL" DIFF="$DIFF" DUMPGEN="$DUMPGEN" $NFT_TEST_UNSHARE_CMD "$NFT_TEST_BASEDIR/helpers/test-wrapper.sh" "$testfile"
+	NFT="$NFT" \
+	NFT_REAL="$NFT_REAL" \
+	DIFF="$DIFF" \
+	DUMPGEN="$DUMPGEN" \
+	NFT_TEST_VERBOSE_TEST="$NFT_TEST_VERBOSE_TEST" \
+	$NFT_TEST_UNSHARE_CMD "$NFT_TEST_BASEDIR/helpers/test-wrapper.sh" "$testfile"
 	local rc_got=$?
 
 	if [ "$NFT_TEST_JOBS" -le 1 ] ; then
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH nft v2 2/2] tests/shell: honor NFT_TEST_VERBOSE_TEST variable to debug tests via `bash -x`
  2023-10-16 13:30 ` [PATCH nft 2/2] tests/shell: honor NFT_TEST_VERBOSE_TEST variable to debug tests via `bash -x` Thomas Haller
@ 2023-10-16 19:43   ` Thomas Haller
  0 siblings, 0 replies; 3+ messages in thread
From: Thomas Haller @ 2023-10-16 19:43 UTC (permalink / raw)
  To: NetFilter; +Cc: Thomas Haller

It can be cumbersome to debug why a test fails. Our tests are just shell
scripts, which for the most part don't print much. That is good, but for
debugging, it can be useful to run the test via `bash -x`. Previously,
we would just patch the source file while debugging.

Add an option "-x" and NFT_TEST_VERBOSE_TEST=y environment variable. If set,
"test-wrapper.sh" will check whether the shebang is "#!/bin/bash" and add
"-x" to the command line.

While at it, let test-wrapper.sh also log a line like

    Command: $CMD

With this, we see in the log the command that was run, and how
NFT_TEST_VERBOSE_TEST may have affected it. This is anyway useful,
because many tests don't print anything at all, and we end up with an
empty "testout.log". Empty files are cumbersome, e.g. I like to use
`grep -R ^` to show the content of all files, which does not show empty
files. Ensuring that something is always written is desirable.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
This replaces "[PATCH nft 2/2] tests/shell: honor NFT_TEST_VERBOSE_TEST variable to debug tests via `bash -x`"
Compared to v1, add a comment about how parsing of the shebang is done
differently by the script (compared to execve). Also, log the actually
executed command to "testout.log" file.

 tests/shell/helpers/test-wrapper.sh | 15 ++++++++++++++-
 tests/shell/run-tests.sh            | 15 ++++++++++++++-
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/tests/shell/helpers/test-wrapper.sh b/tests/shell/helpers/test-wrapper.sh
index 13b918f8b8e1..872a0c56ed54 100755
--- a/tests/shell/helpers/test-wrapper.sh
+++ b/tests/shell/helpers/test-wrapper.sh
@@ -93,7 +93,20 @@ if [ "$rc_test" -eq 0 ] ; then
 fi
 
 if [ "$rc_test" -eq 0 ] ; then
-	"$TEST" &>> "$NFT_TEST_TESTTMPDIR/testout.log" || rc_test=$?
+	CMD=( "$TEST" )
+	if [ "$NFT_TEST_VERBOSE_TEST" = y ] ; then
+		X="$(sed -n '1 s/^#!\(\/bin\/bash\>.*$\)/\1/p' "$TEST" 2>/dev/null)"
+		if [ -n "$X" ] ; then
+			# Note that kernel parses the shebang differently and does not
+			# word splitting for the arguments. We do split the arguments here
+			# which would matter if there are spaces. For our tests, there
+			# are either no arguments or only one argument without space. So
+			# this is good enough.
+			CMD=( $X -x "$TEST" )
+		fi
+	fi
+	printf "Command: $(printf '%q ' "${CMD[@]}")\n" &>> "$NFT_TEST_TESTTMPDIR/testout.log"
+	"${CMD[@]}" &>> "$NFT_TEST_TESTTMPDIR/testout.log" || rc_test=$?
 fi
 
 $NFT list ruleset > "$NFT_TEST_TESTTMPDIR/ruleset-after"
diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 22105c2e90e2..27a0ec43042a 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -163,6 +163,7 @@ usage() {
 	echo " -R|--without-realroot : Sets NFT_TEST_HAS_REALROOT=n."
 	echo " -U|--no-unshare : Sets NFT_TEST_UNSHARE_CMD=\"\"."
 	echo " -k|--keep-logs  : Sets NFT_TEST_KEEP_LOGS=y."
+	echo " -x              : Sets NFT_TEST_VERBOSE_TEST=y."
 	echo " -s|--sequential : Sets NFT_TEST_JOBS=0, which also enables global cleanups."
 	echo "                   Also sets NFT_TEST_SHUFFLE_TESTS=n if left unspecified."
 	echo " -Q|--quick      : Sets NFT_TEST_SKIP_slow=y."
@@ -181,6 +182,8 @@ usage() {
 	echo " NFT_REAL=<CMD> : Real nft comand. Usually this is just the same as \$NFT,"
 	echo "                 however, you may set NFT='valgrind nft' and NFT_REAL to the real command."
 	echo " VERBOSE=*|y   : Enable verbose output."
+	echo " NFT_TEST_VERBOSE_TEST=*|y: if true, enable verbose output for tests. For bash scripts, this means"
+	echo "                 to pass \"-x\" to the interpreter."
 	echo " DUMPGEN=*|y   : Regenerate dump files. Dump files are only recreated if the"
 	echo "                 test completes successfully and the \"dumps\" directory for the"
 	echo "                 test exits."
@@ -275,6 +278,7 @@ _NFT_TEST_JOBS_DEFAULT="$(nproc)"
 _NFT_TEST_JOBS_DEFAULT="$(( _NFT_TEST_JOBS_DEFAULT + (_NFT_TEST_JOBS_DEFAULT + 1) / 2 ))"
 
 VERBOSE="$(bool_y "$VERBOSE")"
+NFT_TEST_VERBOSE_TEST="$(bool_y "$NFT_TEST_VERBOSE_TEST")"
 DUMPGEN="$(bool_y "$DUMPGEN")"
 VALGRIND="$(bool_y "$VALGRIND")"
 KMEMLEAK="$(bool_y "$KMEMLEAK")"
@@ -327,6 +331,9 @@ while [ $# -gt 0 ] ; do
 		-v)
 			VERBOSE=y
 			;;
+		-x)
+			NFT_TEST_VERBOSE_TEST=y
+			;;
 		-g)
 			DUMPGEN=y
 			;;
@@ -627,6 +634,7 @@ exec &> >(tee "$NFT_TEST_TMPDIR/test.log")
 msg_info "conf: NFT=$(printf '%q' "$NFT")"
 msg_info "conf: NFT_REAL=$(printf '%q' "$NFT_REAL")"
 msg_info "conf: VERBOSE=$(printf '%q' "$VERBOSE")"
+msg_info "conf: NFT_TEST_VERBOSE_TEST=$(printf '%q' "$NFT_TEST_VERBOSE_TEST")"
 msg_info "conf: DUMPGEN=$(printf '%q' "$DUMPGEN")"
 msg_info "conf: VALGRIND=$(printf '%q' "$VALGRIND")"
 msg_info "conf: KMEMLEAK=$(printf '%q' "$KMEMLEAK")"
@@ -832,7 +840,12 @@ job_start() {
 	fi
 
 	NFT_TEST_TESTTMPDIR="${JOBS_TEMPDIR["$testfile"]}" \
-	NFT="$NFT" NFT_REAL="$NFT_REAL" DIFF="$DIFF" DUMPGEN="$DUMPGEN" $NFT_TEST_UNSHARE_CMD "$NFT_TEST_BASEDIR/helpers/test-wrapper.sh" "$testfile"
+	NFT="$NFT" \
+	NFT_REAL="$NFT_REAL" \
+	DIFF="$DIFF" \
+	DUMPGEN="$DUMPGEN" \
+	NFT_TEST_VERBOSE_TEST="$NFT_TEST_VERBOSE_TEST" \
+	$NFT_TEST_UNSHARE_CMD "$NFT_TEST_BASEDIR/helpers/test-wrapper.sh" "$testfile"
 	local rc_got=$?
 
 	if [ "$NFT_TEST_JOBS" -le 1 ] ; then
-- 
2.41.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-10-16 19:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-16 13:30 [PATCH nft 1/2] tests/shell: use bash instead of /bin/sh for tests Thomas Haller
2023-10-16 13:30 ` [PATCH nft 2/2] tests/shell: honor NFT_TEST_VERBOSE_TEST variable to debug tests via `bash -x` Thomas Haller
2023-10-16 19:43   ` [PATCH nft v2 " Thomas Haller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).