netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [nft PATCH 0/6] Run all test suites via 'make check'
@ 2025-08-01 16:10 Phil Sutter
  2025-08-01 16:11 ` [nft PATCH 1/6] tests: Prepare exit codes for automake Phil Sutter
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Phil Sutter @ 2025-08-01 16:10 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

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 they're exit codes
match Automake expectations (patch 1) and register them with Automake
(patch 6). Also fix for running 'make check' as non-root (patches 4 and
5).

Phil Sutter (6):
  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
  Makefile: Enable support for 'make check'

 Makefile.am                              |  6 +++++
 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 +++
 6 files changed, 48 insertions(+), 20 deletions(-)

-- 
2.49.0


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

* [nft PATCH 1/6] tests: Prepare exit codes for automake
  2025-08-01 16:10 [nft PATCH 0/6] Run all test suites via 'make check' Phil Sutter
@ 2025-08-01 16:11 ` Phil Sutter
  2025-08-01 16:11 ` [nft PATCH 2/6] tests: monitor: Support running all tests in one go Phil Sutter
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Phil Sutter @ 2025-08-01 16:11 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

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.49.0


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

* [nft PATCH 2/6] tests: monitor: Support running all tests in one go
  2025-08-01 16:10 [nft PATCH 0/6] Run all test suites via 'make check' Phil Sutter
  2025-08-01 16:11 ` [nft PATCH 1/6] tests: Prepare exit codes for automake Phil Sutter
@ 2025-08-01 16:11 ` Phil Sutter
  2025-08-01 16:11 ` [nft PATCH 3/6] tests: py: Set default options based on RUN_FULL_TESTSUITE Phil Sutter
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Phil Sutter @ 2025-08-01 16:11 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

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.49.0


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

* [nft PATCH 3/6] tests: py: Set default options based on RUN_FULL_TESTSUITE
  2025-08-01 16:10 [nft PATCH 0/6] Run all test suites via 'make check' Phil Sutter
  2025-08-01 16:11 ` [nft PATCH 1/6] tests: Prepare exit codes for automake Phil Sutter
  2025-08-01 16:11 ` [nft PATCH 2/6] tests: monitor: Support running all tests in one go Phil Sutter
@ 2025-08-01 16:11 ` Phil Sutter
  2025-08-01 16:11 ` [nft PATCH 4/6] tests: json_echo: Skip if run as non-root Phil Sutter
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Phil Sutter @ 2025-08-01 16:11 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

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.49.0


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

* [nft PATCH 4/6] tests: json_echo: Skip if run as non-root
  2025-08-01 16:10 [nft PATCH 0/6] Run all test suites via 'make check' Phil Sutter
                   ` (2 preceding siblings ...)
  2025-08-01 16:11 ` [nft PATCH 3/6] tests: py: Set default options based on RUN_FULL_TESTSUITE Phil Sutter
