[nft PATCH v2 0/7] Run all test suites via 'make check'

[nft PATCH v2 0/7] Run all test suites via 'make check'

[Phil Sutter]

Help me (and maybe others) to not occasionally forget to run this or
that test suite in this or that mode: Implement "run all variants"
mode in test runners (patches 2 and 3), make sure their exit codes
match Automake expectations (patch 1) and register them with Automake
(patch 7). Also fix for running 'make check' as non-root (patches 4 and
5) and calling build test suite from outside its directory (patch 6).

Changes since v1:
- Also integrate build test suite
- Populate TESTS variable only for non-distcheck builds, so 'make
  distcheck' does not run any test suite

Phil Sutter (7):
  tests: Prepare exit codes for automake
  tests: monitor: Support running all tests in one go
  tests: py: Set default options based on RUN_FULL_TESTSUITE
  tests: json_echo: Skip if run as non-root
  tests: shell: Skip packetpath/nat_ftp in fake root env
  tests: build: Do not assume caller's CWD
  Makefile: Enable support for 'make check'

 Makefile.am                              | 10 +++++++
 configure.ac                             |  5 ++++
 tests/build/run-tests.sh                 |  2 ++
 tests/json_echo/run-test.py              |  4 +++
 tests/monitor/run-tests.sh               | 34 ++++++++++++++----------
 tests/py/nft-test.py                     | 19 +++++++++----
 tests/shell/run-tests.sh                 |  2 +-
 tests/shell/testcases/packetpath/nat_ftp |  3 +++
 8 files changed, 59 insertions(+), 20 deletions(-)

-- 
2.51.0

[nft PATCH v2 1/7] tests: Prepare exit codes for automake

[Phil Sutter]

Make the test suite runners exit 77 when requiring root and running as
regular user, exit 99 for internal errors (unrelated to test cases) and
exit 1 (or any free non-zero value) to indicate test failures.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/monitor/run-tests.sh | 11 ++++-------
 tests/py/nft-test.py       | 12 +++++++-----
 tests/shell/run-tests.sh   |  2 +-
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
index 67d3e618cee07..969afe249201b 100755
--- a/tests/monitor/run-tests.sh
+++ b/tests/monitor/run-tests.sh
@@ -12,18 +12,15 @@ err() {
 	echo "$*" >&2
 }
 
-die() {
-	err "$*"
-	exit 1
-}
-
 if [ "$(id -u)" != "0" ] ; then
-	die "this requires root!"
+	err "this requires root!"
+	exit 77
 fi
 
 testdir=$(mktemp -d)
 if [ ! -d $testdir ]; then
-	die "Failed to create test directory"
+	err "Failed to create test directory"
+	exit 99
 fi
 trap 'rm -rf $testdir; $nft flush ruleset' EXIT
 
diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
index 984f2b937a077..78f3fa9b27df7 100755
--- a/tests/py/nft-test.py
+++ b/tests/py/nft-test.py
@@ -1519,7 +1519,7 @@ def set_delete_elements(set_element, set_name, table, filename=None,
 
     if os.getuid() != 0:
         print("You need to be root to run this, sorry")
-        return
+        return 77
 
     if not args.no_netns and not spawn_netns():
         print_warning("cannot run in own namespace, connectivity might break")
@@ -1538,11 +1538,11 @@ def set_delete_elements(set_element, set_name, table, filename=None,
     if check_lib_path and not os.path.exists(args.library):
         print("The nftables library at '%s' does not exist. "
               "You need to build the project." % args.library)
-        return
+        return 99
 
     if args.enable_schema and not args.enable_json:
         print_error("Option --schema requires option --json")
-        return
+        return 99
 
     global nftables
     nftables = Nftables(sofile = args.library)
@@ -1555,7 +1555,7 @@ def set_delete_elements(set_element, set_name, table, filename=None,
         print_info("Log will be available at %s" % LOGFILE)
     except IOError:
         print_error("Cannot open log file %s" % LOGFILE)
-        return
+        return 99
 
     file_list = []
     if args.filenames:
@@ -1601,5 +1601,7 @@ def set_delete_elements(set_element, set_name, table, filename=None,
                 print("%d test files, %d files passed, %d unit tests, " % (test_files, files_ok, tests))
                 print("%d error, %d warning" % (errors, warnings))
 
+    return errors != 0
+
 if __name__ == '__main__':
-    main()
+    sys.exit(main())
diff --git a/tests/shell/run-tests.sh b/tests/shell/run-tests.sh
index 2d2e0ad146c80..46f523b962b13 100755
--- a/tests/shell/run-tests.sh
+++ b/tests/shell/run-tests.sh
@@ -96,7 +96,7 @@ _msg() {
 		printf '%s\n' "$level: $*"
 	fi
 	if [ "$level" = E ] ; then
-		exit 1
+		exit 99
 	fi
 }
 
-- 
2.51.0

[nft PATCH v2 2/7] tests: monitor: Support running all tests in one go

[Phil Sutter]

Detect RUN_FULL_TESTSUITE env variable set by automake and do an
"unattended" full testrun.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/monitor/run-tests.sh | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
index 969afe249201b..b8589344a9732 100755
--- a/tests/monitor/run-tests.sh
+++ b/tests/monitor/run-tests.sh
@@ -58,7 +58,7 @@ json_output_filter() { # (filename)
 monitor_run_test() {
 	monitor_output=$(mktemp -p $testdir)
 	monitor_args=""
-	$test_json && monitor_args="vm json"
+	$json_mode && monitor_args="vm json"
 	local rc=0
 
 	$nft -nn monitor $monitor_args >$monitor_output &
@@ -77,7 +77,7 @@ monitor_run_test() {
 	sleep 0.5
 	kill $monitor_pid
 	wait >/dev/null 2>&1
-	$test_json && json_output_filter $monitor_output
+	$json_mode && json_output_filter $monitor_output
 	mydiff -q $monitor_output $output_file >/dev/null 2>&1
 	if [[ $rc == 0 && $? != 0 ]]; then
 		err "monitor output differs!"
@@ -156,20 +156,29 @@ while [ -n "$1" ]; do
 	esac
 done
 
-if $test_json; then
-	variants="monitor"
+if [[ $RUN_FULL_TESTSUITE == 1 ]]; then
+	variants="monitor_json monitor echo"
+elif $test_json; then
+	variants="monitor_json"
 else
 	variants="monitor echo"
 fi
 
 rc=0
 for variant in $variants; do
+	orig_variant=$variant
+	if [[ $variant =~ .*_json ]]; then
+		variant=${variant%_json}
+		json_mode=true
+	else
+		json_mode=false
+	fi
 	run_test=${variant}_run_test
 	output_append=${variant}_output_append
 
 	for testcase in ${testcases:-testcases/*.t}; do
 		filename=$(basename $testcase)
-		echo "$variant: running tests from file $filename"
+		echo "$orig_variant: running tests from file $filename"
 		rc_start=$rc
 
 		# files are like this:
@@ -194,11 +203,11 @@ for variant in $variants; do
 				;;
 			O)
 				input_complete=true
-				$test_json || $output_append "$line"
+				$json_mode || $output_append "$line"
 				;;
 			J)
 				input_complete=true
-				$test_json && $output_append "$line"
+				$json_mode && $output_append "$line"
 				;;
 			'#'|'')
 				# ignore comments and empty lines
-- 
2.51.0

[nft PATCH v2 3/7] tests: py: Set default options based on RUN_FULL_TESTSUITE

[Phil Sutter]

Automake is supposed to set this for a full testrun.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/py/nft-test.py | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
index 78f3fa9b27df7..52be394c1975a 100755
--- a/tests/py/nft-test.py
+++ b/tests/py/nft-test.py
@@ -1517,6 +1517,13 @@ def set_delete_elements(set_element, set_name, table, filename=None,
     signal.signal(signal.SIGINT, signal_handler)
     signal.signal(signal.SIGTERM, signal_handler)
 
+    try:
+        if os.environ["RUN_FULL_TESTSUITE"] != 0:
+            enable_json_option = True
+            enable_json_schema = True
+    except KeyError:
+        pass
+
     if os.getuid() != 0:
         print("You need to be root to run this, sorry")
         return 77
-- 
2.51.0

[nft PATCH v2 4/7] tests: json_echo: Skip if run as non-root

[Phil Sutter]

The test suite manipulates the kernel ruleset. Use the well-known return
code 77 to indicate test execution being skipped.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/json_echo/run-test.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/json_echo/run-test.py b/tests/json_echo/run-test.py
index a6bdfc61afd7b..a3085b35ade6b 100755
--- a/tests/json_echo/run-test.py
+++ b/tests/json_echo/run-test.py
@@ -6,6 +6,10 @@ import os
 import json
 import argparse
 
+if os.getuid() != 0:
+    print("You need to be root to run this, sorry")
+    sys.exit(77)
+
 TESTS_PATH = os.path.dirname(os.path.abspath(__file__))
 sys.path.insert(0, os.path.join(TESTS_PATH, '../../py/'))
 
-- 
2.51.0

[nft PATCH v2 5/7] tests: shell: Skip packetpath/nat_ftp in fake root env

[Phil Sutter]

The script relies upon a call to modprobe which does not work in
fake root environments.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/shell/testcases/packetpath/nat_ftp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/shell/testcases/packetpath/nat_ftp b/tests/shell/testcases/packetpath/nat_ftp
index c2fb3a1c8ebcd..d0faf2ef59c57 100755
--- a/tests/shell/testcases/packetpath/nat_ftp
+++ b/tests/shell/testcases/packetpath/nat_ftp
@@ -4,6 +4,9 @@
 # NFT_TEST_REQUIRES(NFT_TEST_HAVE_curl)
 # NFT_TEST_REQUIRES(NFT_TEST_HAVE_vsftpd)
 
+# modprobe does not work in fake root env
+[ "$NFT_TEST_HAS_REALROOT" != y ] && exit 77
+
 . $NFT_TEST_LIBRARY_FILE
 
 cleanup()
-- 
2.51.0

[nft PATCH v2 6/7] tests: build: Do not assume caller's CWD

[Phil Sutter]

Cover for being called from a different directory by changing into the
test suite's directory first.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/build/run-tests.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/build/run-tests.sh b/tests/build/run-tests.sh
index 674383cb6cc74..a5e026a97dd5b 100755
--- a/tests/build/run-tests.sh
+++ b/tests/build/run-tests.sh
@@ -1,5 +1,7 @@
 #!/bin/bash
 
+cd $(dirname $0)
+
 log_file="$(pwd)/tests.log"
 dir=../..
 argument=( --without-cli --with-cli=linenoise --with-cli=editline --enable-debug --with-mini-gmp
-- 
2.51.0

[nft PATCH v2 7/7] Makefile: Enable support for 'make check'

[Phil Sutter]

Add the various testsuite runners to TESTS variable and have make call
them with RUN_FULL_TESTSUITE=1 env var.

Most of the test suites require privileged execution, 'make distcheck'
usually doesn't and probably shouldn't. Assuming the latter is used
during the release process, it may even not run on a machine which is up
to date enough to generate meaningful test suite results. Hence spare
the release process from the likely pointless delay imposed by 'make
check' by keeping TESTS variable empty when in a distcheck environment.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Add an internal configure option set by the distcheck target when
  building the project
- Have this configure option define BUILD_DISTCHECK automake variable
- Leave TESTS empty if BUILD_DISTCHECK is set to avoid test suite runs
  with 'make distcheck'
---
 Makefile.am  | 10 ++++++++++
 configure.ac |  5 +++++
 2 files changed, 15 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 5190a49ae69f1..f65e8d51b501e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -23,6 +23,7 @@ libnftables_LIBVERSION = 2:0:1
 ###############################################################################
 
 ACLOCAL_AMFLAGS = -I m4
+AM_DISTCHECK_CONFIGURE_FLAGS = --enable-distcheck
 
 EXTRA_DIST =
 BUILT_SOURCES =
@@ -429,3 +430,12 @@ doc_DATA = files/nftables/main.nft
 tools/nftables.service: tools/nftables.service.in ${top_builddir}/config.status
 	${AM_V_GEN}${MKDIR_P} tools
 	${AM_V_at}sed -e 's|@''sbindir''@|${sbindir}|g;s|@''pkgsysconfdir''@|${pkgsysconfdir}|g' <${srcdir}/tools/nftables.service.in >$@
+
+AM_TESTS_ENVIRONMENT = RUN_FULL_TESTSUITE=1; export RUN_FULL_TESTSUITE;
+if !BUILD_DISTCHECK
+TESTS = tests/build/run-tests.sh \
+	tests/json_echo/run-test.py \
+	tests/monitor/run-tests.sh \
+	tests/py/nft-test.py \
+	tests/shell/run-tests.sh
+endif
diff --git a/configure.ac b/configure.ac
index da16a6e257c91..8073d4d8193e2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -155,6 +155,11 @@ AC_CONFIG_COMMANDS([nftversion.h], [
 AC_SUBST([MAKE_STAMP], ["\$(shell date +%s)"])
 CFLAGS="${CFLAGS} -DMAKE_STAMP=\${MAKE_STAMP}"
 
+AC_ARG_ENABLE([distcheck],
+	      AS_HELP_STRING([--enable-distcheck], [Build for distcheck]),
+	      [enable_distcheck=yes], [])
+AM_CONDITIONAL([BUILD_DISTCHECK], [test "x$enable_distcheck" = "xyes"])
+
 AC_CONFIG_FILES([					\
 		Makefile				\
 		libnftables.pc				\
-- 
2.51.0

Re: [nft PATCH v2 2/7] tests: monitor: Support running all tests in one go

[Pablo Neira Ayuso]

On Fri, Aug 29, 2025 at 05:51:58PM +0200, Phil Sutter wrote:
> Detect RUN_FULL_TESTSUITE env variable set by automake and do an
> "unattended" full testrun.

This test is so small that I think it is better to enable both modes:
w/json and w/o json by default.

> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  tests/monitor/run-tests.sh | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
> index 969afe249201b..b8589344a9732 100755
> --- a/tests/monitor/run-tests.sh
> +++ b/tests/monitor/run-tests.sh
> @@ -58,7 +58,7 @@ json_output_filter() { # (filename)
>  monitor_run_test() {
>  	monitor_output=$(mktemp -p $testdir)
>  	monitor_args=""
> -	$test_json && monitor_args="vm json"
> +	$json_mode && monitor_args="vm json"
>  	local rc=0
>  
>  	$nft -nn monitor $monitor_args >$monitor_output &
> @@ -77,7 +77,7 @@ monitor_run_test() {
>  	sleep 0.5
>  	kill $monitor_pid
>  	wait >/dev/null 2>&1
> -	$test_json && json_output_filter $monitor_output
> +	$json_mode && json_output_filter $monitor_output
>  	mydiff -q $monitor_output $output_file >/dev/null 2>&1
>  	if [[ $rc == 0 && $? != 0 ]]; then
>  		err "monitor output differs!"
> @@ -156,20 +156,29 @@ while [ -n "$1" ]; do
>  	esac
>  done
>  
> -if $test_json; then
> -	variants="monitor"
> +if [[ $RUN_FULL_TESTSUITE == 1 ]]; then
> +	variants="monitor_json monitor echo"
> +elif $test_json; then
> +	variants="monitor_json"
>  else
>  	variants="monitor echo"
>  fi
>  
>  rc=0
>  for variant in $variants; do
> +	orig_variant=$variant
> +	if [[ $variant =~ .*_json ]]; then
> +		variant=${variant%_json}
> +		json_mode=true
> +	else
> +		json_mode=false
> +	fi
>  	run_test=${variant}_run_test
>  	output_append=${variant}_output_append
>  
>  	for testcase in ${testcases:-testcases/*.t}; do
>  		filename=$(basename $testcase)
> -		echo "$variant: running tests from file $filename"
> +		echo "$orig_variant: running tests from file $filename"
>  		rc_start=$rc
>  
>  		# files are like this:
> @@ -194,11 +203,11 @@ for variant in $variants; do
>  				;;
>  			O)
>  				input_complete=true
> -				$test_json || $output_append "$line"
> +				$json_mode || $output_append "$line"
>  				;;
>  			J)
>  				input_complete=true
> -				$test_json && $output_append "$line"
> +				$json_mode && $output_append "$line"
>  				;;
>  			'#'|'')
>  				# ignore comments and empty lines
> -- 
> 2.51.0
> 

Re: [nft PATCH v2 7/7] Makefile: Enable support for 'make check'

[Pablo Neira Ayuso]

On Fri, Aug 29, 2025 at 05:52:03PM +0200, Phil Sutter wrote:
> Add the various testsuite runners to TESTS variable and have make call
> them with RUN_FULL_TESTSUITE=1 env var.

Given you add a env var for every test, would you instead use
distcheck-hook: in Makefile.am to short circuit the test run and
display SKIPPED?

> Most of the test suites require privileged execution, 'make distcheck'
> usually doesn't and probably shouldn't. Assuming the latter is used
> during the release process, it may even not run on a machine which is up
> to date enough to generate meaningful test suite results. Hence spare
> the release process from the likely pointless delay imposed by 'make
> check' by keeping TESTS variable empty when in a distcheck environment.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
> Changes since v1:
> - Add an internal configure option set by the distcheck target when
>   building the project
> - Have this configure option define BUILD_DISTCHECK automake variable
> - Leave TESTS empty if BUILD_DISTCHECK is set to avoid test suite runs
>   with 'make distcheck'
> ---
>  Makefile.am  | 10 ++++++++++
>  configure.ac |  5 +++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 5190a49ae69f1..f65e8d51b501e 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -23,6 +23,7 @@ libnftables_LIBVERSION = 2:0:1
>  ###############################################################################
>  
>  ACLOCAL_AMFLAGS = -I m4
> +AM_DISTCHECK_CONFIGURE_FLAGS = --enable-distcheck
>  
>  EXTRA_DIST =
>  BUILT_SOURCES =
> @@ -429,3 +430,12 @@ doc_DATA = files/nftables/main.nft
>  tools/nftables.service: tools/nftables.service.in ${top_builddir}/config.status
>  	${AM_V_GEN}${MKDIR_P} tools
>  	${AM_V_at}sed -e 's|@''sbindir''@|${sbindir}|g;s|@''pkgsysconfdir''@|${pkgsysconfdir}|g' <${srcdir}/tools/nftables.service.in >$@
> +
> +AM_TESTS_ENVIRONMENT = RUN_FULL_TESTSUITE=1; export RUN_FULL_TESTSUITE;
> +if !BUILD_DISTCHECK
> +TESTS = tests/build/run-tests.sh \
> +	tests/json_echo/run-test.py \
> +	tests/monitor/run-tests.sh \
> +	tests/py/nft-test.py \
> +	tests/shell/run-tests.sh
> +endif
> diff --git a/configure.ac b/configure.ac
> index da16a6e257c91..8073d4d8193e2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -155,6 +155,11 @@ AC_CONFIG_COMMANDS([nftversion.h], [
>  AC_SUBST([MAKE_STAMP], ["\$(shell date +%s)"])
>  CFLAGS="${CFLAGS} -DMAKE_STAMP=\${MAKE_STAMP}"
>  
> +AC_ARG_ENABLE([distcheck],
> +	      AS_HELP_STRING([--enable-distcheck], [Build for distcheck]),
> +	      [enable_distcheck=yes], [])
> +AM_CONDITIONAL([BUILD_DISTCHECK], [test "x$enable_distcheck" = "xyes"])
> +
>  AC_CONFIG_FILES([					\
>  		Makefile				\
>  		libnftables.pc				\
> -- 
> 2.51.0
> 
> 

Re: [nft PATCH v2 3/7] tests: py: Set default options based on RUN_FULL_TESTSUITE

[Pablo Neira Ayuso]

On Fri, Aug 29, 2025 at 05:51:59PM +0200, Phil Sutter wrote:
> Automake is supposed to set this for a full testrun.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  tests/py/nft-test.py | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
> index 78f3fa9b27df7..52be394c1975a 100755
> --- a/tests/py/nft-test.py
> +++ b/tests/py/nft-test.py
> @@ -1517,6 +1517,13 @@ def set_delete_elements(set_element, set_name, table, filename=None,
>      signal.signal(signal.SIGINT, signal_handler)
>      signal.signal(signal.SIGTERM, signal_handler)
>  
> +    try:
> +        if os.environ["RUN_FULL_TESTSUITE"] != 0:
> +            enable_json_option = True
> +            enable_json_schema = True
> +    except KeyError:
> +        pass

I would revisit options for tests to:

1) Run all tests by default, ie. native syntax and json.
2) Add options to run native syntax (-pick-one-here) and json test only (-j).

Option 1) (default) will be fine for make check... and CI in general.
Option 2) will be only useful for development, for troubleshooting
broken tests.

Then, add the env variable to shortcircuit tests with distcheck-hook:

Re: [nft PATCH v2 2/7] tests: monitor: Support running all tests in one go

[Phil Sutter]

On Tue, Sep 02, 2025 at 04:43:59PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Aug 29, 2025 at 05:51:58PM +0200, Phil Sutter wrote:
> > Detect RUN_FULL_TESTSUITE env variable set by automake and do an
> > "unattended" full testrun.
> 
> This test is so small that I think it is better to enable both modes:
> w/json and w/o json by default.

ACK, good point. I just investigated and it is feasible to use the
stored JSON output for testing '--echo --json' as well. So I can extend
the test suite to test echo and monitor mode for both standard and JSON
syntax.

Thanks, Phil

Re: [nft PATCH v2 7/7] Makefile: Enable support for 'make check'

[Phil Sutter]

On Tue, Sep 02, 2025 at 04:45:43PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Aug 29, 2025 at 05:52:03PM +0200, Phil Sutter wrote:
> > Add the various testsuite runners to TESTS variable and have make call
> > them with RUN_FULL_TESTSUITE=1 env var.
> 
> Given you add a env var for every test, would you instead use
> distcheck-hook: in Makefile.am to short circuit the test run and
> display SKIPPED?

I don't follow, sorry. The RUN_FULL_TESTSUITE variable is merely used to
enable "run all variants"-mode in test suites. Test suites are skipped
only if they require root and caller is not - one may still run 'make
check' as root or not, irrespective of the hack to leave 'make
distcheck' alone.

Cheers, Phil

Re: [nft PATCH v2 7/7] Makefile: Enable support for 'make check'

[Pablo Neira Ayuso]

On Wed, Sep 03, 2025 at 01:06:43PM +0200, Phil Sutter wrote:
> On Tue, Sep 02, 2025 at 04:45:43PM +0200, Pablo Neira Ayuso wrote:
> > On Fri, Aug 29, 2025 at 05:52:03PM +0200, Phil Sutter wrote:
> > > Add the various testsuite runners to TESTS variable and have make call
> > > them with RUN_FULL_TESTSUITE=1 env var.
> > 
> > Given you add a env var for every test, would you instead use
> > distcheck-hook: in Makefile.am to short circuit the test run and
> > display SKIPPED?
> 
> I don't follow, sorry. The RUN_FULL_TESTSUITE variable is merely used to
> enable "run all variants"-mode in test suites.

For make check, I think tests should be updated to run all variants by
default. Then provide options to run independent variants.

> Test suites are skipped only if they require root and caller is not
> - one may still run 'make check' as root or not, irrespective of the
> hack to leave 'make distcheck' alone.

Right, I call diskcheck as non-root.

Maybe we can just skip tests for non-root when make check is called.

If the tests are updated to run all variants by default, then
RUN_FULL_TESTSUITE is not needed.

Re: [nft PATCH v2 3/7] tests: py: Set default options based on RUN_FULL_TESTSUITE

[Phil Sutter]

On Tue, Sep 02, 2025 at 05:33:34PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Aug 29, 2025 at 05:51:59PM +0200, Phil Sutter wrote:
> > Automake is supposed to set this for a full testrun.
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  tests/py/nft-test.py | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
> > index 78f3fa9b27df7..52be394c1975a 100755
> > --- a/tests/py/nft-test.py
> > +++ b/tests/py/nft-test.py
> > @@ -1517,6 +1517,13 @@ def set_delete_elements(set_element, set_name, table, filename=None,
> >      signal.signal(signal.SIGINT, signal_handler)
> >      signal.signal(signal.SIGTERM, signal_handler)
> >  
> > +    try:
> > +        if os.environ["RUN_FULL_TESTSUITE"] != 0:
> > +            enable_json_option = True
> > +            enable_json_schema = True
> > +    except KeyError:
> > +        pass
> 
> I would revisit options for tests to:
> 
> 1) Run all tests by default, ie. native syntax and json.
> 2) Add options to run native syntax (-pick-one-here) and json test only (-j).

Ah, that's indeed a sensible measure - I did the same for
iptables-test.py and the nice side-effect is that adding a new test-mode
won't require education for callers, they just run it as well.

> 
> Option 1) (default) will be fine for make check... and CI in general.
> Option 2) will be only useful for development, for troubleshooting
> broken tests.
> 
> Then, add the env variable to shortcircuit tests with distcheck-hook:

The interesting part is missing here. ;)
I didn't find a better way to avoid test suite runs from 'make
distcheck' while retaining 'make check'.

Cheers, Phil

Re: [nft PATCH v2 7/7] Makefile: Enable support for 'make check'

[Phil Sutter]

On Wed, Sep 03, 2025 at 01:11:15PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Sep 03, 2025 at 01:06:43PM +0200, Phil Sutter wrote:
> > On Tue, Sep 02, 2025 at 04:45:43PM +0200, Pablo Neira Ayuso wrote:
> > > On Fri, Aug 29, 2025 at 05:52:03PM +0200, Phil Sutter wrote:
> > > > Add the various testsuite runners to TESTS variable and have make call
> > > > them with RUN_FULL_TESTSUITE=1 env var.
> > > 
> > > Given you add a env var for every test, would you instead use
> > > distcheck-hook: in Makefile.am to short circuit the test run and
> > > display SKIPPED?
> > 
> > I don't follow, sorry. The RUN_FULL_TESTSUITE variable is merely used to
> > enable "run all variants"-mode in test suites.
> 
> For make check, I think tests should be updated to run all variants by
> default. Then provide options to run independent variants.
> 
> > Test suites are skipped only if they require root and caller is not
> > - one may still run 'make check' as root or not, irrespective of the
> > hack to leave 'make distcheck' alone.
> 
> Right, I call diskcheck as non-root.
> 
> Maybe we can just skip tests for non-root when make check is called.

I had considered this, but dropping the fakeroot functionality from
shell test suite just for that feels wrong (and is pretty tedious as
well). Keeping it, letting 'make check' run as much as possible and
making sure 'make distcheck' remains unaffected is a cleaner solution
IMO.

> If the tests are updated to run all variants by default, then
> RUN_FULL_TESTSUITE is not needed.

ACK, I'll work on that. It will also improve live for non-"make check"
users.

Thanks, Phil