[PATCH nft v2 0/5] add infrastructure for unit tests

[PATCH nft v2 0/5] add infrastructure for unit tests

[Thomas Haller]

Changes to v1:

- rename some `make targets`
- add `make check-unit` to alias `make check-TESTS`. These targets
  run the tests hooked up as "TESTS=" (which for now are the tests
  in tests/unit).
- improve commit messages and various minor code changes.

Thomas Haller (5):
  build: add basic "check-{local,more,all}" and "build-all" make targets
  build: add `make check-build` to run `./tests/build/run-tests.sh`
  build: add `make check-tree` to check consistency of source tree
  build: cleanup if-blocks for conditional compilation in "Makefile.am"
  tests/unit: add unit tests for libnftables

 .gitignore                           |  15 +-
 Makefile.am                          | 134 ++++++++++++---
 src/.gitignore                       |   5 -
 tests/unit/nft-test.h                |  14 ++
 tests/unit/test-libnftables-static.c |  16 ++
 tests/unit/test-libnftables.c        |  21 +++
 tools/test-runner.sh                 | 235 +++++++++++++++++++++++++++
 7 files changed, 412 insertions(+), 28 deletions(-)
 create mode 100644 tests/unit/nft-test.h
 create mode 100644 tests/unit/test-libnftables-static.c
 create mode 100644 tests/unit/test-libnftables.c
 create mode 100755 tools/test-runner.sh

-- 
2.41.0

[PATCH nft v2 1/5] build: add basic "check-{local,more,all}" and "build-all" make targets

[Thomas Haller]

Add targets "check-local" and "check-more", which later will hook
up additional tests. For now, they are empty targets.

- with autotools, `make distcheck` implies `make check`.
- with autotools, `make check` implies `make check-local` and `make
  check-TESTS`.

Most tests should of course hook via `check-local` or via `TESTS=`
(`check-TESTS`). There is a small place for additional tests, in
particular "tests/build/run-tests.sh", which itself runs `make
distcheck`. So `make check-more` contains additional tests not run by
`make check`.

And `make check-all` just means `make check check-more`.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 Makefile.am | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 0ed831a19e95..93bd47970077 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -31,8 +31,11 @@ lib_LTLIBRARIES =
 noinst_LTLIBRARIES =
 sbin_PROGRAMS =
 check_PROGRAMS =
+check_LTLIBRARIES =
 dist_man_MANS =
 CLEANFILES =
+check_local =
+check_more =
 
 ###############################################################################
 
@@ -409,3 +412,23 @@ EXTRA_DIST += \
 
 pkgconfigdir = $(libdir)/pkgconfig
 pkgconfig_DATA = libnftables.pc
+
+###############################################################################
+
+build-all: all $(check_PROGRAMS) $(check_LTLIBRARIES)
+
+.PHONY: build-all
+
+###############################################################################
+
+check-local: build-all $(check_local)
+
+.PHONY: check-local $(check_local)
+
+check-more: build-all $(check_more)
+
+.PHONY: check-more $(check_more)
+
+check-all: check check-more
+
+.PHONY: check-all
-- 
2.41.0

[PATCH nft v2 2/5] build: add `make check-build` to run `./tests/build/run-tests.sh`

[Thomas Haller]

`./tests/build/run-tests.sh` performs a build check. Integrate it in
make as `make check-build` target.

`make check-build` is hooked into `make check-more`, which in turn is
hooked as `make check-all`.

Note that this is intentionally not part of `make check`, because `make
check` is run by `make distcheck`, and the build test itself calls `make
distcheck`. Even if that cycle would be resolved, doing a build check
several times during `make check` seems wrong and would add another
minute to the test runtime.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 Makefile.am | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 93bd47970077..6a0b04641afc 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -421,6 +421,14 @@ build-all: all $(check_PROGRAMS) $(check_LTLIBRARIES)
 
 ###############################################################################
 
+check-build: build-all
+	cd "$(srcdir)/tests/build/" ; \
+	CC="$(CC)" CFLAGS='-Werror' ./run-tests.sh
+
+check_more += check-build
+
+###############################################################################
+
 check-local: build-all $(check_local)
 
 .PHONY: check-local $(check_local)
-- 
2.41.0

[PATCH nft v2 3/5] build: add `make check-tree` to check consistency of source tree

[Thomas Haller]

