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 --- 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 Introduce -J/--disable-json and -S/--no-schema to explicitly disable them if desired. Signed-off-by: Phil Sutter --- 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 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 --- 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 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 --- 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 The script relies upon a call to modprobe which does not work in fake root environments. Signed-off-by: Phil Sutter --- 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 Cover for being called from a different directory by changing into the test suite's directory first. Signed-off-by: Phil Sutter --- 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 When called by 'make check', the test suite runs with a MAKEFLAGS variable in environment which defines TEST_LOGS variable with the test suites' corresponding logs as value. This in turn causes the called 'make distcheck' to run test suites although it is not supposed to. Signed-off-by: Phil Sutter --- tests/build/run-tests.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/build/run-tests.sh b/tests/build/run-tests.sh index a5e026a97dd5b..80fa10168d003 100755 --- a/tests/build/run-tests.sh +++ b/tests/build/run-tests.sh @@ -20,6 +20,10 @@ fi git clone "$dir" "$tmpdir" &>>"$log_file" cd "$tmpdir" || exit +# do not leak data from a calling 'make check' run into the new build otherwise +# this will defeat the test suite invocation prevention for 'make distcheck' +unset MAKEFLAGS + if ! autoreconf -fi &>>"$log_file" ; then echo "Something went wrong. Check the log '${log_file}' for details." exit 1 -- 2.51.0 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 --- Changes since v3: - gitignore 'make check' generated logs and reports Changes since v2: - Drop RUN_FULL_TESTSUITE env var, it is not needed anymore Changes since v1: - Add an internal configure option set by the distcheck target when building the project - Have this configure option define BUILD_DISTCHECK automake variable - Leave TESTS empty if BUILD_DISTCHECK is set to avoid test suite runs with 'make distcheck' --- .gitignore | 13 +++++++++++++ Makefile.am | 9 +++++++++ configure.ac | 5 +++++ 3 files changed, 27 insertions(+) diff --git a/.gitignore b/.gitignore index 1e3bc5146b2f1..db329eafa5298 100644 --- a/.gitignore +++ b/.gitignore @@ -23,6 +23,19 @@ nftversion.h *.payload.got tests/build/tests.log +# make check results +/test-suite.log +/tests/build/run-tests.sh.log +/tests/build/run-tests.sh.trs +/tests/json_echo/run-test.py.log +/tests/json_echo/run-test.py.trs +/tests/monitor/run-tests.sh.log +/tests/monitor/run-tests.sh.trs +/tests/py/nft-test.py.log +/tests/py/nft-test.py.trs +/tests/shell/run-tests.sh.log +/tests/shell/run-tests.sh.trs + # Debian package build temporary files build-stamp diff --git a/Makefile.am b/Makefile.am index 5190a49ae69f1..9112faa2d5c04 100644 --- a/Makefile.am +++ b/Makefile.am @@ -23,6 +23,7 @@ libnftables_LIBVERSION = 2:0:1 ############################################################################### ACLOCAL_AMFLAGS = -I m4 +AM_DISTCHECK_CONFIGURE_FLAGS = --enable-distcheck EXTRA_DIST = BUILT_SOURCES = @@ -429,3 +430,11 @@ doc_DATA = files/nftables/main.nft tools/nftables.service: tools/nftables.service.in ${top_builddir}/config.status ${AM_V_GEN}${MKDIR_P} tools ${AM_V_at}sed -e 's|@''sbindir''@|${sbindir}|g;s|@''pkgsysconfdir''@|${pkgsysconfdir}|g' <${srcdir}/tools/nftables.service.in >$@ + +if !BUILD_DISTCHECK +TESTS = tests/build/run-tests.sh \ + tests/json_echo/run-test.py \ + tests/monitor/run-tests.sh \ + tests/py/nft-test.py \ + tests/shell/run-tests.sh +endif diff --git a/configure.ac b/configure.ac index da16a6e257c91..8073d4d8193e2 100644 --- a/configure.ac +++ b/configure.ac @@ -155,6 +155,11 @@ AC_CONFIG_COMMANDS([nftversion.h], [ AC_SUBST([MAKE_STAMP], ["\$(shell date +%s)"]) CFLAGS="${CFLAGS} -DMAKE_STAMP=\${MAKE_STAMP}" +AC_ARG_ENABLE([distcheck], + AS_HELP_STRING([--enable-distcheck], [Build for distcheck]), + [enable_distcheck=yes], []) +AM_CONDITIONAL([BUILD_DISTCHECK], [test "x$enable_distcheck" = "xyes"]) + AC_CONFIG_FILES([ \ Makefile \ libnftables.pc \ -- 2.51.0