@ 2025-08-01 16:11 ` Phil Sutter
  2025-08-01 16:11 ` [nft PATCH 5/6] tests: shell: Skip packetpath/nat_ftp in fake root env Phil Sutter
  2025-08-01 16:11 ` [nft PATCH 6/6] Makefile: Enable support for 'make check' Phil Sutter
  5 siblings, 0 replies; 13+ messages in thread
From: Phil Sutter @ 2025-08-01 16:11 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

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.49.0


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

* [nft PATCH 5/6] tests: shell: Skip packetpath/nat_ftp in fake root env
  2025-08-01 16:10 [nft PATCH 0/6] Run all test suites via 'make check' Phil Sutter
                   ` (3 preceding siblings ...)
  2025-08-01 16:11 ` [nft PATCH 4/6] tests: json_echo: Skip if run as non-root Phil Sutter
@ 2025-08-01 16:11 ` Phil Sutter
  2025-08-01 16:11 ` [nft PATCH 6/6] Makefile: Enable support for 'make check' Phil Sutter
  5 siblings, 0 replies; 13+ messages in thread
From: Phil Sutter @ 2025-08-01 16:11 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

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.49.0


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

* [nft PATCH 6/6] Makefile: Enable support for 'make check'
  2025-08-01 16:10 [nft PATCH 0/6] Run all test suites via 'make check' Phil Sutter
                   ` (4 preceding siblings ...)
  2025-08-01 16:11 ` [nft PATCH 5/6] tests: shell: Skip packetpath/nat_ftp in fake root env Phil Sutter
@ 2025-08-01 16:11 ` Phil Sutter
  2025-08-06 17:05   ` Pablo Neira Ayuso
  5 siblings, 1 reply; 13+ messages in thread
From: Phil Sutter @ 2025-08-01 16:11 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

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

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 Makefile.am | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index ba09e7f0953d5..4fb75b85a5d59 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -409,5 +409,11 @@ EXTRA_DIST += \
 	tests \
 	$(NULL)
 
+AM_TESTS_ENVIRONMENT = RUN_FULL_TESTSUITE=1; export RUN_FULL_TESTSUITE;
+TESTS = tests/json_echo/run-test.py \
+	tests/monitor/run-tests.sh \
+	tests/py/nft-test.py \
+	tests/shell/run-tests.sh
+
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = libnftables.pc
-- 
2.49.0


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

* Re: [nft PATCH 6/6] Makefile: Enable support for 'make check'
  2025-08-01 16:11 ` [nft PATCH 6/6] Makefile: Enable support for 'make check' Phil Sutter
@ 2025-08-06 17:05   ` Pablo Neira Ayuso
  2025-08-07 11:51     ` Phil Sutter
  0 siblings, 1 reply; 13+ messages in thread
From: Pablo Neira Ayuso @ 2025-08-06 17:05 UTC (permalink / raw)
  To: Phil Sutter; +Cc: netfilter-devel

On Fri, Aug 01, 2025 at 06:11:05PM +0200, Phil Sutter wrote:
> Add the various testsuite runners to TESTS variable and have make call
> them with RUN_FULL_TESTSUITE=1 env var.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  Makefile.am | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Makefile.am b/Makefile.am
> index ba09e7f0953d5..4fb75b85a5d59 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -409,5 +409,11 @@ EXTRA_DIST += \
>  	tests \
>  	$(NULL)
>  
> +AM_TESTS_ENVIRONMENT = RUN_FULL_TESTSUITE=1; export RUN_FULL_TESTSUITE;

I use make distcheck to build the tarballs.

I would prefer not to run the tests at the time of the release
process, I always do this before release, but I prefer not to inline
this to the release process.

Maybe we can make this work this way?

  export RUN_FULL_TESTSUITE=1; make check

so make check is no-op without this variable?

Does this make sense to you?

> +TESTS = tests/json_echo/run-test.py \
> +	tests/monitor/run-tests.sh \
> +	tests/py/nft-test.py \
> +	tests/shell/run-tests.sh

BTW, there are also tests/build/ that are slow but useful, that helped
me find this:

https://git.netfilter.org/nftables/commit/?id=0584f1c1c2073ff082badc7b49ed667de41002d9

Thanks.

>  pkgconfigdir = $(libdir)/pkgconfig
>  pkgconfig_DATA = libnftables.pc
> -- 
> 2.49.0
> 

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