The script "tools/check-tree.sh" performs some consistency checks of the
source tree. Call it from a make target "check-tree".

Note that `make check-tree` is hooked into `make check-local`, which in
turn is hooked into `make check` and `make distcheck`.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 Makefile.am | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Makefile.am b/Makefile.am
index 6a0b04641afc..48d138636d2f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -408,6 +408,7 @@ EXTRA_DIST += \
 EXTRA_DIST += \
 	files \
 	tests \
+	tools \
 	$(NULL)
 
 pkgconfigdir = $(libdir)/pkgconfig
@@ -421,6 +422,13 @@ build-all: all $(check_PROGRAMS) $(check_LTLIBRARIES)
 
 ###############################################################################
 
+check-tree:
+	"$(srcdir)/tools/check-tree.sh"
+
+check_local += check-tree
+
+###############################################################################
+
 check-build: build-all
 	cd "$(srcdir)/tests/build/" ; \
 	CC="$(CC)" CFLAGS='-Werror' ./run-tests.sh
-- 
2.41.0

[PATCH nft v2 4/5] build: cleanup if-blocks for conditional compilation in "Makefile.am"

[Thomas Haller]

`configure` sets those $(*_LIBS) variables to something empty, when the
dependency is not available. It's cumbersome and unnecessary to
explicitly check.

Also, the order in which libraries are specified on the command line
matters. Commonly we want our own libraries first ("src/libminigmp.la"
should come pretty early). That is cumbersome to get right otherwise.

Thus, remove those conditional checks and just refer to those $(*_LIBS)
variables unconditionally. Rely on them being empty, when the dependency
is disabled.

Also, as "src/xt.c" is always built, just directly add it to the list of
source files.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 Makefile.am | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 48d138636d2f..396bf3fa2c22 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -195,6 +195,12 @@ src_libminigmp_la_CFLAGS = \
 	-Wno-sign-compare \
 	$(NULL)
 
+LIBMINIGMP_LIBS = src/libminigmp.la
+
+else
+
+LIBMINIGMP_LIBS =
+
 endif
 
 ###############################################################################
@@ -247,10 +253,9 @@ src_libnftables_la_SOURCES = \
 	src/tcpopt.c \
 	src/utils.c \
 	src/xfrm.c \
+	src/xt.c \
 	$(NULL)
 
-src_libnftables_la_SOURCES += src/xt.c
-
 if BUILD_JSON
 src_libnftables_la_SOURCES += \
 	src/json.c \
@@ -264,23 +269,14 @@ src_libnftables_la_LDFLAGS = \
 	$(NULL)
 
 src_libnftables_la_LIBADD = \
+	src/libparser.la \
+	$(LIBMINIGMP_LIBS) \
 	$(LIBMNL_LIBS) \
 	$(LIBNFTNL_LIBS) \
-	src/libparser.la \
+	$(XTABLES_LIBS) \
+	$(JANSSON_LIBS) \
 	$(NULL)
 
-if BUILD_MINIGMP
-src_libnftables_la_LIBADD += src/libminigmp.la
-endif
-
-if BUILD_XTABLES
-src_libnftables_la_LIBADD += $(XTABLES_LIBS)
-endif
-
-if BUILD_JSON
-src_libnftables_la_LIBADD += $(JANSSON_LIBS)
-endif
-
 ###############################################################################
 
 sbin_PROGRAMS += src/nft
-- 
2.41.0

[PATCH nft v2 5/5] tests/unit: add unit tests for libnftables

[Thomas Haller]

We have a lot of tests/shell tests, that use the "nft" binary and we
have python tests that can use the public API of "libnftables.so".
However, it's useful to also write unit tests, that can test the
internal C code more immediately.

Since no such tests infrastructure exist yet, it would be cumbersome to
write such a test. Add two new test binaries, that can be used as a
place for such tests. Currently there are no real tests there, it's only
to show how it works, and that those dummy tests pass (including that
linking and execution passes).

To access the internals, build an intermediate static library
"src/libnftables-static.la", which then makes up the public, dynamic
"src/libnftables.la" library. Add two tests:

  - tests/unit/test-libnftables-static
  - tests/unit/test-libnftables

The former statically links with "src/libnftables-static.la" and can test
internal API from headers under "include/*.h". The latter dynamically
links with "src/libnftables.la", and can only test the public API from
"includes/nftables/libnftables.h".

