[nft PATCH v3 00/11] Run all test suites via 'make check'

[nft PATCH v3 00/11] 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 5 and 6),
make sure their exit codes
match Automake expectations (patch 7) and register them with Automake
(patch 11). Also fix for running 'make check' as non-root (patches 8 and
9) and calling build test suite from outside its directory (patch 10).

The first four patches are fallout from enabling all variants by default
in monitor test suite, which includes implementing previously missing
JSON echo testing.

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 (11):
  tests: monitor: Label diffs to help users
  tests: monitor: Fix regex collecting expected echo output
  tests: monitor: Test JSON echo mode as well
  tests: monitor: Extend debug output a bit
  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
  Makefile: Enable support for 'make check'

 Makefile.am                              |   9 ++
 configure.ac                             |   5 +
 tests/build/run-tests.sh                 |   2 +
 tests/json_echo/run-test.py              |   4 +
 tests/monitor/run-tests.sh               | 182 ++++++++++++++---------
 tests/py/nft-test.py                     |  28 ++--
 tests/shell/run-tests.sh                 |   2 +-
 tests/shell/testcases/packetpath/nat_ftp |   3 +
 8 files changed, 155 insertions(+), 80 deletions(-)

-- 
2.51.0

[nft PATCH v3 01/11] tests: monitor: Label diffs to help users

[Phil Sutter]

Clarify what was expected and what was actually received.

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

diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
index 67d3e618cee07..03091d1745212 100755
--- a/tests/monitor/run-tests.sh
+++ b/tests/monitor/run-tests.sh
@@ -5,7 +5,7 @@ debug=false
 test_json=false
 
 mydiff() {
-	diff -w -I '^# ' "$@"
+	diff -w -I '^# ' --label "expected" --label "got" "$@"
 }
 
 err() {
-- 
2.51.0

[nft PATCH v3 02/11] tests: monitor: Fix regex collecting expected echo output

[Phil Sutter]

No input triggered this bug, but the match would accept "insert" and
"replace" keywords anywhere in the line not just at the beginning as was
intended.

Fixes: b2506e5504fed ("tests: Merge monitor and echo test suites")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/monitor/run-tests.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
index 03091d1745212..4cbdee587f47c 100755
--- a/tests/monitor/run-tests.sh
+++ b/tests/monitor/run-tests.sh
@@ -52,7 +52,7 @@ echo_output_append() {
 		grep '^\(add\|replace\|insert\)' $command_file >>$output_file
 		return
 	}
-	[[ "$*" =~ ^add|replace|insert ]] && echo "$*" >>$output_file
+	[[ "$*" =~ ^(add|replace|insert) ]] && echo "$*" >>$output_file
 }
 json_output_filter() { # (filename)
 	# unify handle values
-- 
2.51.0

[nft PATCH v3 03/11] tests: monitor: Test JSON echo mode as well

[Phil Sutter]

Reuse the expected JSON monitor output for --echo testing as it is
supposed to be "identical" - apart from formatting differences. To match
lines of commands (monitor output) against a single line of JSON object
(echo output), join the former's lines and drop the surrounding object
in the latter since this seems to be the simplest way.

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

diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
index 4cbdee587f47c..38c20adb1dc61 100755
--- a/tests/monitor/run-tests.sh
+++ b/tests/monitor/run-tests.sh
@@ -52,7 +52,7 @@ echo_output_append() {
 		grep '^\(add\|replace\|insert\)' $command_file >>$output_file
 		return
 	}
-	[[ "$*" =~ ^(add|replace|insert) ]] && echo "$*" >>$output_file
+	[[ "$*" =~ ^(\{\")?(add|replace|insert) ]] && echo "$*" >>$output_file
 }
 json_output_filter() { # (filename)
 	# unify handle values
@@ -96,16 +96,32 @@ monitor_run_test() {
 
 echo_run_test() {
 	echo_output=$(mktemp -p $testdir)
+	echo_args="-nn -e"
+	$test_json && echo_args+=" -j"
 	local rc=0
 
 	$debug && {
 		echo "command file:"
 		cat $command_file
 	}
-	$nft -nn -e -f - <$command_file >$echo_output || {
+	$nft $echo_args -f - <$command_file >$echo_output || {
 		err "nft command failed!"
 		rc=1
 	}
+	if $test_json; then
+		# Extract commands from the surrounding JSON object
+		sed -i -e 's/^{"nftables": \[//' -e 's/\]}$//' $echo_output
+		json_output_filter $echo_output
+
+		# Replace newlines by ", " in output file
+		readarray -t output_file_lines <$output_file
+		sep=""
+		for ((i = 0; i < ${#output_file_lines[*]}; i++)); do
+			printf "${sep}${output_file_lines[$i]}"
+			sep=", "
+		done >$output_file
+		[ $i -gt 0 ] && echo "" >>$output_file
+	fi
 	mydiff -q $echo_output $output_file >/dev/null 2>&1
 	if [[ $rc == 0 && $? != 0 ]]; then
 		err "echo output differs!"
@@ -159,12 +175,7 @@ while [ -n "$1" ]; do
 	esac
 done
 
-if $test_json; then
-	variants="monitor"
-else
-	variants="monitor echo"
-fi
-
+variants="monitor echo"
 rc=0
 for variant in $variants; do
 	run_test=${variant}_run_test
-- 
2.51.0

[nft PATCH v3 04/11] tests: monitor: Extend debug output a bit

[Phil Sutter]

Dump echo output and output file, surrounded by markers to highlight
empty files and extra newlines.

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

diff --git a/tests/monitor/run-tests.sh b/tests/monitor/run-tests.sh
index 38c20adb1dc61..b09b72ae034cb 100755
--- a/tests/monitor/run-tests.sh
+++ b/tests/monitor/run-tests.sh
@@ -101,8 +101,9 @@ echo_run_test() {
 	local rc=0
 
 	$debug && {
-		echo "command file:"
+		echo ">>> command file"
 		cat $command_file
+		echo "<<< command file"
 	}
 	$nft $echo_args -f - <$command_file >$echo_output || {
 		err "nft command failed!"
@@ -122,6 +123,15 @@ echo_run_test() {
 		done >$output_file
 		[ $i -gt 0 ] && echo "" >>$output_file
 	fi
+	$debug && {
+		echo ">>> output file"
+		cat $output_file
+		echo "<<< output file"
+		echo ">>> echo output"
+		cat $echo_output
+		echo "<<< echo output"
+	}
+
 	mydiff -q $echo_output $output_file >/dev/null 2>&1
 	if [[ $rc == 0 && $? != 0 ]]; then
 		err "echo output differs!"
-- 
2.51.0

[nft PATCH v3 05/11] 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 v3 06/11] 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 v3 07/11] 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 v3 08/11] 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 v3 09/11] 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 v3 10/11] 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 v3 11/11] 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 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'
---
 Makefile.am  | 9 +++++++++
 configure.ac | 5 +++++
 2 files changed, 14 insertions(+)

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 v3 00/11] Run all test suites via 'make check'

[Phil Sutter]

On Wed, Sep 03, 2025 at 07:22:48PM +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 5 and 6),
> make sure their exit codes
> match Automake expectations (patch 7) and register them with Automake
> (patch 11). Also fix for running 'make check' as non-root (patches 8 and
> 9) and calling build test suite from outside its directory (patch 10).
> 
> The first four patches are fallout from enabling all variants by default
> in monitor test suite, which includes implementing previously missing
> JSON echo testing.
> 
> 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 (11):
>   tests: monitor: Label diffs to help users
>   tests: monitor: Fix regex collecting expected echo output
>   tests: monitor: Test JSON echo mode as well
>   tests: monitor: Extend debug output a bit

Applied these four patches as they are pretty harmless and not related
to the remaining series.

Cheers, Phil

Re: [nft PATCH v3 06/11] tests: py: Enable JSON and JSON schema by default

[Pablo Neira Ayuso]

On Wed, Sep 03, 2025 at 07:22:54PM +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')

Maybe -s for standard syntax only and -j for json only.

Otherwise, default to run them all.

Re: [nft PATCH v3 11/11] Makefile: Enable support for 'make check'

[Pablo Neira Ayuso]

On Wed, Sep 03, 2025 at 07:22:59PM +0200, Phil Sutter wrote:
> 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 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'
> ---
>  Makefile.am  | 9 +++++++++
>  configure.ac | 5 +++++
>  2 files changed, 14 insertions(+)
> 
> 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"])

Oh no, with distcheck-hook: this is a lot cleaner.

Please revert this.

Re: [nft PATCH v3 11/11] Makefile: Enable support for 'make check'

[Pablo Neira Ayuso]

On Thu, Sep 04, 2025 at 05:25:02PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Sep 03, 2025 at 07:22:59PM +0200, Phil Sutter wrote:
> > 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 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'
> > ---
> >  Makefile.am  | 9 +++++++++
> >  configure.ac | 5 +++++
> >  2 files changed, 14 insertions(+)
> > 
> > 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"])
> 
> Oh no, with distcheck-hook: this is a lot cleaner.
> 
> Please revert this.

Sorry I misunderstood.

You only applied four patches.

My apologies.

Re: [nft PATCH v3 11/11] Makefile: Enable support for 'make check'

[Phil Sutter]

On Thu, Sep 04, 2025 at 05:25:54PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Sep 04, 2025 at 05:25:02PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Sep 03, 2025 at 07:22:59PM +0200, Phil Sutter wrote:
> > > 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 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'
> > > ---
> > >  Makefile.am  | 9 +++++++++
> > >  configure.ac | 5 +++++
> > >  2 files changed, 14 insertions(+)
> > > 
> > > 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"])
> > 
> > Oh no, with distcheck-hook: this is a lot cleaner.
> > 
> > Please revert this.
> 
> Sorry I misunderstood.
> 
> You only applied four patches.

Yes, sorry. I started collecting unrelated fallout again to avoid the
added dependency between two patch series. This is really a bad habit as
it makes reviews harder.

> My apologies.

No need to, thanks for bearing with my mess. I'll check what I can
achieve with distcheck-hook, thanks for the pointer!

Cheers, Phil

Re: [nft PATCH v3 11/11] Makefile: Enable support for 'make check'

[Phil Sutter]

On Thu, Sep 04, 2025 at 05:24:59PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Sep 03, 2025 at 07:22:59PM +0200, Phil Sutter wrote:
[...]
> > 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"])
> 
> Oh no, with distcheck-hook: this is a lot cleaner.

Hmm, I really don't see how it could be used for this purpose: It is
called before starting the VPATH build, here's an excerpt:

| mkdir nftables-1.1.5/_build nftables-1.1.5/_build/sub nftables-1.1.5/_inst
| chmod a-w nftables-1.1.5
| test -d nftables-1.1.5/_build || exit 0; \
| dc_install_base=`CDPATH="${ZSH_VERSION+.}:" && cd nftables-1.1.5/_inst && pwd | sed -e 's,^[^:\\/]:[\\/],/,'` \
|   && dc_destdir="${TMPDIR-/tmp}/am-dc-$$/" \
|   && make  distcheck-hook \
|   && am__cwd=`pwd` \
|   && CDPATH="${ZSH_VERSION+.}:" && cd nftables-1.1.5/_build/sub \
|   && ../../configure \
|       --enable-distcheck \
|          \
|       --srcdir=../.. --prefix="$dc_install_base" \
|   && make  \
|   && make  dvi \
|   && make  check \

So by the time distcheck-hook runs, there is no Makefile(.in) I could
modify, only the top-level ones (which are tracked in git). What am I
missing here?

Cheers, Phil

Re: [nft PATCH v3 11/11] Makefile: Enable support for 'make check'

[Pablo Neira Ayuso]

On Thu, Sep 04, 2025 at 06:21:59PM +0200, Phil Sutter wrote:
> On Thu, Sep 04, 2025 at 05:24:59PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Sep 03, 2025 at 07:22:59PM +0200, Phil Sutter wrote:
> [...]
> > > 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"])
> > 
> > Oh no, with distcheck-hook: this is a lot cleaner.
> 
> Hmm, I really don't see how it could be used for this purpose: It is
> called before starting the VPATH build, here's an excerpt:
> 
> | mkdir nftables-1.1.5/_build nftables-1.1.5/_build/sub nftables-1.1.5/_inst
> | chmod a-w nftables-1.1.5
> | test -d nftables-1.1.5/_build || exit 0; \
> | dc_install_base=`CDPATH="${ZSH_VERSION+.}:" && cd nftables-1.1.5/_inst && pwd | sed -e 's,^[^:\\/]:[\\/],/,'` \
> |   && dc_destdir="${TMPDIR-/tmp}/am-dc-$$/" \
> |   && make  distcheck-hook \
> |   && am__cwd=`pwd` \
> |   && CDPATH="${ZSH_VERSION+.}:" && cd nftables-1.1.5/_build/sub \
> |   && ../../configure \
> |       --enable-distcheck \
> |          \
> |       --srcdir=../.. --prefix="$dc_install_base" \
> |   && make  \
> |   && make  dvi \
> |   && make  check \
> 
> So by the time distcheck-hook runs, there is no Makefile(.in) I could
> modify, only the top-level ones (which are tracked in git). What am I
> missing here?

distcheck-hook: could set a env var so test just print a [SKIP].

Similar to your previous approach with the env var, but logic reversed.

Re: [nft PATCH v3 11/11] Makefile: Enable support for 'make check'

[Phil Sutter]

On Thu, Sep 04, 2025 at 06:57:52PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Sep 04, 2025 at 06:21:59PM +0200, Phil Sutter wrote:
> > On Thu, Sep 04, 2025 at 05:24:59PM +0200, Pablo Neira Ayuso wrote:
> > > On Wed, Sep 03, 2025 at 07:22:59PM +0200, Phil Sutter wrote:
> > [...]
> > > > 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"])
> > > 
> > > Oh no, with distcheck-hook: this is a lot cleaner.
> > 
> > Hmm, I really don't see how it could be used for this purpose: It is
> > called before starting the VPATH build, here's an excerpt:
> > 
> > | mkdir nftables-1.1.5/_build nftables-1.1.5/_build/sub nftables-1.1.5/_inst
> > | chmod a-w nftables-1.1.5
> > | test -d nftables-1.1.5/_build || exit 0; \
> > | dc_install_base=`CDPATH="${ZSH_VERSION+.}:" && cd nftables-1.1.5/_inst && pwd | sed -e 's,^[^:\\/]:[\\/],/,'` \
> > |   && dc_destdir="${TMPDIR-/tmp}/am-dc-$$/" \
> > |   && make  distcheck-hook \
> > |   && am__cwd=`pwd` \
> > |   && CDPATH="${ZSH_VERSION+.}:" && cd nftables-1.1.5/_build/sub \
> > |   && ../../configure \
> > |       --enable-distcheck \
> > |          \
> > |       --srcdir=../.. --prefix="$dc_install_base" \
> > |   && make  \
> > |   && make  dvi \
> > |   && make  check \
> > 
> > So by the time distcheck-hook runs, there is no Makefile(.in) I could
> > modify, only the top-level ones (which are tracked in git). What am I
> > missing here?
> 
> distcheck-hook: could set a env var so test just print a [SKIP].
> 
> Similar to your previous approach with the env var, but logic reversed.

I don't think the 'make distcheck-hook' call is able to inject variables
into the following 'make check' call's environment. It could create a
special file though which all test suites recognize and exit 77
immediately.

Cheers, Phil

Re: [nft PATCH v3 11/11] Makefile: Enable support for 'make check'

[Phil Sutter]

Hi Pablo,

On Thu, Sep 04, 2025 at 08:39:40PM +0200, Phil Sutter wrote:
> On Thu, Sep 04, 2025 at 06:57:52PM +0200, Pablo Neira Ayuso wrote:
[...]
> > distcheck-hook: could set a env var so test just print a [SKIP].
> > 
> > Similar to your previous approach with the env var, but logic reversed.
> 
> I don't think the 'make distcheck-hook' call is able to inject variables
> into the following 'make check' call's environment. It could create a
> special file though which all test suites recognize and exit 77
> immediately.

What are your thoughts about this? IMO, the special configure option is
much cleaner than creating a special file and patching all test runners
to check for it. It should be doable though, so if you prefer this I
don't mind.

Thanks, Phil

Re: [nft PATCH v3 11/11] Makefile: Enable support for 'make check'

[Pablo Neira Ayuso]

On Thu, Sep 11, 2025 at 02:02:37PM +0200, Phil Sutter wrote:
> Hi Pablo,
> 
> On Thu, Sep 04, 2025 at 08:39:40PM +0200, Phil Sutter wrote:
> > On Thu, Sep 04, 2025 at 06:57:52PM +0200, Pablo Neira Ayuso wrote:
> [...]
> > > distcheck-hook: could set a env var so test just print a [SKIP].
> > > 
> > > Similar to your previous approach with the env var, but logic reversed.
> > 
> > I don't think the 'make distcheck-hook' call is able to inject variables
> > into the following 'make check' call's environment. It could create a
> > special file though which all test suites recognize and exit 77
> > immediately.
> 
> What are your thoughts about this? IMO, the special configure option is
> much cleaner than creating a special file and patching all test runners
> to check for it.

Yes, makes sense, thanks.