* Re: [nft PATCH 6/6] Makefile: Enable support for 'make check'
  2025-08-06 17:05   ` Pablo Neira Ayuso
@ 2025-08-07 11:51     ` Phil Sutter
  2025-08-07 12:14       ` Pablo Neira Ayuso
  0 siblings, 1 reply; 13+ messages in thread
From: Phil Sutter @ 2025-08-07 11:51 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On Wed, Aug 06, 2025 at 07:05:02PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Aug 01, 2025 at 06:11:05PM +0200, Phil Sutter wrote:
> > Add the various testsuite runners to TESTS variable and have make call
> > them with RUN_FULL_TESTSUITE=1 env var.
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  Makefile.am | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/Makefile.am b/Makefile.am
> > index ba09e7f0953d5..4fb75b85a5d59 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -409,5 +409,11 @@ EXTRA_DIST += \
> >  	tests \
> >  	$(NULL)
> >  
> > +AM_TESTS_ENVIRONMENT = RUN_FULL_TESTSUITE=1; export RUN_FULL_TESTSUITE;
> 
> I use make distcheck to build the tarballs.
> 
> I would prefer not to run the tests at the time of the release
> process, I always do this before release, but I prefer not to inline
> this to the release process.

Oh, good to know. Running just 'make dist' is no option for you?

BTW: There is the same situation with iptables, though if called as
unprivileged user there is only the xlate test suite which runs (and
quickly finishes).

> Maybe we can make this work this way?
> 
>   export RUN_FULL_TESTSUITE=1; make check
> 
> so make check is no-op without this variable?
> 
> Does this make sense to you?

It seems odd to enable 'make check' only to disable it again, but
there's still added value in it.

I'm currently looking into distcheck-hook and DISTCHECK_CONFIGURE_FLAGS
in order to identify the caller to 'make check' call.

An alternative would be to drop fake root functionality from shell
test suite, then it would skip just like all the other test suites if run
as non-root (assuming you don't run 'make distcheck' as root).

Another side-quest extracted from this mail: There's an odd failure from
shell test suite when run by 'make distcheck':

| E: cannot execute nft command: ../../tests/shell/../../src/nft
| ERROR tests/shell/run-tests.sh (exit status: 99)

> > +TESTS = tests/json_echo/run-test.py \
> > +	tests/monitor/run-tests.sh \
> > +	tests/py/nft-test.py \
> > +	tests/shell/run-tests.sh
> 
> BTW, there are also tests/build/ that are slow but useful, that helped
> me find this:
> 
> https://git.netfilter.org/nftables/commit/?id=0584f1c1c2073ff082badc7b49ed667de41002d9

Ah, I just "discovered" that bug as well, it surfaces from the build
performed by 'make distcheck', too.

Thanks, Phil

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

* Re: [nft PATCH 6/6] Makefile: Enable support for 'make check'
  2025-08-07 11:51     ` Phil Sutter
@ 2025-08-07 12:14       ` Pablo Neira Ayuso
  2025-08-07 13:04         ` Phil Sutter
  0 siblings, 1 reply; 13+ messages in thread
From: Pablo Neira Ayuso @ 2025-08-07 12:14 UTC (permalink / raw)
  To: Phil Sutter, netfilter-devel

On Thu, Aug 07, 2025 at 01:51:09PM +0200, Phil Sutter wrote:
> On Wed, Aug 06, 2025 at 07:05:02PM +0200, Pablo Neira Ayuso wrote:
> > On Fri, Aug 01, 2025 at 06:11:05PM +0200, Phil Sutter wrote:
> > > Add the various testsuite runners to TESTS variable and have make call
> > > them with RUN_FULL_TESTSUITE=1 env var.
> > > 
> > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > ---
> > >  Makefile.am | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/Makefile.am b/Makefile.am
> > > index ba09e7f0953d5..4fb75b85a5d59 100644
> > > --- a/Makefile.am
> > > +++ b/Makefile.am
> > > @@ -409,5 +409,11 @@ EXTRA_DIST += \
> > >  	tests \
> > >  	$(NULL)
> > >  
> > > +AM_TESTS_ENVIRONMENT = RUN_FULL_TESTSUITE=1; export RUN_FULL_TESTSUITE;
> > 
> > I use make distcheck to build the tarballs.
> > 
> > I would prefer not to run the tests at the time of the release
> > process, I always do this before release, but I prefer not to inline
> > this to the release process.
> 
> Oh, good to know. Running just 'make dist' is no option for you?

I can just modify the script to do so, no idea on the implications.

Or wait for one more hour for the test run to finish during the
release process.

