[nft PATCH v4 0/8] Run all test suites via 'make check'

[nft PATCH v4 0/8] 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:

Have test suites execute all variants by default (patches 1 and 2), make
sure their exit codes match Automake expectations (patch 3) and register
them with Automake (patch 8). Also fix for running 'make check' as
non-root (patches 4 and 5) and calling build test suite from outside its
directory (patch 6).

There is a "funny" problem with build test suite calling 'make
distcheck' which behaves differently under the environment polluted by
the calling 'make check' invocation, details in patch 7.

Changes since v3:
- Applied the initial monitor test suite enhancements already
- gitignore generated logs and reports
- New patch 7

Changes since v2:
- Drop the need for RUN_FULL_TESTSUITE env var by making the "all
  variants" mode the default in all test suites
- Implement JSON echo testing into monitor test suite, stored JSON
  output matches echo output after minor adjustment

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 (8):
  tests: monitor: Excercise all syntaxes and variants by default
  tests: py: Enable JSON and JSON schema by default
  tests: Prepare exit codes for automake
  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
  tests: build: Avoid a recursive 'make check' run
  Makefile: Enable support for 'make check'

 .gitignore                               |  13 ++
 Makefile.am                              |   9 ++
 configure.ac                             |   5 +
 tests/build/run-tests.sh                 |   6 +
 tests/json_echo/run-test.py              |   4 +
 tests/monitor/run-tests.sh               | 145 +++++++++++++----------
 tests/py/nft-test.py                     |  28 +++--
 tests/shell/run-tests.sh                 |   2 +-
 tests/shell/testcases/packetpath/nat_ftp |   3 +
 9 files changed, 143 insertions(+), 72 deletions(-)

-- 
2.51.0

[nft PATCH v4 1/8] tests: monitor: Excercise all syntaxes and variants by default

[Phil Sutter]

Introduce -s/--standard flag to restrict execution to standard syntax
and let users select a specific variant by means of -e/--echo and
-m/--monitor flags. Run all four possible combinations by default.

To keep indenting sane, introduce run_testcase() executing tests in a
single test case for a given syntax and variant.

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

diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
index b09b72ae034cb..32b1b86e0cc6e 100755
--- a/tests/monitor/run-tests.sh
+++ b/tests/monitor/run-tests.sh
@@ -154,14 +154,28 @@ if $netns; then
 fi
 
 testcases=""
+variants=""
+syntaxes=""
 while [ -n "$1" ]; do
 	case "$1" in
 	-d|--debug)
 		debug=true
 		shift
 		;;
+	-s|--standard)
+		syntaxes+=" standard"
+		shift
+		;;
 	-j|--json)
-		test_json=true
+		syntaxes+=" json"
+		shift
+		;;
+	-e|--echo)
+		variants+=" echo"
+		shift
+		;;
+	-m|--monitor)
+		variants+=" monitor"
 		shift
 		;;
 	--no-netns)
@@ -179,64 +193,74 @@ while [ -n "$1" ]; do
 		echo "unknown option '$1'"
 		;&
 	-h|--help)
-		echo "Usage: $(basename $0) [-j|--json] [-d|--debug] [testcase ...]"
+		echo "Usage: $(basename $0) [(-e|--echo)|(-m|--monitor)] [(-j|--json)|(-s|--standard)] [-d|--debug] [testcase ...]"
 		exit 1
 		;;
 	esac
 done
 
