From c3aafd9dabac1ead75e6199cdd83e3da5e431e78 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 (cherry picked from commit 5031fc3c7163b254697f2c0a74b8a750de5fe7a8) --- 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 258d39370b..2cec325bbe 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -275,17 +275,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; #if !GLIB_CHECK_VERSION (2, 35, 0) @@ -361,6 +350,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 8bdb2aaeea59e5b0c06980cee232be31490833c0 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. (cherry picked from commit c8174f0f9f2a38f0022ef49fb905d3b891b97e85) --- 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 2cec325bbe..354f7efaf3 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -142,6 +142,7 @@ struct __nmtst_internal gboolean assert_logging; gboolean no_expect_message; gboolean test_quick; + gboolean test_tap_log; char *sudo_cmd; char **orig_argv; }; @@ -347,6 +348,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 7eed71d860b5e91c2da1c66396f8c3135eb37eb7 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(). (cherry picked from commit a6a2fd7eef5a1aefcc7ab6d35b84a8d4c0039cd1) --- 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 354f7efaf3..3630d78bcd 100644 --- a/include/nm-test-utils.h +++ b/include/nm-test-utils.h @@ -361,7 +361,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 243d0facb89203647c24bca2fe06e2c917e6a8fb 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. (cherry picked from commit 73cb57910835ef46d43bb92309def186e38534cf) --- 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 3630d78bcd..0067fa7446 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 @@ -257,6 +261,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; @@ -315,6 +321,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; @@ -353,7 +367,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. */ @@ -364,6 +384,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++) @@ -374,6 +395,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; @@ -399,6 +435,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 f29aee87ec84f355864ba880358effcd32797c84 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 (cherry picked from commit 4dacf0b1ad716747310e56188a0d283ee47ebee0) --- 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 b44b3b80d4..7603ac455c 100644 --- a/configure.ac +++ b/configure.ac @@ -891,7 +891,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=$?