> BTW: There is the same situation with iptables, though if called as
> unprivileged user there is only the xlate test suite which runs (and
> quickly finishes).
> 
> > Maybe we can make this work this way?
> > 
> >   export RUN_FULL_TESTSUITE=1; make check
> > 
> > so make check is no-op without this variable?
> > 
> > Does this make sense to you?
> 
> It seems odd to enable 'make check' only to disable it again, but
> there's still added value in it.
> 
> I'm currently looking into distcheck-hook and DISTCHECK_CONFIGURE_FLAGS
> in order to identify the caller to 'make check' call.
> 
> An alternative would be to drop fake root functionality from shell
> test suite, then it would skip just like all the other test suites if run
> as non-root (assuming you don't run 'make distcheck' as root).

Looking at the current release script I have, it all runs as non-root.

Maybe simply as a run-all.sh under nftables/tests/?

It is not so elegant as make check, but still allows for running _all_
tests with minimal changes.

tests/build is slow but very useful.

> Another side-quest extracted from this mail: There's an odd failure from
> shell test suite when run by 'make distcheck':
> 
> | E: cannot execute nft command: ../../tests/shell/../../src/nft
> | ERROR tests/shell/run-tests.sh (exit status: 99)
> 
> > > +TESTS = tests/json_echo/run-test.py \
> > > +	tests/monitor/run-tests.sh \
> > > +	tests/py/nft-test.py \
> > > +	tests/shell/run-tests.sh
> > 
> > BTW, there are also tests/build/ that are slow but useful, that helped
> > me find this:
> > 
> > https://git.netfilter.org/nftables/commit/?id=0584f1c1c2073ff082badc7b49ed667de41002d9
> 
> Ah, I just "discovered" that bug as well, it surfaces from the build
> performed by 'make distcheck', too.
> 
> Thanks, Phil

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

* Re: [nft PATCH 6/6] Makefile: Enable support for 'make check'
  2025-08-07 12:14       ` Pablo Neira Ayuso
@ 2025-08-07 13:04         ` Phil Sutter
  2025-08-27 23:08           ` Pablo Neira Ayuso
  0 siblings, 1 reply; 13+ messages in thread
From: Phil Sutter @ 2025-08-07 13:04 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

On Thu, Aug 07, 2025 at 02:14:13PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Aug 07, 2025 at 01:51:09PM +0200, Phil Sutter wrote:
> > On Wed, Aug 06, 2025 at 07:05:02PM +0200, Pablo Neira Ayuso wrote:
> > > On Fri, Aug 01, 2025 at 06:11:05PM +0200, Phil Sutter wrote:
> > > > Add the various testsuite runners to TESTS variable and have make call
> > > > them with RUN_FULL_TESTSUITE=1 env var.
> > > > 
> > > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > > ---
> > > >  Makefile.am | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > > 
> > > > diff --git a/Makefile.am b/Makefile.am
> > > > index ba09e7f0953d5..4fb75b85a5d59 100644
> > > > --- a/Makefile.am
> > > > +++ b/Makefile.am
> > > > @@ -409,5 +409,11 @@ EXTRA_DIST += \
> > > >  	tests \
> > > >  	$(NULL)
> > > >  
> > > > +AM_TESTS_ENVIRONMENT = RUN_FULL_TESTSUITE=1; export RUN_FULL_TESTSUITE;
> > > 
> > > I use make distcheck to build the tarballs.
> > > 
> > > I would prefer not to run the tests at the time of the release
> > > process, I always do this before release, but I prefer not to inline
> > > this to the release process.
> > 
> > Oh, good to know. Running just 'make dist' is no option for you?
> 
> I can just modify the script to do so, no idea on the implications.

There is more to distcheck than just the 'make check' call, so it's
definitely worth doing it. The best option might be to run 'make
distcheck' before the release for a complete test run and only 'make
dist' during the release process. Though this requires to run 'make
distcheck' as root, not sure if that is a good idea.

> Or wait for one more hour for the test run to finish during the
> release process.

Does not seem feasible, especially for a redundant test run you're not
interested in. It also implies that you're creating the distribution on
a system which is able to pass (or skip) all tests, which may not be the
case.

> > BTW: There is the same situation with iptables, though if called as
> > unprivileged user there is only the xlate test suite which runs (and
> > quickly finishes).
> > 
> > > Maybe we can make this work this way?
> > > 
> > >   export RUN_FULL_TESTSUITE=1; make check
> > > 
> > > so make check is no-op without this variable?
> > > 
> > > Does this make sense to you?
> > 
> > It seems odd to enable 'make check' only to disable it again, but
> > there's still added value in it.
> > 
> > I'm currently looking into distcheck-hook and DISTCHECK_CONFIGURE_FLAGS
> > in order to identify the caller to 'make check' call.
> > 
> > An alternative would be to drop fake root functionality from shell
> > test suite, then it would skip just like all the other test suites if run
> > as non-root (assuming you don't run 'make distcheck' as root).
> 
> Looking at the current release script I have, it all runs as non-root.
> 
> Maybe simply as a run-all.sh under nftables/tests/?

Also an option, yes. Or a custom 'make testrun' or so.

> It is not so elegant as make check, but still allows for running _all_
> tests with minimal changes.

A cool feature of automake is to run tests in parallel. I have a local
branch which implements that for iptables. Makefile.am has to list every
test case though, which adds overhead (and room for errors) when writing
new tests.

> tests/build is slow but very useful.

Yes, I don't see a way to instruct 'make distcheck' to perform multiple
builds with different configure options.

tests/build is funny: It runs 'make distcheck' itself, so adding
tests/build/run-tests.sh to TESTS variable in Makefile.am should lead to
an infinite distcheck loop. :D

> > Another side-quest extracted from this mail: There's an odd failure from
> > shell test suite when run by 'make distcheck':
> > 
> > | E: cannot execute nft command: ../../tests/shell/../../src/nft
> > | ERROR tests/shell/run-tests.sh (exit status: 99)

This happens due to the separate source and build directories, the
compiled 'nft' tool (actually: wrapper) is not in the same tree as the
test runner.

Cheers, Phil

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

* Re: [nft PATCH 6/6] Makefile: Enable support for 'make check'
  2025-08-07 13:04         ` Phil Sutter
@ 2025-08-27 23:08           ` Pablo Neira Ayuso
  2025-08-28 10:38             ` Phil Sutter
  0 siblings, 1 reply; 13+ messages in thread
From: Pablo Neira Ayuso @ 2025-08-27 23:08 UTC (permalink / raw)
  To: Phil Sutter, netfilter-devel

On Thu, Aug 07, 2025 at 03:04:46PM +0200, Phil Sutter wrote:
> On Thu, Aug 07, 2025 at 02:14:13PM +0200, Pablo Neira Ayuso wrote:
> > On Thu, Aug 07, 2025 at 01:51:09PM +0200, Phil Sutter wrote:
> > > On Wed, Aug 06, 2025 at 07:05:02PM +0200, Pablo Neira Ayuso wrote:
> > > > On Fri, Aug 01, 2025 at 06:11:05PM +0200, Phil Sutter wrote:
> > > > > Add the various testsuite runners to TESTS variable and have make call
> > > > > them with RUN_FULL_TESTSUITE=1 env var.
> > > > > 
> > > > > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > > > > ---
> > > > >  Makefile.am | 6 ++++++
> > > > >  1 file changed, 6 insertions(+)
> > > > > 
> > > > > diff --git a/Makefile.am b/Makefile.am
> > > > > index ba09e7f0953d5..4fb75b85a5d59 100644
> > > > > --- a/Makefile.am
> > > > > +++ b/Makefile.am
> > > > > @@ -409,5 +409,11 @@ EXTRA_DIST += \
> > > > >  	tests \
> > > > >  	$(NULL)
> > > > >  
> > > > > +AM_TESTS_ENVIRONMENT = RUN_FULL_TESTSUITE=1; export RUN_FULL_TESTSUITE;
> > > > 
> > > > I use make distcheck to build the tarballs.
> > > > 
> > > > I would prefer not to run the tests at the time of the release
> > > > process, I always do this before release, but I prefer not to inline
> > > > this to the release process.
> > > 
> > > Oh, good to know. Running just 'make dist' is no option for you?
> > 
> > I can just modify the script to do so, no idea on the implications.
> 
> There is more to distcheck than just the 'make check' call, so it's
> definitely worth doing it. The best option might be to run 'make
> distcheck' before the release for a complete test run and only 'make
> dist' during the release process. Though this requires to run 'make
> distcheck' as root, not sure if that is a good idea.

Hm, I prefer not to run make distcheck as root.

> > Or wait for one more hour for the test run to finish during the
> > release process.
> 
> Does not seem feasible, especially for a redundant test run you're not
> interested in. It also implies that you're creating the distribution on
> a system which is able to pass (or skip) all tests, which may not be the
> case.

Yes.

> > > BTW: There is the same situation with iptables, though if called as
> > > unprivileged user there is only the xlate test suite which runs (and
> > > quickly finishes).
> > > 
> > > > Maybe we can make this work this way?
> > > > 
> > > >   export RUN_FULL_TESTSUITE=1; make check
> > > > 
> > > > so make check is no-op without this variable?
> > > > 
> > > > Does this make sense to you?
> > > 
> > > It seems odd to enable 'make check' only to disable it again, but
> > > there's still added value in it.
> > > 
> > > I'm currently looking into distcheck-hook and DISTCHECK_CONFIGURE_FLAGS
> > > in order to identify the caller to 'make check' call.
> > > 
> > > An alternative would be to drop fake root functionality from shell
> > > test suite, then it would skip just like all the other test suites if run
> > > as non-root (assuming you don't run 'make distcheck' as root).
> > 
> > Looking at the current release script I have, it all runs as non-root.
> > 
> > Maybe simply as a run-all.sh under nftables/tests/?
> 
> Also an option, yes. Or a custom 'make testrun' or so.

'make testrun' sounds nicer than my run-it-all shell script proposal,
it would be nice to have a short summary at the test run not to scroll
up to find each individual test result. And I think 'make testrun'
should continue on errors so it is also useful for testing patches
under development.

Thanks.

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

* Re: [nft PATCH 6/6] Makefile: Enable support for 'make check'
  2025-08-27 23:08           ` Pablo Neira Ayuso
@ 2025-08-28 10:38             ` Phil Sutter
  0 siblings, 0 replies; 13+ messages in thread
From: Phil Sutter @ 2025-08-28 10:38 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

Hi,

On Thu, Aug 28, 2025 at 01:08:26AM +0200, Pablo Neira Ayuso wrote:
[...]
> 'make testrun' sounds nicer than my run-it-all shell script proposal,
> it would be nice to have a short summary at the test run not to scroll
> up to find each individual test result. And I think 'make testrun'
> should continue on errors so it is also useful for testing patches
> under development.

After a second look (and with Jan's distcheck fix patch in mind) I found
a way to distinguish between 'make distcheck' calling 'make check' and a
user doing so. This way we can use 'make check' as it is intended and
leverage automake's test suite integration while at the same time keep
'make distcheck' untouched.

I'll send a new series once I have collected what's really needed for
it.

Cheers, Phil

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

end of thread, other threads:[~2025-08-28 10:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-01 16:10 [nft PATCH 0/6] Run all test suites via 'make check' Phil Sutter
2025-08-01 16:11 ` [nft PATCH 1/6] tests: Prepare exit codes for automake Phil Sutter
2025-08-01 16:11 ` [nft PATCH 2/6] tests: monitor: Support running all tests in one go Phil Sutter
2025-08-01 16:11 ` [nft PATCH 3/6] tests: py: Set default options based on RUN_FULL_TESTSUITE Phil Sutter
2025-08-01 16:11 ` [nft PATCH 4/6] tests: json_echo: Skip if run as non-root Phil Sutter
2025-08-01 16:11 ` [nft PATCH 5/6] tests: shell: Skip packetpath/nat_ftp in fake root env Phil Sutter
2025-08-01 16:11 ` [nft PATCH 6/6] Makefile: Enable support for 'make check' Phil Sutter
2025-08-06 17:05   ` Pablo Neira Ayuso
2025-08-07 11:51     ` Phil Sutter
2025-08-07 12:14       ` Pablo Neira Ayuso
2025-08-07 13:04         ` Phil Sutter
2025-08-27 23:08           ` Pablo Neira Ayuso
2025-08-28 10:38             ` Phil Sutter

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