You can run the unit tests alone with `make check-TESTS`. There is also
`make check-unit`, which aliases `check-TESTS` target. Calling
`VALGRIND=y make check` works as expected.

Also add a LOG_COMPILER script "tools/test-runner.sh". This wraps the
execution of the tests. Even for manual testing, you likely don't want
to run "tests/unit/test-*" executables directly, but rather

  $ ./tools/test-runner.sh tests/unit/test-libnftables

This sets up an unshared namespace, honors VALGRIND=y to run the
test under valgrind, handles libtool, and supports options --make
and --gdb. It also set up some environment variables that will be useful
for some tests (e.g. "$SRCDIR").

Also, ignore some build artifacts from top level gitignore file. The
build artifacts like "*.o" will not only be found under "src/". Move
those patterns.

Signed-off-by: Thomas Haller <thaller@redhat.com>
---
 .gitignore                           |  15 +-
 Makefile.am                          |  75 ++++++++-
 src/.gitignore                       |   5 -
 tests/unit/nft-test.h                |  14 ++
 tests/unit/test-libnftables-static.c |  16 ++
 tests/unit/test-libnftables.c        |  21 +++
 tools/test-runner.sh                 | 235 +++++++++++++++++++++++++++
 7 files changed, 365 insertions(+), 16 deletions(-)
 create mode 100644 tests/unit/nft-test.h
 create mode 100644 tests/unit/test-libnftables-static.c
 create mode 100644 tests/unit/test-libnftables.c
 create mode 100755 tools/test-runner.sh

diff --git a/.gitignore b/.gitignore
index a62e31f31c6b..369678a13987 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,6 +1,11 @@
-# Generated by autoconf/configure/automake
+# Generated by autoconf/configure/automake/make
 *.m4
+*.la
+*.lo
+*.o
+.deps/
 .dirstamp
+.libs/
 Makefile
 Makefile.in
 stamp-h1
@@ -20,7 +25,13 @@ libtool
 
 # Generated by tests
 *.payload.got
-tests/build/tests.log
+test-suite.log
+tests/**/*.log
+tests/**/*.trs
+tests/**/*.valgrind-log
+
+tests/unit/test-libnftables
+tests/unit/test-libnftables-static
 
 # Debian package build temporary files
 build-stamp
diff --git a/Makefile.am b/Makefile.am
index 396bf3fa2c22..d4656c340a31 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -31,12 +31,18 @@ lib_LTLIBRARIES =
 noinst_LTLIBRARIES =
 sbin_PROGRAMS =
 check_PROGRAMS =
+noinst_PROGRAMS =
 check_LTLIBRARIES =
 dist_man_MANS =
 CLEANFILES =
+TESTS =
+test_programs =
 check_local =
 check_more =
 
+TESTS_ENVIRONMENT = VERBOSE="$(V)" NFT_TEST_MAKE=n
+LOG_COMPILER = "$(srcdir)/tools/test-runner.sh" --srcdir "$(abs_srcdir)" --builddir "$(abs_builddir)" --
+
 ###############################################################################
 
 pkginclude_HEADERS = \
@@ -205,11 +211,9 @@ endif
 
 ###############################################################################
 
-lib_LTLIBRARIES += src/libnftables.la
+noinst_LTLIBRARIES += src/libnftables-static.la
 
-src_libnftables_la_SOURCES = \
-	src/libnftables.map \
-	\
+src_libnftables_static_la_SOURCES = \
 	src/cache.c \
 	src/cmd.c \
 	src/ct.c \
@@ -257,20 +261,36 @@ src_libnftables_la_SOURCES = \
 	$(NULL)
 
 if BUILD_JSON
-src_libnftables_la_SOURCES += \
+src_libnftables_static_la_SOURCES += \
 	src/json.c \
 	src/parser_json.c \
 	$(NULL)
 endif
 
+src_libnftables_static_la_LIBADD = \
+	src/libparser.la \
+	$(LIBMINIGMP_LIBS) \
+	$(LIBMNL_LIBS) \
+	$(LIBNFTNL_LIBS) \
+	$(XTABLES_LIBS) \
+	$(JANSSON_LIBS) \
+	$(NULL)
+
+###############################################################################
+
+lib_LTLIBRARIES += src/libnftables.la
+
+src_libnftables_la_SOURCES = \
+	src/libnftables.map \
+	$(NULL)
+
 src_libnftables_la_LDFLAGS = \
 	-version-info "${libnftables_LIBVERSION}" \
 	-Wl,--version-script="$(srcdir)/src//libnftables.map" \
 	$(NULL)
 
 src_libnftables_la_LIBADD = \