-variants="monitor echo"
-rc=0
-for variant in $variants; do
-	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"
-		rc_start=$rc
-
-		# files are like this:
-		#
-		# I add table ip t
-		# O add table ip t
-		# I add chain ip t c
-		# O add chain ip t c
-
-		$nft flush ruleset
-
-		input_complete=false
-		while read dir line; do
-			case $dir in
-			I)
-				$input_complete && {
-					$run_test
-					let "rc += $?"
-				}
-				input_complete=false
-				cmd_append "$line"
-				;;
-			O)
-				input_complete=true
-				$test_json || $output_append "$line"
-				;;
-			J)
-				input_complete=true
-				$test_json && $output_append "$line"
-				;;
-			'#'|'')
-				# ignore comments and empty lines
-				;;
-			esac
-		done <$testcase
-		$input_complete && {
-			$run_test
-			let "rc += $?"
-		}
-
-		let "rc_diff = rc - rc_start"
-		[[ $rc_diff -ne 0 ]] && \
-			echo "$variant: $rc_diff tests from file $filename failed"
+# run the single test in $1
+# expect $variant and $test_json to be set appropriately
+run_testcase() {
+	testcase="$1"
+	filename=$(basename $testcase)
+	rc=0
+	$test_json && printf "json-"
+	echo "$variant: running tests from file $filename"
+
+	# files are like this:
+	#
+	# I add table ip t
+	# O add table ip t
+	# I add chain ip t c
+	# O add chain ip t c
+
+	$nft flush ruleset
+
+	input_complete=false
+	while read dir line; do
+		case $dir in
+		I)
+			$input_complete && {
+				${variant}_run_test
+				$run_test
+				let "rc += $?"
+			}
+			input_complete=false
+			cmd_append "$line"
+			;;
+		O)
+			input_complete=true
+			$test_json || ${variant}_output_append "$line"
+			;;
+		J)
+			input_complete=true
+			$test_json && ${variant}_output_append "$line"
+			;;
+		'#'|'')
+			# ignore comments and empty lines
+			;;
+		esac
+	done <$testcase
+	$input_complete && {
+		${variant}_run_test
+		let "rc += $?"
+	}
+
+	[[ $rc -ne 0 ]] && \
+		echo "$variant: $rc tests from file $filename failed"
+	return $rc
+}
+
+total_rc=0
+for syntax in ${syntaxes:-standard json}; do
+	[ $syntax == json ] && test_json=true || test_json=false
+	for variant in ${variants:-echo monitor}; do
+		for testcase in ${testcases:-testcases/*.t}; do
+			run_testcase "$testcase"
+			let "total_rc += $?"
+		done
 	done
 done
-exit $rc
+
+exit $total_rc
-- 
2.51.0

[nft PATCH v4 2/8] tests: py: Enable JSON and JSON schema by default

[Phil Sutter]

Introduce -J/--disable-json and -S/--no-schema to explicitly disable
them if desired.

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

diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
index 984f2b937a077..12c6174b01257 100755
--- a/tests/py/nft-test.py
+++ b/tests/py/nft-test.py
@@ -1488,7 +1488,11 @@ def set_delete_elements(set_element, set_name, table, filename=None,
 
     parser.add_argument('-j', '--enable-json', action='store_true',
                         dest='enable_json',
-                        help='test JSON functionality as well')
+                        help='test JSON functionality as well (default)')
+
+    parser.add_argument('-J', '--disable-json', action='store_true',
+                        dest='disable_json',
+                        help='Do not test JSON functionality as well')
 
     parser.add_argument('-l', '--library', default=None,
                         help='path to libntables.so.1, overrides --host')
@@ -1499,7 +1503,11 @@ def set_delete_elements(set_element, set_name, table, filename=None,
 
     parser.add_argument('-s', '--schema', action='store_true',
                         dest='enable_schema',
-                        help='verify json input/output against schema')
+                        help='verify json input/output against schema (default)')
+
+    parser.add_argument('-S', '--no-schema', action='store_true',
+                        dest='disable_schema',
+                        help='Do not verify json input/output against schema')
 
     parser.add_argument('-v', '--version', action='version',
                         version='1.0',
@@ -1510,8 +1518,8 @@ def set_delete_elements(set_element, set_name, table, filename=None,
     debug_option = args.debug
     need_fix_option = args.need_fix_line
     force_all_family_option = args.force_all_family
-    enable_json_option = args.enable_json
-    enable_json_schema = args.enable_schema
+    enable_json_option = not args.disable_json
+    enable_json_schema = not args.disable_json and not args.disable_schema
     specific_file = False
 
     signal.signal(signal.SIGINT, signal_handler)
-- 
2.51.0

[nft PATCH v4 3/8] 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 32b1b86e0cc6e..44f21a285b17c 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 12c6174b01257..35b29fc90870b 100755
--- a/tests/py/nft-test.py
+++ b/tests/py/nft-test.py
@@ -1527,7 +1527,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")
@@ -1546,11 +1546,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)
@@ -1563,7 +1563,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:
@@ -1609,5 +1609,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 v4 4/8] 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 v4 5/8] 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 v4 6/8] 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 v4 7/8] tests: build: Avoid a recursive 'make check' run

[Phil Sutter]

When called by 'make check', the test suite runs with a MAKEFLAGS
variable in environment which defines TEST_LOGS variable with the test
suites' corresponding logs as value. This in turn causes the called
'make distcheck' to run test suites although it is not supposed to.

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

diff --git a/tests/build/run-tests.sh b/tests/build/run-tests.sh
index a5e026a97dd5b..80fa10168d003 100755
--- a/tests/build/run-tests.sh
+++ b/tests/build/run-tests.sh
@@ -20,6 +20,10 @@ fi
 git clone "$dir" "$tmpdir" &>>"$log_file"
 cd "$tmpdir" || exit
 
+# do not leak data from a calling 'make check' run into the new build otherwise
+# this will defeat the test suite invocation prevention for 'make distcheck'
+unset MAKEFLAGS
+
 if ! autoreconf -fi &>>"$log_file" ; then
 	echo "Something went wrong. Check the log '${log_file}' for details."
 	exit 1
-- 
2.51.0

[nft PATCH v4 8/8] Makefile: Enable support for 'make check'

[Phil Sutter]

With all test suites running all variants by default, add the various
testsuite runners to TESTS variable so 'make check' will execute them.

Introduce --enable-distcheck configure flag for internal use during
builds triggered by 'make distcheck'. This flag will force TESTS
variable to remain empty, so 'make check' run as part of distcheck will
not call any test suite: 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'.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v3:
- gitignore 'make check' generated logs and reports

Changes since v2:
- Drop RUN_FULL_TESTSUITE env var, it is not needed anymore

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'
---
 .gitignore   | 13 +++++++++++++
 Makefile.am  |  9 +++++++++
 configure.ac |  5 +++++
 3 files changed, 27 insertions(+)

diff --git a/.gitignore b/.gitignore
index 1e3bc5146b2f1..db329eafa5298 100644
--- a/.gitignore
+++ b/.gitignore
@@ -23,6 +23,19 @@ nftversion.h
 *.payload.got
 tests/build/tests.log
 
+# make check results
+/test-suite.log
+/tests/build/run-tests.sh.log
+/tests/build/run-tests.sh.trs
+/tests/json_echo/run-test.py.log
+/tests/json_echo/run-test.py.trs
+/tests/monitor/run-tests.sh.log
+/tests/monitor/run-tests.sh.trs
+/tests/py/nft-test.py.log
+/tests/py/nft-test.py.trs
+/tests/shell/run-tests.sh.log
+/tests/shell/run-tests.sh.trs
+
 # Debian package build temporary files
 build-stamp
 
diff --git a/Makefile.am b/Makefile.am
index 5190a49ae69f1..9112faa2d5c04 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,11 @@ 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 >$@
+
+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 v4 2/8] tests: py: Enable JSON and JSON schema by default

[Pablo Neira Ayuso]

On Thu, Sep 04, 2025 at 05:24:48PM +0200, Phil Sutter wrote:
> Introduce -J/--disable-json and -S/--no-schema to explicitly disable
> them if desired.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  tests/py/nft-test.py | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
> index 984f2b937a077..12c6174b01257 100755
> --- a/tests/py/nft-test.py
> +++ b/tests/py/nft-test.py
> @@ -1488,7 +1488,11 @@ def set_delete_elements(set_element, set_name, table, filename=None,
>  
>      parser.add_argument('-j', '--enable-json', action='store_true',
>                          dest='enable_json',
> -                        help='test JSON functionality as well')
> +                        help='test JSON functionality as well (default)')
> +
> +    parser.add_argument('-J', '--disable-json', action='store_true',
> +                        dest='disable_json',
> +                        help='Do not test JSON functionality as well')

Would it be possible to have common options to the different tests?

1/8 uses -s and -j.

I am not sure we have to worry about breaking backward for test
syntax, we only run this.

Thanks.

Re: [nft PATCH v4 2/8] tests: py: Enable JSON and JSON schema by default

[Phil Sutter]

On Thu, Sep 04, 2025 at 05:29:54PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Sep 04, 2025 at 05:24:48PM +0200, Phil Sutter wrote:
> > Introduce -J/--disable-json and -S/--no-schema to explicitly disable
> > them if desired.
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  tests/py/nft-test.py | 16 ++++++++++++----
> >  1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
> > index 984f2b937a077..12c6174b01257 100755
> > --- a/tests/py/nft-test.py
> > +++ b/tests/py/nft-test.py
> > @@ -1488,7 +1488,11 @@ def set_delete_elements(set_element, set_name, table, filename=None,
> >  
> >      parser.add_argument('-j', '--enable-json', action='store_true',
> >                          dest='enable_json',
> > -                        help='test JSON functionality as well')
> > +                        help='test JSON functionality as well (default)')
> > +
> > +    parser.add_argument('-J', '--disable-json', action='store_true',
> > +                        dest='disable_json',
> > +                        help='Do not test JSON functionality as well')
> 
> Would it be possible to have common options to the different tests?
> 
> 1/8 uses -s and -j.
> 
> I am not sure we have to worry about breaking backward for test
> syntax, we only run this.

It's a bit of a mess with nft-test.py as it always performs standard
syntax testing and JSON syntax is an add-on one may enable (or not). So
to test JSON only, I'd have to refactor the ~300 lines long rule_add()
function. Not the worst thing to do, but much more work than "just"
having --enable-json being the default.

Cheers, Phil

Re: [nft PATCH v4 2/8] tests: py: Enable JSON and JSON schema by default

[Pablo Neira Ayuso]

On Thu, Sep 04, 2025 at 06:13:06PM +0200, Phil Sutter wrote:
> On Thu, Sep 04, 2025 at 05:29:54PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Sep 04, 2025 at 05:24:48PM +0200, Phil Sutter wrote:
> > > Introduce -J/--disable-json and -S/--no-schema to explicitly disable
> > > them if desired.
> > > 
> > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > ---
> > >  tests/py/nft-test.py | 16 ++++++++++++----
> > >  1 file changed, 12 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
> > > index 984f2b937a077..12c6174b01257 100755
> > > --- a/tests/py/nft-test.py
> > > +++ b/tests/py/nft-test.py
> > > @@ -1488,7 +1488,11 @@ def set_delete_elements(set_element, set_name, table, filename=None,
> > >  
> > >      parser.add_argument('-j', '--enable-json', action='store_true',
> > >                          dest='enable_json',
> > > -                        help='test JSON functionality as well')
> > > +                        help='test JSON functionality as well (default)')
> > > +
> > > +    parser.add_argument('-J', '--disable-json', action='store_true',
> > > +                        dest='disable_json',
> > > +                        help='Do not test JSON functionality as well')
> > 
> > Would it be possible to have common options to the different tests?
> > 
> > 1/8 uses -s and -j.
> > 
> > I am not sure we have to worry about breaking backward for test
> > syntax, we only run this.
> 
> It's a bit of a mess with nft-test.py as it always performs standard
> syntax testing and JSON syntax is an add-on one may enable (or not). So
> to test JSON only, I'd have to refactor the ~300 lines long rule_add()
> function. Not the worst thing to do, but much more work than "just"
> having --enable-json being the default.

Oh, I see, so this is:

* no -j, then only standard is tested.
* -j, both standard and json are tested.

Maybe more simple is to reverse this logic, ie.

* no -j, then both standard and json syntax are tested.
* -s, only standard is tested.

Does this help?

Re: [nft PATCH v4 2/8] tests: py: Enable JSON and JSON schema by default

[Phil Sutter]

On Thu, Sep 04, 2025 at 06:56:08PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Sep 04, 2025 at 06:13:06PM +0200, Phil Sutter wrote:
> > On Thu, Sep 04, 2025 at 05:29:54PM +0200, Pablo Neira Ayuso wrote:
> > > On Thu, Sep 04, 2025 at 05:24:48PM +0200, Phil Sutter wrote:
> > > > Introduce -J/--disable-json and -S/--no-schema to explicitly disable
> > > > them if desired.
> > > > 
> > > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > > ---
> > > >  tests/py/nft-test.py | 16 ++++++++++++----
> > > >  1 file changed, 12 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
> > > > index 984f2b937a077..12c6174b01257 100755
> > > > --- a/tests/py/nft-test.py
> > > > +++ b/tests/py/nft-test.py
> > > > @@ -1488,7 +1488,11 @@ def set_delete_elements(set_element, set_name, table, filename=None,
> > > >  
> > > >      parser.add_argument('-j', '--enable-json', action='store_true',
> > > >                          dest='enable_json',
> > > > -                        help='test JSON functionality as well')
> > > > +                        help='test JSON functionality as well (default)')
> > > > +
> > > > +    parser.add_argument('-J', '--disable-json', action='store_true',
> > > > +                        dest='disable_json',
> > > > +                        help='Do not test JSON functionality as well')
> > > 
> > > Would it be possible to have common options to the different tests?
> > > 
> > > 1/8 uses -s and -j.
> > > 
> > > I am not sure we have to worry about breaking backward for test
> > > syntax, we only run this.
> > 
> > It's a bit of a mess with nft-test.py as it always performs standard
> > syntax testing and JSON syntax is an add-on one may enable (or not). So
> > to test JSON only, I'd have to refactor the ~300 lines long rule_add()
> > function. Not the worst thing to do, but much more work than "just"
> > having --enable-json being the default.
> 
> Oh, I see, so this is:
> 
> * no -j, then only standard is tested.
> * -j, both standard and json are tested.
> 
> Maybe more simple is to reverse this logic, ie.
> 
> * no -j, then both standard and json syntax are tested.
> * -s, only standard is tested.

So basically rename -J to -s in my patch? ;)
Note that -s clashes with --schema (although one could drop it as my
patch enables it by default, as well).

> Does this help?

Let me suggest an alternative: I promise to refactor that famous italian
pasta style function into smaller chunks like:

- Gather test data from files
- Instantiate a rule object with methods:
  - apply()
  - json_apply()
  - validate_payload()
  - validate_listing()
- Perform checks as per user choice (-j, -s, both/none)

And we live with the inconsistent UI for now?

Cheers, Phil

Re: [nft PATCH v4 0/8] Run all test suites via 'make check'

[Phil Sutter]

On Thu, Sep 04, 2025 at 05:24:46PM +0200, Phil Sutter wrote:
> Help me (and maybe others) to not occasionally forget to run this or
> that test suite in this or that mode:
> 
> Have test suites execute all variants by default (patches 1 and 2), make
> sure their exit codes match Automake expectations (patch 3) and register
> them with Automake (patch 8). Also fix for running 'make check' as
> non-root (patches 4 and 5) and calling build test suite from outside its
> directory (patch 6).
> 
> There is a "funny" problem with build test suite calling 'make
> distcheck' which behaves differently under the environment polluted by
> the calling 'make check' invocation, details in patch 7.
> 
> Changes since v3:
> - Applied the initial monitor test suite enhancements already
> - gitignore generated logs and reports
> - New patch 7
> 
> Changes since v2:
> - Drop the need for RUN_FULL_TESTSUITE env var by making the "all
>   variants" mode the default in all test suites
> - Implement JSON echo testing into monitor test suite, stored JSON
>   output matches echo output after minor adjustment
> 
> 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 (8):
>   tests: monitor: Excercise all syntaxes and variants by default
>   tests: py: Enable JSON and JSON schema by default
>   tests: Prepare exit codes for automake
>   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
>   tests: build: Avoid a recursive 'make check' run
>   Makefile: Enable support for 'make check'

Series applied. I will follow up with the promised refactoring of
rule_add() function to finally align cmdline options with other test
suite runners.