From 5031fc3c7163b254697f2c0a74b8a750de5fe7a8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 23 Nov 2015 17:40:44 +0100 Subject: [PATCH 1/5] nmtst: initialize g_test_init() after parsing NMTST_DEBUG --- include/nm-test-utils.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/include/nm-test-utils.h b/include/nm-test-utils.h index a978a318f7..8f23b79e17 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -286,17 +286,6 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ if (argc) __nmtst_internal.orig_argv = g_strdupv (*argv); - if (argc && !g_test_initialized ()) { - /* g_test_init() is a variadic function, so we cannot pass it - * (variadic) arguments. If you need to pass additional parameters, - * call nmtst_init() with argc==NULL and call g_test_init() yourself. */ - - /* g_test_init() sets g_log_set_always_fatal() for G_LOG_LEVEL_WARNING - * and G_LOG_LEVEL_CRITICAL. So, beware that the test will fail if you - * have any WARN or ERR log messages -- unless you g_test_expect_message(). */ - g_test_init (argc, argv, NULL); - } - __nmtst_internal.assert_logging = !!assert_logging; nm_g_type_init (); @@ -370,6 +359,17 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ } } + if (argc && !g_test_initialized ()) { + /* g_test_init() is a variadic function, so we cannot pass it + * (variadic) arguments. If you need to pass additional parameters, + * call nmtst_init() with argc==NULL and call g_test_init() yourself. */ + + /* g_test_init() sets g_log_set_always_fatal() for G_LOG_LEVEL_WARNING + * and G_LOG_LEVEL_CRITICAL. So, beware that the test will fail if you + * have any WARN or ERR log messages -- unless you g_test_expect_message(). */ + g_test_init (argc, argv, NULL); + } + if (test_quick_set) __nmtst_internal.test_quick = test_quick; else if (test_quick_argv) From c8174f0f9f2a38f0022ef49fb905d3b891b97e85 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 23 Nov 2015 18:50:48 +0100 Subject: [PATCH 2/5] nmtst: detect whether the test runs as tap test Same as gtestutils does, look for --tap command line argument. --- include/nm-test-utils.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/nm-test-utils.h b/include/nm-test-utils.h index 8f23b79e17..13dd4ebad1 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -153,6 +153,7 @@ struct __nmtst_internal gboolean assert_logging; gboolean no_expect_message; gboolean test_quick; + gboolean test_tap_log; char *sudo_cmd; char **orig_argv; }; @@ -356,6 +357,8 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ || !strcmp (*(a+1), "slow") || !strcmp (*(a+1), "thorough")))) test_quick_argv = TRUE; + else if (strcmp (*a, "--tap") == 0) + __nmtst_internal.test_tap_log = TRUE; } } From a6a2fd7eef5a1aefcc7ab6d35b84a8d4c0039cd1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 24 Nov 2015 09:40:39 +0100 Subject: [PATCH 3/5] nmtst: pass -m=quick when specifying quick test in $NMTST_DEBUG When the environment variable indicates that we want to run quick tests, pass "-m=quick" to g_test_init(). --- include/nm-test-utils.h | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/include/nm-test-utils.h b/include/nm-test-utils.h index 13dd4ebad1..f896253a38 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -370,7 +370,44 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ /* g_test_init() sets g_log_set_always_fatal() for G_LOG_LEVEL_WARNING * and G_LOG_LEVEL_CRITICAL. So, beware that the test will fail if you * have any WARN or ERR log messages -- unless you g_test_expect_message(). */ - g_test_init (argc, argv, NULL); + GPtrArray *arg_array = g_ptr_array_new (); + gs_free char **arg_array_c = NULL; + int arg_array_n, j; + + if (*argc) { + for (i = 0; i < *argc; i++) + g_ptr_array_add (arg_array, (*argv)[i]); + } else + g_ptr_array_add (arg_array, "./test"); + + if (test_quick_set && !test_quick_argv) + g_ptr_array_add (arg_array, "-m=quick"); + + g_ptr_array_add (arg_array, NULL); + + arg_array_n = arg_array->len - 1; + arg_array_c = (char **) g_ptr_array_free (arg_array, FALSE); + + g_test_init (&arg_array_n, &arg_array_c, NULL); + + if (*argc > 1) { + /* collaps argc/argv by removing the arguments detected + * by g_test_init(). */ + for (i = 1, j = 1; i < *argc; i++) { + if ((*argv)[i] == arg_array_c[j]) + j++; + else + (*argv)[i] = NULL; + } + for (i = 1, j = 1; i < *argc; i++) { + if ((*argv)[i]) { + (*argv)[j++] = (*argv)[i]; + if (i >= j) + (*argv)[i] = NULL; + } + } + *argc = j; + } } if (test_quick_set) From 73cb57910835ef46d43bb92309def186e38534cf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 23 Nov 2015 18:15:39 +0100 Subject: [PATCH 4/5] nmtst: support -p and -s arguments from gtestutils via $NMTST_DEBUG For tests based on glib's test framework, you can select which tests to run by passing -p/-s on the command line. Usually, we want to invoke our tests via `make check` which conveniently calls valgrind (run-test-valgrind.sh) or spawns a private D-Bus server (libnm-test-launch.sh, libnm-glib-test-launch.sh). At that point, it is not directly possible to specify command line arguments for the tests, which is why it is convenient to specify arguments via $NMTST_DEBUG environment variable. Parse "p" and "s" arguments from $NMTST_DEBUG and pass them to g_test_init() to select which tests to run. NMTST_DEBUG=p=/core/general/test_setting_ip4_changed_signal ./libnm-core/tests/test-general However, there is a problem here: in gtestutils, -p/-s conflicts with --tap, which is how our Makefile invokes the tests. Thus the new options explicitly don't work when being called during `make check`. Which makes this much less useful. I only noticed that afterwards, so still keep the patch because it might still be convenient during developing tests to set the environment variable once, and then repeatedly spawn the tests, without specifying -p/-s. --- include/nm-test-utils.h | 50 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/include/nm-test-utils.h b/include/nm-test-utils.h index f896253a38..9fc6fad618 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -80,6 +80,10 @@ * - respect g_test_quick(), if the command line contains '-mslow', '-mquick', '-mthorough'. * - use compile time default * + * "p=PATH"|"s=PATH": passes the path to g_test_init() as "-p" and "-s", respectively. + * Unfortunately, these options conflict with "--tap" which our makefile passes to the + * tests, thus it's only useful outside of `make check`. + * *******************************************************************************/ #include @@ -268,6 +272,8 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ gboolean test_quick = FALSE; gboolean test_quick_set = FALSE; gboolean test_quick_argv = FALSE; + gs_unref_ptrarray GPtrArray *p_tests = NULL; + gs_unref_ptrarray GPtrArray *s_tests = NULL; if (!out_set_logging) out_set_logging = &_out_set_logging; @@ -324,6 +330,14 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ sudo_cmd = g_strdup (&debug[strlen ("sudo-cmd=")]); } else if (!g_ascii_strcasecmp (debug, "no-expect-message")) { no_expect_message = TRUE; + } else if (!g_ascii_strncasecmp (debug, "p=", strlen ("p="))) { + if (!p_tests) + p_tests = g_ptr_array_new_with_free_func (g_free); + g_ptr_array_add (p_tests, g_strdup (&debug[strlen ("p=")])); + } else if (!g_ascii_strncasecmp (debug, "s=", strlen ("s="))) { + if (!s_tests) + s_tests = g_ptr_array_new_with_free_func (g_free); + g_ptr_array_add (s_tests, g_strdup (&debug[strlen ("s=")])); } else if (!g_ascii_strcasecmp (debug, "slow") || !g_ascii_strcasecmp (debug, "thorough")) { test_quick = FALSE; test_quick_set = TRUE; @@ -362,7 +376,13 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ } } - if (argc && !g_test_initialized ()) { + if (!argc || g_test_initialized ()) { + if (p_tests || s_tests) { + char *msg = g_strdup_printf (">>> nmtst: ignore -p and -s options for test which calls g_test_init() itself"); + + g_array_append_val (debug_messages, msg); + } + } else { /* g_test_init() is a variadic function, so we cannot pass it * (variadic) arguments. If you need to pass additional parameters, * call nmtst_init() with argc==NULL and call g_test_init() yourself. */ @@ -373,6 +393,7 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ GPtrArray *arg_array = g_ptr_array_new (); gs_free char **arg_array_c = NULL; int arg_array_n, j; + static char **s_tests_x, **p_tests_x; if (*argc) { for (i = 0; i < *argc; i++) @@ -383,6 +404,21 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ if (test_quick_set && !test_quick_argv) g_ptr_array_add (arg_array, "-m=quick"); + if (!__nmtst_internal.test_tap_log) { + for (i = 0; p_tests && i < p_tests->len; i++) { + g_ptr_array_add (arg_array, "-p"); + g_ptr_array_add (arg_array, p_tests->pdata[i]); + } + for (i = 0; s_tests && i < s_tests->len; i++) { + g_ptr_array_add (arg_array, "-s"); + g_ptr_array_add (arg_array, s_tests->pdata[i]); + } + } else if (p_tests || s_tests) { + char *msg = g_strdup_printf (">>> nmtst: ignore -p and -s options for tap-tests"); + + g_array_append_val (debug_messages, msg); + } + g_ptr_array_add (arg_array, NULL); arg_array_n = arg_array->len - 1; @@ -408,6 +444,18 @@ __nmtst_init (int *argc, char ***argv, gboolean assert_logging, const char *log_ } *argc = j; } + + /* we must "leak" the test paths because they are not cloned by g_test_init(). */ + if (!__nmtst_internal.test_tap_log) { + if (p_tests) { + p_tests_x = (char **) g_ptr_array_free (p_tests, FALSE); + p_tests = NULL; + } + if (s_tests) { + s_tests_x = (char **) g_ptr_array_free (s_tests, FALSE); + s_tests = NULL; + } + } } if (test_quick_set) From 4dacf0b1ad716747310e56188a0d283ee47ebee0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 23 Nov 2015 19:23:45 +0100 Subject: [PATCH 5/5] nmtst/valgrind: allow calling 'run-test-valgrind.sh' script directly When you want to run valgrind for a test, you either had to invoke valgrind manually, or doing it via `make check` (provided you configured --with-valgrind). Make it more convenient to run valgrind by passing the test to run to the "run-test-valgrind.sh" wrapper. This also allows to pass -p/-s to the test, which is not possible during `make check` because selecting tests conflicts with "--tap". The following invocations are largely equivalent and work as expected: $ ./tools/run-test-valgrind.sh ./src/platform/tests/test-link-linux -p /link/software/detect/vlan $ NMTST_DEBUG=no-debug,p=/link/software/detect/vlan ./tools/run-test-valgrind.sh ./src/platform/tests/test-link-linux --- configure.ac | 2 +- tools/run-test-valgrind.sh | 93 ++++++++++++++++++++++++++++++++++---- 2 files changed, 85 insertions(+), 10 deletions(-) diff --git a/configure.ac b/configure.ac index 6f38831d3a..fd5a47c5a4 100644 --- a/configure.ac +++ b/configure.ac @@ -945,7 +945,7 @@ else fi fi AS_IF([test "$with_valgrind" != "no"], - AC_SUBST(VALGRIND_RULES, 'LOG_COMPILER = "$(top_srcdir)/tools/run-test-valgrind.sh" "$(LIBTOOL)" "$(with_valgrind)" '"$with_valgrind_suppressions"), + AC_SUBST(VALGRIND_RULES, 'LOG_COMPILER = "$(top_srcdir)/tools/run-test-valgrind.sh" --called-from-make "$(LIBTOOL)" "$(with_valgrind)" '"$with_valgrind_suppressions"), AC_SUBST(VALGRIND_RULES, [])) AM_CONDITIONAL(WITH_VALGRIND, test "${with_valgrind}" != "no") diff --git a/tools/run-test-valgrind.sh b/tools/run-test-valgrind.sh index ff5bc44ce3..424f633d06 100755 --- a/tools/run-test-valgrind.sh +++ b/tools/run-test-valgrind.sh @@ -1,19 +1,93 @@ -#!/bin/sh +#!/bin/bash + +die() { + echo "$@" + exit 5 +} + +SCRIPT_PATH="${SCRIPT_PATH:-$(readlink -f "$(dirname "$0")")}" -LIBTOOL="$1"; shift -VALGRIND="$1"; shift -SUPPRESSIONS="$1"; shift VALGRIND_ERROR=37 -if [ "$1" = "--launch-dbus" ]; then +if [ "$1" == "--called-from-make" ]; then + shift + NMTST_LIBTOOL=($1 --mode=execute); shift + NMTST_VALGRIND="$1"; shift + SUPPRESSIONS="$1"; shift + if [ "$1" = "--launch-dbus" ]; then + NMTST_LAUNCH_DBUS=yes + shift + else + NMTST_LAUNCH_DBUS=no + fi + TEST="$1"; shift +else + if [ -n "${NMTST_LIBTOOL-:x}" ]; then + NMTST_LIBTOOL=(sh "$SCRIPT_PATH/../libtool" --mode=execute) + elif [ -n "${NMTST_LIBTOOL-x}" ]; then + NMTST_LIBTOOL=() + else + NMTST_LIBTOOL=($NMTST_LIBTOOL --mode=execute) + fi + for a in "$@"; do + case "$a" in + "--launch-dbus") + NMTST_LAUNCH_DBUS=yes + shift + ;; + "--no-launch-dbus"|"-D") + NMTST_LAUNCH_DBUS=no + shift + ;; + "--no-libtool") + NMTST_LIBTOOL=() + shift + ;; + "--") + shift + break + ;; + *) + break + ;; + esac + done + # we support calling the script directly. In this case, + # only pass the path to the test to run. + TEST="$1"; shift + NMTST_VALGRIND="${NMTST_VALGRIND:-valgrind}" + if [ "$SUPPRESSIONS" == "" ]; then + SUPPRESSIONS="$SCRIPT_PATH/../valgrind.suppressions" + fi + + [ -x "$TEST" ] || die "Test \"$TEST\" does not exist" + + TEST_PATH="$(readlink -f "$(dirname "$TEST")")" + + if [ -n "${NMTST_LAUNCH_DBUS-x}" ]; then + # autodetect whether to launch D-Bus based on the test path. + if [[ $TEST_PATH == */libnm/tests || $TEST_PATH == */libnm-glib/tests ]]; then + NMTST_LAUNCH_DBUS=yes + else + NMTST_LAUNCH_DBUS=no + fi + fi + + # some tests require you to cd into the base directory. + # do that. + if [ "$NMTST_VALGRIND_NO_CD" == "" ]; then + cd "$TEST_PATH" + TEST="./$(basename "$TEST")" + fi +fi + +if [ "$NMTST_LAUNCH_DBUS" == "yes" ]; then # Spawn DBus eval `dbus-launch --sh-syntax` trap "kill $DBUS_SESSION_BUS_PID" EXIT - shift fi -TEST="$1" if [ "$NMTST_NO_VALGRIND" != "" ]; then - "$@" + "$TEST" "$@" exit $? fi @@ -21,7 +95,7 @@ LOGFILE="valgrind-`echo "$TEST" | tr -cd '[:alpha:]-'`.log" export G_SLICE=always-malloc export G_DEBUG=gc-friendly -$LIBTOOL --mode=execute "$VALGRIND" \ +"${NMTST_LIBTOOL[@]}" "$NMTST_VALGRIND" \ --quiet \ --error-exitcode=$VALGRIND_ERROR \ --leak-check=full \ @@ -29,6 +103,7 @@ $LIBTOOL --mode=execute "$VALGRIND" \ --suppressions="$SUPPRESSIONS" \ --num-callers=100 \ --log-file="$LOGFILE" \ + "$TEST" \ "$@" RESULT=$?