-	src/libparser.la \
-	$(LIBMINIGMP_LIBS) \
+	src/libnftables-static.la \
 	$(LIBMNL_LIBS) \
 	$(LIBNFTNL_LIBS) \
 	$(XTABLES_LIBS) \
@@ -303,6 +323,22 @@ examples_nft_json_file_LDADD = src/libnftables.la
 
 ###############################################################################
 
+EXTRA_DIST += tests/unit/nft-test.h
+
+###############################################################################
+
+test_programs += tests/unit/test-libnftables-static
+
+tests_unit_test_libnftables_static_LDADD = src/libnftables-static.la
+
+###############################################################################
+
+test_programs += tests/unit/test-libnftables
+
+tests_unit_test_libnftables_LDADD = src/libnftables.la
+
+###############################################################################
+
 if BUILD_MAN
 
 dist_man_MANS += \
@@ -390,6 +426,16 @@ dist_pkgsysconf_DATA = \
 
 ###############################################################################
 
+EXTRA_DIST += \
+	tests/build \
+	tests/json_echo \
+	tests/monitor \
+	tests/py \
+	tests/shell \
+	$(NULL)
+
+###############################################################################
+
 EXTRA_DIST += \
 	py/pyproject.toml \
 	py/setup.cfg \
@@ -403,7 +449,6 @@ EXTRA_DIST += \
 
 EXTRA_DIST += \
 	files \
-	tests \
 	tools \
 	$(NULL)
 
@@ -412,12 +457,24 @@ pkgconfig_DATA = libnftables.pc
 
 ###############################################################################
 
-build-all: all $(check_PROGRAMS) $(check_LTLIBRARIES)
+check_PROGRAMS += $(test_programs)
+
+TESTS += $(test_programs)
+
+###############################################################################
+
+build-all: all $(check_PROGRAMS) $(test_programs) $(check_LTLIBRARIES)
 
 .PHONY: build-all
 
 ###############################################################################
 
+check-unit: check-TESTS
+
+.PHONY: check-unit
+
+###############################################################################
+
 check-tree:
 	"$(srcdir)/tools/check-tree.sh"
 
diff --git a/src/.gitignore b/src/.gitignore
index 2d907425cbb0..f34105c6cda4 100644
--- a/src/.gitignore
+++ b/src/.gitignore
@@ -1,8 +1,3 @@
-*.la
-*.lo
-*.o
-.deps/
-.libs/
 nft
 parser_bison.c
 parser_bison.h
diff --git a/tests/unit/nft-test.h b/tests/unit/nft-test.h
new file mode 100644
index 000000000000..cab97b42c669
--- /dev/null
+++ b/tests/unit/nft-test.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#ifndef __NFT_TEST__H__
+#define __NFT_TEST__H__
+
+#undef NDEBUG
+
+#include <nft.h>
+
+#include <assert.h>
+#include <stdio.h>
+#include <string.h>
+
+#endif /* __NFT_TEST__H__ */
diff --git a/tests/unit/test-libnftables-static.c b/tests/unit/test-libnftables-static.c
new file mode 100644
index 000000000000..e34fcfd77f39
--- /dev/null
+++ b/tests/unit/test-libnftables-static.c
@@ -0,0 +1,16 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "nft-test.h"
+
+#include <datatype.h>
+
+static void test_datatype(void)
+{
+	assert(!datatype_lookup(-1));
+}
+
+int main(int argc, char **argv)
+{
+	test_datatype();
+	return 0;
+}
diff --git a/tests/unit/test-libnftables.c b/tests/unit/test-libnftables.c
new file mode 100644
index 000000000000..100558cd5e0f
--- /dev/null
+++ b/tests/unit/test-libnftables.c
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+#include "nft-test.h"
+
+#include <nftables/libnftables.h>
+
+static void test_nft_ctx(void)
+{
+	struct nft_ctx *ctx;
+
+	ctx = nft_ctx_new(0);
+	assert(ctx);
+
+	nft_ctx_free(ctx);
+}
+
+int main(int argc, char **argv)
+{
+	test_nft_ctx();
+	return 0;
+}
diff --git a/tools/test-runner.sh b/tools/test-runner.sh
new file mode 100755
index 000000000000..bb10af19c3a4
--- /dev/null
+++ b/tools/test-runner.sh
@@ -0,0 +1,235 @@
+#!/bin/bash
+
+set -e
+
+die() {
+	printf '%s\n' "$*"
+	exit 1
+}
+
+usage() {
+	echo "  $0 [OPTIONS] TEST"
+	echo
+	echo "Run TEST. Usually you don't want to run our unit test"
+	echo "executables directly, but via this wrapper script."
+	echo
+	echo "This script is also the LOG_COMPILER for all TESTS in Makefile.am."
+	echo
+	echo "Interesting Options:"
+	echo "  -V|--valgrind: Sets VALGRIND=y to run under valgrind."
+	echo "  -G|--gdb: Sets NFT_TEST_WRAPPER=gdb to start a debugger."
+	echo "  -m|--make: Sets NFT_TEST_MAKE=y to build the test first."
+	echo "Other options:"
+	echo "  --srcdir dir: Sets SRCDIR=dir."
+	echo "  --builddir dir: Sets BUILDDIR=dir."
+	echo "  --: Separates options from test name."
+	echo "  TEST: the path of the test executable to run."
+	echo
+	echo "Environment variables:"
+	echo "  SRCDIR: set to \$(srcdir) of the project."
+	echo "  BUILDDIR: set to \$(builddir) of the project."
+	echo "  VERBOSE: for verbose output."
+	echo "  VALGRIND: if set to TRUE, run the test under valgrind. NFT_TEST_UNDER_VALGRIND is ignored."
+	echo "  NFT_TEST_UNSHARE_CMD: override the command to unshare the netns."
+	echo "      Set to empty to not unshare."
+	echo "  NFT_TEST_WRAPPER: usually empty. For manual testing, this is prepended to TEST."
+	echo "      Set for example got \"gdb\"."
+	echo "  NFT_TEST_MAKE: if true, call \`make\` on the test first."
+}
+
+usage_and_die() {
+	usage
+	echo
+	die "$@"
+}
+
+TIMESTAMP=$(date '+%Y%m%d-%H%M%S.%3N')
+
+as_bool() {
+	if [ "$#" -eq 0 ] ; then
+		return 1
+	fi
+	case "$1" in
+		y|Y|yes|Yes|YES|1|true|True|TRUE)
+			return 0
+			;;
+		n|N|no|No|NO|0|false|False|FALSE)
+			return 1
+			;;
+		*)
+			# Fallback to the next in the list.
+			shift
+			rc=0
+			as_bool "$@" || rc=$?
+			return "$rc"
+	esac
+}
+
+as_bool_str() {
+	if as_bool "$@" ; then
+		printf y
+	else
+		printf n
+	fi
+}
+
+# Honored environment variables from the caller (and their defaults).
+# SRCDIR unset
+# BUILDDIR unset
+# TMP unset
+# NFT_TEST_VALGRIND_OPTS unset
+VERBOSE="$(as_bool_str "$VERBOSE")"
+VALGRIND="$(as_bool_str "$VALGRIND")"
+NFT_TEST_UNSHARE_CMD="${NFT_TEST_UNSHARE_CMD-unshare -m -U --map-root-user -n}"
+NFT_TEST_WRAPPER="${NFT_TEST_WRAPPER}"
+NFT_TEST_MAKE="$(as_bool_str "$NFT_TEST_MAKE")"
+
+unset TEST
+
+while [ $# -gt 0 ] ; do
+	A="$1"
+	shift
+	case "$A" in
+		-h|--help)
+			usage
+			exit 0
+			;;
+		--srcdir)
+			SRCDIR="$1"
+			shift
+			;;
+		--builddir)
+			BUILDDIR="$1"
+			shift
+			;;
+		-V|--valgrind)
+			VALGRIND=y
+			;;
+		-G|--gdb)
+			NFT_TEST_WRAPPER=gdb
+			;;
+		-m|--make)
+			NFT_TEST_MAKE=y
+			;;
+		--)
+			if [ $# -ne 1 ] ; then
+				usage_and_die "Requires a TEST argument after --"
+			fi
+			TEST="$1"
+			shift
+			;;
+		*)
+			if [ -n "${TEST+x}" ] ; then
+				usage_and_die "Unknown argument \"$A\""
+			fi
+			TEST="$A"
+			;;
+	esac
+done
+
+if [ -z "$TEST" ] ; then
+	usage_and_die "Missing test argument. See --help"
+fi
+
+if [ -z "${SRCDIR+x}" ] ; then
+	SRCDIR="$(readlink -f "$(dirname "$0")/..")"
+fi
+if [ ! -d "$SRCDIR" ] ; then
+	die "Invalid \$SRCDIR=\"$SRCDIR\""
+fi
+
+if [ -z "${BUILDDIR+x}" ] ; then
+	re='^(.*/|)(tests/unit/test-[^/]*)$'
+	if [[ "$TEST" =~ $re ]] ; then
+		BUILDDIR="$(readlink -f "${BASH_REMATCH[1]:-.}")" || :
+	else
+		BUILDDIR="$SRCDIR"
+	fi
+fi
+if [ ! -d "$BUILDDIR" ] ; then
+	die "Invalid \$BUILDDIR=\"$BUILDDIR\""
+fi
+
+export TESTDIR="$(dirname "$TEST")"
+export SRCDIR
+export BUILDDIR
+
+if [ "$VALGRIND" = y ] ; then
+	export NFT_TEST_UNDER_VALGRIND=1
+fi
+
+run_unit() {
+	local TEST="$1"
+
+	if [ "$NFT_TEST_MAKE" = y ] ; then
+		local d="$(readlink -f "$BUILDDIR")"
+		local tf="$(readlink -f "$TEST")"
+		local t="${tf#$d/}"
+
+		if [ "$tf" != "$d/$t" ] ; then
+			die "Cannot detect paths for making \"$TEST\" in \"$BUILDDIR\" (\"$d\" and \"$t\")"
+		fi
+		# Don't use "make -C" to avoid the extra "Entering/Leaving directory" messages
+		( cd "$d" && make "$t" ) || die "Making \"$TEST\" failed"
+	fi
+
+	if [ "$VALGRIND" != y ] ; then
+		rc_test=0
+		$NFT_TEST_UNSHARE_CMD \
+			libtool \
+			--mode=execute \
+			$NFT_TEST_WRAPPER \
+			"$TEST" || rc_test=$?
+		if [ "$rc_test" -ne 0 -a "$rc_test" -ne 77 ] ; then
+			die "exec \"$TEST\" failed with exit code $rc_test"
+		fi
+		exit "$rc_test"
+	fi
+
+	LOGFILE="$TEST.valgrind-log"
+
+	rc_test=0
+	$NFT_TEST_UNSHARE_CMD \
+		libtool \
+		--mode=execute \
+		valgrind \
+		--quiet \
+		--error-exitcode=122 \
+		--leak-check=full \
+		--gen-suppressions=all \
+		--num-callers=100 \
+		--log-file="$LOGFILE" \
+		--vgdb-prefix="${TMP:-/tmp}/vgdb-pipe-nft-test-runner-$TIMESTAMP-$$" \
+		$NFT_TEST_VALGRIND_OPTS \
+		"$TEST" \
+		|| rc_test=$?
+
+	if [ -s "$LOGFILE" ] ; then
+		if [ "$rc_test" -eq 0 -o "$rc_test" -eq 122 ] ; then
+			echo "valgrind failed. Logfile at \"$LOGFILE\" :"
+			cat "$LOGFILE"
+			rc_test=122
+		else
+			echo "valgrind also failed. Check logfile at \"$LOGFILE\""
+		fi
+	elif [ ! -f "$LOGFILE" ] ; then
+		echo "valgrind logfile \"$LOGFILE\" missing"
+		if [ "$rc_test" -eq 0 ] ; then
+			rc_test=122
+		fi
+	else
+		rm -rf "$LOGFILE"
+	fi
+
+	exit "$rc_test"
+}
+
+case "$TEST" in
+	tests/unit/test-* | \
+	*/tests/unit/test-* )
+		run_unit "$TEST"
+		;;
+	*)
+		die "Unrecognized test \"$TEST\""
+		;;
+esac
-- 
2.41.0

Re: [PATCH nft v2 0/5] add infrastructure for unit tests

[Thomas Haller]

On Sun, 2023-11-05 at 16:08 +0100, Thomas Haller wrote:
> Changes to v1:
> 
> - rename some `make targets`
> - add `make check-unit` to alias `make check-TESTS`. These targets
>   run the tests hooked up as "TESTS=" (which for now are the tests
>   in tests/unit).
> - improve commit messages and various minor code changes.
> 
> Thomas Haller (5):
>   build: add basic "check-{local,more,all}" and "build-all" make
> targets
>   build: add `make check-build` to run `./tests/build/run-tests.sh`
>   build: add `make check-tree` to check consistency of source tree
>   build: cleanup if-blocks for conditional compilation in
> "Makefile.am"
>   tests/unit: add unit tests for libnftables
> 
>  .gitignore                           |  15 +-
>  Makefile.am                          | 134 ++++++++++++---
>  src/.gitignore                       |   5 -
>  tests/unit/nft-test.h                |  14 ++
>  tests/unit/test-libnftables-static.c |  16 ++
>  tests/unit/test-libnftables.c        |  21 +++
>  tools/test-runner.sh                 | 235
> +++++++++++++++++++++++++++
>  7 files changed, 412 insertions(+), 28 deletions(-)
>  create mode 100644 tests/unit/nft-test.h
>  create mode 100644 tests/unit/test-libnftables-static.c
>  create mode 100644 tests/unit/test-libnftables.c
>  create mode 100755 tools/test-runner.sh


Hi Pablo,

any concerns about this? Could it be merged?

Thank you,
Thomas

Re: [PATCH nft v2 0/5] add infrastructure for unit tests

[Pablo Neira Ayuso]

On Tue, Nov 21, 2023 at 01:34:54PM +0100, Thomas Haller wrote:
> Hi Pablo,
> 
> any concerns about this? Could it be merged?

Sorry. JSON support is not working, I had to locally revert those
patches to run tests on -stable 5.4 here.

Let's agree on some basic rule from now on: One series at a time only,
anything else coming after will be marked as deferred in patchwork.

Thanks.

Re: [PATCH nft v2 0/5] add infrastructure for unit tests

[Thomas Haller]

On Tue, 2023-11-21 at 13:37 +0100, Pablo Neira Ayuso wrote:
> On Tue, Nov 21, 2023 at 01:34:54PM +0100, Thomas Haller wrote:
> > Hi Pablo,
> > 
> > any concerns about this? Could it be merged?
> 
> Sorry. JSON support is not working, I had to locally revert those
> patches to run tests on -stable 5.4 here.
> 
> Let's agree on some basic rule from now on: One series at a time
> only,
> anything else coming after will be marked as deferred in patchwork.
> 
> Thanks.
> 


Hi,

Could this be considered?

This provides the basis for unit tests (and the possibility to even add
any such tests).

It also hooks up tests to `make check`. Which would be desirable to
build upon. `make check` currently does nothing. For example, Florian's
afl++ patches could hook into `make check` (or `make check-more`), if
this basis was there.


Thomas

Re: [PATCH nft v2 0/5] add infrastructure for unit tests

[Pablo Neira Ayuso]

On Wed, Dec 06, 2023 at 08:53:23AM +0100, Thomas Haller wrote:
> On Tue, 2023-11-21 at 13:37 +0100, Pablo Neira Ayuso wrote:
> > On Tue, Nov 21, 2023 at 01:34:54PM +0100, Thomas Haller wrote:
> > > Hi Pablo,
> > > 
> > > any concerns about this? Could it be merged?
> > 
> > Sorry. JSON support is not working, I had to locally revert those
> > patches to run tests on -stable 5.4 here.
> > 
> > Let's agree on some basic rule from now on: One series at a time
> > only,
> > anything else coming after will be marked as deferred in patchwork.
> > 
> > Thanks.
> > 
> 
> 
> Hi,
> 
> Could this be considered?
> 
> This provides the basis for unit tests (and the possibility to even add
> any such tests).

We are still discussing the json integration into tests/shell. I
suggest, let dust settle on each front before making more changes.

> It also hooks up tests to `make check`. Which would be desirable to
> build upon. `make check` currently does nothing. For example, Florian's
> afl++ patches could hook into `make check` (or `make check-more`), if
> this basis was there.

I still doubt `make check` provides any benefit to the release
process, which will exercise this path because of `make distcheck'
which I might have to relax it to `make dist' to skip this to ensure
release process is reliable.

I think all these tests should continously and provide reports to us,
but not necessarily integrate them into `make check'.