From 4f7e9bd33637b38eee6c2bfb4340166a5e218486 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 30 Jul 2014 17:20:16 -0400 Subject: [PATCH 1/6] libnm-glib: compile test-networkmanager-service.py path into test binaries Rather than passing the path to the test service on the command line, compile it into the test programs. (Among other things, this makes it easier to run the test directly from the command line.) --- libnm-glib/tests/Makefile.am | 11 +++++------ libnm-glib/tests/test-nm-client.c | 13 ++----------- libnm-glib/tests/test-remote-settings-client.c | 9 +++------ 3 files changed, 10 insertions(+), 23 deletions(-) diff --git a/libnm-glib/tests/Makefile.am b/libnm-glib/tests/Makefile.am index 86732f95f0..2723aa4479 100644 --- a/libnm-glib/tests/Makefile.am +++ b/libnm-glib/tests/Makefile.am @@ -6,6 +6,7 @@ AM_CPPFLAGS = \ -I$(top_builddir)/libnm-util \ -I$(top_srcdir)/libnm-glib \ -DNM_VERSION_MAX_ALLOWED=NM_VERSION_NEXT_STABLE \ + -DTEST_NM_SERVICE=\"$(abs_top_srcdir)/tools/test-networkmanager-service.py\" \ $(GLIB_CFLAGS) \ $(DBUS_CFLAGS) @@ -35,15 +36,13 @@ test_remote_settings_client_LDADD = \ ########################################### -TEST_NM_SERVICE = $(top_srcdir)/tools/test-networkmanager-service.py - check-local: test-nm-client test-remote-settings-client if test -z "$$DBUS_SESSION_BUS_ADDRESS" ; then \ - dbus-launch --exit-with-session $(abs_builddir)/test-nm-client $(abs_srcdir) $(TEST_NM_SERVICE); \ - dbus-launch --exit-with-session $(abs_builddir)/test-remote-settings-client $(abs_srcdir) $(TEST_NM_SERVICE); \ + dbus-launch --exit-with-session $(abs_builddir)/test-nm-client; \ + dbus-launch --exit-with-session $(abs_builddir)/test-remote-settings-client; \ else \ - $(abs_builddir)/test-nm-client $(abs_srcdir) $(TEST_NM_SERVICE); \ - $(abs_builddir)/test-remote-settings-client $(abs_srcdir) $(TEST_NM_SERVICE); \ + $(abs_builddir)/test-nm-client; \ + $(abs_builddir)/test-remote-settings-client; \ fi; endif diff --git a/libnm-glib/tests/test-nm-client.c b/libnm-glib/tests/test-nm-client.c index 572e1fdc6b..b1874045a2 100644 --- a/libnm-glib/tests/test-nm-client.c +++ b/libnm-glib/tests/test-nm-client.c @@ -33,9 +33,6 @@ #include "nm-device-wimax.h" #include "nm-glib-compat.h" -static const char *fake_path; -static const char *fake_bin; -static const char *fake_exec; static GMainLoop *loop = NULL; /*******************************************************************/ @@ -129,7 +126,7 @@ service_init (void) { DBusGConnection *bus; ServiceInfo *sinfo; - const char *args[2] = { fake_exec, NULL }; + const char *args[2] = { TEST_NM_SERVICE, NULL }; GError *error = NULL; int i = 100; @@ -142,7 +139,7 @@ service_init (void) sinfo->bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL); test_assert (sinfo->bus); - if (!g_spawn_async (fake_path, (char **) args, NULL, 0, NULL, NULL, &sinfo->pid, &error)) + if (!g_spawn_async (NULL, (char **) args, NULL, 0, NULL, NULL, &sinfo->pid, &error)) test_assert_no_error (error); /* Wait until the service is registered on the bus */ @@ -924,18 +921,12 @@ test_devices_array (void) int main (int argc, char **argv) { - g_assert (argc == 3); - #if !GLIB_CHECK_VERSION (2, 35, 0) g_type_init (); #endif g_test_init (&argc, &argv, NULL); - fake_path = argv[1]; - fake_bin = argv[2]; - fake_exec = g_strdup_printf ("%s/%s", argv[1], argv[2]); - loop = g_main_loop_new (NULL, FALSE); g_test_add_func ("/libnm-glib/device-added", test_device_added); diff --git a/libnm-glib/tests/test-remote-settings-client.c b/libnm-glib/tests/test-remote-settings-client.c index 9246cbbc82..c6912276ed 100644 --- a/libnm-glib/tests/test-remote-settings-client.c +++ b/libnm-glib/tests/test-remote-settings-client.c @@ -358,13 +358,11 @@ test_remove_connection (void) int main (int argc, char **argv) { - char *service_argv[3] = { NULL, NULL, NULL }; + char *service_argv[2] = { TEST_NM_SERVICE, NULL }; int ret; GError *error = NULL; int i = 100; - g_assert (argc == 3); - #if !GLIB_CHECK_VERSION (2, 35, 0) g_type_init (); #endif @@ -377,9 +375,8 @@ main (int argc, char **argv) g_assert (error == NULL); } - service_argv[0] = g_strdup_printf ("%s/%s", argv[1], argv[2]); - if (!g_spawn_async (argv[1], service_argv, NULL, 0, NULL, NULL, &spid, &error)) { - g_warning ("Error spawning %s: %s", argv[2], error->message); + if (!g_spawn_async (NULL, service_argv, NULL, 0, NULL, NULL, &spid, &error)) { + g_warning ("Error spawning %s: %s", TEST_NM_SERVICE, error->message); g_assert (error == NULL); } From ade4f2e84e0fa3df47d8db4140652ca50b8e9fa8 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Wed, 30 Jul 2014 17:32:35 -0400 Subject: [PATCH 2/6] libnm-glib: use automake test infrastructure for test programs Use "TESTS = tests-nm-client test-remote-settings-client" rather than overriding "check-local". Add a script "libnm-test-launch.sh" that will handle redirecting the test via dbus-launch if needed. --- libnm-glib/tests/Makefile.am | 15 ++++++--------- libnm-glib/tests/libnm-glib-test-launch.sh | 7 +++++++ 2 files changed, 13 insertions(+), 9 deletions(-) create mode 100755 libnm-glib/tests/libnm-glib-test-launch.sh diff --git a/libnm-glib/tests/Makefile.am b/libnm-glib/tests/Makefile.am index 2723aa4479..dc5a7b8771 100644 --- a/libnm-glib/tests/Makefile.am +++ b/libnm-glib/tests/Makefile.am @@ -10,7 +10,9 @@ AM_CPPFLAGS = \ $(GLIB_CFLAGS) \ $(DBUS_CFLAGS) -noinst_PROGRAMS = test-nm-client test-remote-settings-client +noinst_PROGRAMS = $(TESTS) + +TESTS = test-nm-client test-remote-settings-client ####### NMClient and non-settings tests ####### @@ -36,13 +38,8 @@ test_remote_settings_client_LDADD = \ ########################################### -check-local: test-nm-client test-remote-settings-client - if test -z "$$DBUS_SESSION_BUS_ADDRESS" ; then \ - dbus-launch --exit-with-session $(abs_builddir)/test-nm-client; \ - dbus-launch --exit-with-session $(abs_builddir)/test-remote-settings-client; \ - else \ - $(abs_builddir)/test-nm-client; \ - $(abs_builddir)/test-remote-settings-client; \ - fi; +TESTS_ENVIRONMENT = $(srcdir)/libnm-glib-test-launch.sh endif + +EXTRA_DIST = libnm-glib-test-launch.sh diff --git a/libnm-glib/tests/libnm-glib-test-launch.sh b/libnm-glib/tests/libnm-glib-test-launch.sh new file mode 100755 index 0000000000..1db656ad06 --- /dev/null +++ b/libnm-glib/tests/libnm-glib-test-launch.sh @@ -0,0 +1,7 @@ +#!/bin/sh + +if [ -z "$DBUS_SESSION_BUS_ADDRESS" ]; then + exec dbus-launch --exit-with-session "$@" +else + exec "$@" +fi From bd8a7f74b1f7d9b1712741797096bf97ef5df9aa Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 31 Jul 2014 13:32:10 -0400 Subject: [PATCH 3/6] libnm-glib: split out test service launching code from tests --- libnm-glib/tests/Makefile.am | 4 + libnm-glib/tests/common.c | 113 +++++++++ libnm-glib/tests/common.h | 30 +++ libnm-glib/tests/test-nm-client.c | 225 ++++++------------ .../tests/test-remote-settings-client.c | 31 +-- 5 files changed, 224 insertions(+), 179 deletions(-) create mode 100644 libnm-glib/tests/common.c create mode 100644 libnm-glib/tests/common.h diff --git a/libnm-glib/tests/Makefile.am b/libnm-glib/tests/Makefile.am index dc5a7b8771..7b8f3627e2 100644 --- a/libnm-glib/tests/Makefile.am +++ b/libnm-glib/tests/Makefile.am @@ -17,6 +17,8 @@ TESTS = test-nm-client test-remote-settings-client ####### NMClient and non-settings tests ####### test_nm_client_SOURCES = \ + common.c \ + common.h \ test-nm-client.c test_nm_client_LDADD = \ @@ -28,6 +30,8 @@ test_nm_client_LDADD = \ ####### remote settings client test ####### test_remote_settings_client_SOURCES = \ + common.c \ + common.h \ test-remote-settings-client.c test_remote_settings_client_LDADD = \ diff --git a/libnm-glib/tests/common.c b/libnm-glib/tests/common.c new file mode 100644 index 0000000000..0cc5da0130 --- /dev/null +++ b/libnm-glib/tests/common.c @@ -0,0 +1,113 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ +/* + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2, or (at your option) + * any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Copyright 2010 - 2014 Red Hat, Inc. + * + */ + +#include +#include +#include + +#include "NetworkManager.h" + +#include "common.h" + +static gboolean +name_exists (GDBusConnection *c, const char *name) +{ + GVariant *reply; + gboolean exists = FALSE; + + reply = g_dbus_connection_call_sync (c, + DBUS_SERVICE_DBUS, + DBUS_PATH_DBUS, + DBUS_INTERFACE_DBUS, + "GetNameOwner", + g_variant_new ("(s)", name), + NULL, + G_DBUS_CALL_FLAGS_NO_AUTO_START, + -1, + NULL, + NULL); + if (reply != NULL) { + exists = TRUE; + g_variant_unref (reply); + } + + return exists; +} + +NMTestServiceInfo * +nm_test_service_init (void) +{ + NMTestServiceInfo *info; + const char *args[2] = { TEST_NM_SERVICE, NULL }; + GError *error = NULL; + int i; + + info = g_malloc0 (sizeof (*info)); + + info->bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, &error); + g_assert_no_error (error); + + g_spawn_async (NULL, (char **) args, NULL, 0, NULL, NULL, &info->pid, &error); + g_assert_no_error (error); + + /* Wait until the service is registered on the bus */ + for (i = 100; i > 0; i--) { + if (name_exists (info->bus, "org.freedesktop.NetworkManager")) + break; + g_usleep (G_USEC_PER_SEC / 50); + } + g_assert (i > 0); + + /* Grab a proxy to our fake NM service to trigger tests */ + info->proxy = g_dbus_proxy_new_sync (info->bus, + G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES | + G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS | + G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, + NULL, + NM_DBUS_SERVICE, + NM_DBUS_PATH, + "org.freedesktop.NetworkManager.LibnmGlibTest", + NULL, &error); + g_assert_no_error (error); + + return info; +} + +void +nm_test_service_cleanup (NMTestServiceInfo *info) +{ + int i; + + g_object_unref (info->proxy); + kill (info->pid, SIGTERM); + + /* Wait until the bus notices the service is gone */ + for (i = 100; i > 0; i--) { + if (!name_exists (info->bus, "org.freedesktop.NetworkManager")) + break; + g_usleep (G_USEC_PER_SEC / 50); + } + g_assert (i > 0); + + g_object_unref (info->bus); + + memset (info, 0, sizeof (*info)); + g_free (info); +} diff --git a/libnm-glib/tests/common.h b/libnm-glib/tests/common.h new file mode 100644 index 0000000000..a35274a41d --- /dev/null +++ b/libnm-glib/tests/common.h @@ -0,0 +1,30 @@ +/* -*- Mode: C; tab-width: 4; indent-tabs-mode: t; c-basic-offset: 4 -*- */ +/* + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the + * Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, + * Boston, MA 02110-1301 USA. + * + * Copyright 2014 Red Hat, Inc. + */ + +#include + +typedef struct { + GDBusConnection *bus; + GDBusProxy *proxy; + GPid pid; +} NMTestServiceInfo; + +NMTestServiceInfo *nm_test_service_init (void); +void nm_test_service_cleanup (NMTestServiceInfo *info); diff --git a/libnm-glib/tests/test-nm-client.c b/libnm-glib/tests/test-nm-client.c index b1874045a2..e34d12516a 100644 --- a/libnm-glib/tests/test-nm-client.c +++ b/libnm-glib/tests/test-nm-client.c @@ -33,161 +33,65 @@ #include "nm-device-wimax.h" #include "nm-glib-compat.h" +#include "common.h" + static GMainLoop *loop = NULL; +static NMTestServiceInfo *sinfo; /*******************************************************************/ -typedef struct { - GDBusConnection *bus; - GDBusProxy *proxy; - GPid pid; - NMClient *client; -} ServiceInfo; - #define test_assert(condition) \ do { \ if (!G_LIKELY (condition)) \ - service_cleanup (); \ + nm_test_service_cleanup (sinfo); \ g_assert (condition); \ } while (0) #define test_assert_cmpint(a, b, c) \ do { \ if (!G_LIKELY (a b c)) \ - service_cleanup (); \ + nm_test_service_cleanup (sinfo); \ g_assert_cmpint (a, b, c); \ } while (0) #define test_assert_cmpstr(a, b, c) \ do { \ if (!G_LIKELY (g_str_hash (a) b g_str_hash (c))) \ - service_cleanup (); \ + nm_test_service_cleanup (sinfo); \ g_assert_cmpstr (a, b, c); \ } while (0) #define test_assert_no_error(e) \ do { \ if (G_UNLIKELY (e)) \ - service_cleanup (); \ + nm_test_service_cleanup (sinfo); \ g_assert_no_error (e); \ } while (0) -static ServiceInfo * sinfo_static = NULL; - -static void -service_cleanup (void) -{ - ServiceInfo *info = sinfo_static; - - sinfo_static = NULL; - - if (info) { - if (info->proxy) - g_object_unref (info->proxy); - if (info->bus) - g_object_unref (info->bus); - if (info->client) - g_object_unref (info->client); - if (info->pid) - kill (info->pid, SIGTERM); - memset (info, 0, sizeof (*info)); - g_free (info); - } else - g_assert_not_reached (); -} - -static gboolean -name_exists (GDBusConnection *c, const char *name) -{ - GVariant *reply; - gboolean exists = FALSE; - - reply = g_dbus_connection_call_sync (c, - DBUS_SERVICE_DBUS, - DBUS_PATH_DBUS, - DBUS_INTERFACE_DBUS, - "GetNameOwner", - g_variant_new ("(s)", name), - NULL, - G_DBUS_CALL_FLAGS_NO_AUTO_START, - -1, - NULL, - NULL); - if (reply != NULL) { - exists = TRUE; - g_variant_unref (reply); - } - - return exists; -} - -static ServiceInfo * -service_init (void) +static NMClient * +test_client_new (void) { + NMClient *client; DBusGConnection *bus; - ServiceInfo *sinfo; - const char *args[2] = { TEST_NM_SERVICE, NULL }; GError *error = NULL; - int i = 100; - - g_assert (!sinfo_static); - - sinfo = g_malloc0 (sizeof (*sinfo)); - - sinfo_static = sinfo; - - sinfo->bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL); - test_assert (sinfo->bus); - - if (!g_spawn_async (NULL, (char **) args, NULL, 0, NULL, NULL, &sinfo->pid, &error)) - test_assert_no_error (error); - - /* Wait until the service is registered on the bus */ - while (i > 0) { - g_usleep (G_USEC_PER_SEC / 50); - if (name_exists (sinfo->bus, "org.freedesktop.NetworkManager")) - break; - i--; - } - test_assert (i > 0); - - /* Grab a proxy to our fake NM service to trigger tests */ - sinfo->proxy = g_dbus_proxy_new_sync (sinfo->bus, - G_DBUS_PROXY_FLAGS_DO_NOT_LOAD_PROPERTIES | - G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS | - G_DBUS_PROXY_FLAGS_DO_NOT_AUTO_START, - NULL, - NM_DBUS_SERVICE, - NM_DBUS_PATH, - "org.freedesktop.NetworkManager.LibnmGlibTest", - NULL, NULL); - test_assert (sinfo->proxy); bus = dbus_g_bus_get (DBUS_BUS_SESSION, &error); g_assert_no_error (error); - sinfo->client = g_object_new (NM_TYPE_CLIENT, - NM_OBJECT_DBUS_CONNECTION, bus, - NM_OBJECT_DBUS_PATH, NM_DBUS_PATH, - NULL); - test_assert (sinfo->client != NULL); + client = g_object_new (NM_TYPE_CLIENT, + NM_OBJECT_DBUS_CONNECTION, bus, + NM_OBJECT_DBUS_PATH, NM_DBUS_PATH, + NULL); + test_assert (client != NULL); dbus_g_connection_unref (bus); - g_initable_init (G_INITABLE (sinfo->client), NULL, &error); + + g_initable_init (G_INITABLE (client), NULL, &error); g_assert_no_error (error); - return sinfo; + return client; } -static ServiceInfo * -service_get (void) -{ - g_assert (sinfo_static); - return sinfo_static; -} - -#define _sinfo (service_get ()) - /*******************************************************************/ static gboolean @@ -203,7 +107,7 @@ add_device (const char *method, const char *ifname, char **out_path) GError *error = NULL; GVariant *ret; - ret = g_dbus_proxy_call_sync (_sinfo->proxy, + ret = g_dbus_proxy_call_sync (sinfo->proxy, method, g_variant_new ("(s)", ifname), G_DBUS_CALL_FLAGS_NO_AUTO_START, @@ -275,25 +179,27 @@ devices_notify_cb (NMClient *c, static void test_device_added (void) { + NMClient *client; const GPtrArray *devices; NMDevice *device; DeviceAddedInfo info = { loop, FALSE, FALSE, 0, 0 }; - service_init (); + sinfo = nm_test_service_init (); + client = test_client_new (); - devices = nm_client_get_devices (_sinfo->client); + devices = nm_client_get_devices (client); test_assert (devices == NULL); /* Tell the test service to add a new device */ add_device ("AddWiredDevice", "eth0", NULL); - g_signal_connect (_sinfo->client, + g_signal_connect (client, "device-added", (GCallback) device_added_cb, &info); info.quit_count++; - g_signal_connect (_sinfo->client, + g_signal_connect (client, "notify::devices", (GCallback) devices_notify_cb, &info); @@ -306,10 +212,10 @@ test_device_added (void) test_assert (info.signaled); test_assert (info.notified); - g_signal_handlers_disconnect_by_func (_sinfo->client, device_added_cb, &info); - g_signal_handlers_disconnect_by_func (_sinfo->client, devices_notify_cb, &info); + g_signal_handlers_disconnect_by_func (client, device_added_cb, &info); + g_signal_handlers_disconnect_by_func (client, devices_notify_cb, &info); - devices = nm_client_get_devices (_sinfo->client); + devices = nm_client_get_devices (client); test_assert (devices); test_assert_cmpint (devices->len, ==, 1); @@ -317,7 +223,8 @@ test_device_added (void) test_assert (device); test_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); - service_cleanup (); + g_object_unref (client); + g_clear_pointer (&sinfo, nm_test_service_cleanup); } /*******************************************************************/ @@ -427,19 +334,21 @@ wifi_ap_remove_notify_cb (NMDeviceWifi *w, static void test_wifi_ap_added_removed (void) { + NMClient *client; NMDeviceWifi *wifi; WifiApInfo info = { loop, FALSE, FALSE, 0, 0 }; GVariant *ret; GError *error = NULL; char *expected_path = NULL; - service_init (); + sinfo = nm_test_service_init (); + client = test_client_new (); /*************************************/ /* Add the wifi device */ add_device ("AddWifiDevice", "wlan0", NULL); - g_signal_connect (_sinfo->client, + g_signal_connect (client, "device-added", (GCallback) wifi_device_added_cb, &info); @@ -450,9 +359,9 @@ test_wifi_ap_added_removed (void) g_main_loop_run (loop); test_assert (info.found); - g_signal_handlers_disconnect_by_func (_sinfo->client, wifi_device_added_cb, &info); + g_signal_handlers_disconnect_by_func (client, wifi_device_added_cb, &info); - wifi = (NMDeviceWifi *) nm_client_get_device_by_iface (_sinfo->client, "wlan0"); + wifi = (NMDeviceWifi *) nm_client_get_device_by_iface (client, "wlan0"); test_assert (NM_IS_DEVICE_WIFI (wifi)); /*************************************/ @@ -461,7 +370,7 @@ test_wifi_ap_added_removed (void) info.notified = FALSE; info.quit_id = 0; - ret = g_dbus_proxy_call_sync (_sinfo->proxy, + ret = g_dbus_proxy_call_sync (sinfo->proxy, "AddWifiAp", g_variant_new ("(sss)", "wlan0", "test-ap", expected_bssid), G_DBUS_CALL_FLAGS_NO_AUTO_START, @@ -503,7 +412,7 @@ test_wifi_ap_added_removed (void) info.notified = FALSE; info.quit_id = 0; - ret = g_dbus_proxy_call_sync (_sinfo->proxy, + ret = g_dbus_proxy_call_sync (sinfo->proxy, "RemoveWifiAp", g_variant_new ("(so)", "wlan0", expected_path), G_DBUS_CALL_FLAGS_NO_AUTO_START, @@ -536,7 +445,9 @@ test_wifi_ap_added_removed (void) g_free (info.ap_path); g_free (expected_path); - service_cleanup (); + + g_object_unref (client); + g_clear_pointer (&sinfo, nm_test_service_cleanup); } /*******************************************************************/ @@ -646,19 +557,21 @@ wimax_nsp_remove_notify_cb (NMDeviceWimax *w, static void test_wimax_nsp_added_removed (void) { + NMClient *client; NMDeviceWimax *wimax; WimaxNspInfo info = { loop, FALSE, FALSE, 0, 0 }; GVariant *ret; GError *error = NULL; char *expected_path = NULL; - service_init (); + sinfo = nm_test_service_init (); + client = test_client_new (); /*************************************/ /* Add the wimax device */ add_device ("AddWimaxDevice", "wmx0", NULL); - g_signal_connect (_sinfo->client, + g_signal_connect (client, "device-added", (GCallback) wimax_device_added_cb, &info); @@ -669,9 +582,9 @@ test_wimax_nsp_added_removed (void) g_main_loop_run (loop); test_assert (info.found); - g_signal_handlers_disconnect_by_func (_sinfo->client, wimax_device_added_cb, &info); + g_signal_handlers_disconnect_by_func (client, wimax_device_added_cb, &info); - wimax = (NMDeviceWimax *) nm_client_get_device_by_iface (_sinfo->client, "wmx0"); + wimax = (NMDeviceWimax *) nm_client_get_device_by_iface (client, "wmx0"); test_assert (NM_IS_DEVICE_WIMAX (wimax)); /*************************************/ @@ -680,7 +593,7 @@ test_wimax_nsp_added_removed (void) info.notified = FALSE; info.quit_id = 0; - ret = g_dbus_proxy_call_sync (_sinfo->proxy, + ret = g_dbus_proxy_call_sync (sinfo->proxy, "AddWimaxNsp", g_variant_new ("(ss)", "wmx0", expected_nsp_name), G_DBUS_CALL_FLAGS_NO_AUTO_START, @@ -722,7 +635,7 @@ test_wimax_nsp_added_removed (void) info.notified = FALSE; info.quit_id = 0; - ret = g_dbus_proxy_call_sync (_sinfo->proxy, + ret = g_dbus_proxy_call_sync (sinfo->proxy, "RemoveWimaxNsp", g_variant_new ("(so)", "wmx0", expected_path), G_DBUS_CALL_FLAGS_NO_AUTO_START, @@ -755,7 +668,9 @@ test_wimax_nsp_added_removed (void) g_free (info.nsp_path); g_free (expected_path); - service_cleanup (); + + g_object_unref (client); + g_clear_pointer (&sinfo, nm_test_service_cleanup); } /*******************************************************************/ @@ -825,6 +740,7 @@ da_devices_notify_cb (NMClient *c, static void test_devices_array (void) { + NMClient *client; DaInfo info = { loop }; char *paths[3] = { NULL, NULL, NULL }; NMDevice *device; @@ -832,7 +748,8 @@ test_devices_array (void) GError *error = NULL; GVariant *ret; - service_init (); + sinfo = nm_test_service_init (); + client = test_client_new (); /*************************************/ /* Add some devices */ @@ -841,7 +758,7 @@ test_devices_array (void) add_device ("AddWiredDevice", "eth1", &paths[2]); info.quit_count = 3; - g_signal_connect (_sinfo->client, + g_signal_connect (client, "device-added", (GCallback) da_device_added_cb, &info); @@ -851,25 +768,25 @@ test_devices_array (void) g_main_loop_run (loop); test_assert_cmpint (info.quit_count, ==, 0); - g_signal_handlers_disconnect_by_func (_sinfo->client, da_device_added_cb, &info); + g_signal_handlers_disconnect_by_func (client, da_device_added_cb, &info); /* Ensure the devices now exist */ - devices = nm_client_get_devices (_sinfo->client); + devices = nm_client_get_devices (client); test_assert (devices); test_assert_cmpint (devices->len, ==, 3); - device = nm_client_get_device_by_iface (_sinfo->client, "wlan0"); + device = nm_client_get_device_by_iface (client, "wlan0"); test_assert (NM_IS_DEVICE_WIFI (device)); - device = nm_client_get_device_by_iface (_sinfo->client, "eth0"); + device = nm_client_get_device_by_iface (client, "eth0"); test_assert (NM_IS_DEVICE_ETHERNET (device)); - device = nm_client_get_device_by_iface (_sinfo->client, "eth1"); + device = nm_client_get_device_by_iface (client, "eth1"); test_assert (NM_IS_DEVICE_ETHERNET (device)); /********************************/ /* Now remove the device in the middle */ - ret = g_dbus_proxy_call_sync (_sinfo->proxy, + ret = g_dbus_proxy_call_sync (sinfo->proxy, "RemoveDevice", g_variant_new ("(o)", paths[1]), G_DBUS_CALL_FLAGS_NO_AUTO_START, @@ -880,12 +797,12 @@ test_devices_array (void) test_assert (ret); g_variant_unref (ret); - g_signal_connect (_sinfo->client, + g_signal_connect (client, "device-removed", (GCallback) da_device_removed_cb, &info); - g_signal_connect (_sinfo->client, + g_signal_connect (client, "notify::devices", (GCallback) da_devices_notify_cb, &info); @@ -896,24 +813,26 @@ test_devices_array (void) g_main_loop_run (loop); test_assert_cmpint (info.quit_count, ==, 0); - g_signal_handlers_disconnect_by_func (_sinfo->client, da_device_removed_cb, &info); - g_signal_handlers_disconnect_by_func (_sinfo->client, da_devices_notify_cb, &info); + g_signal_handlers_disconnect_by_func (client, da_device_removed_cb, &info); + g_signal_handlers_disconnect_by_func (client, da_devices_notify_cb, &info); /* Ensure only two are left */ - devices = nm_client_get_devices (_sinfo->client); + devices = nm_client_get_devices (client); test_assert (devices); test_assert_cmpint (devices->len, ==, 2); - device = nm_client_get_device_by_iface (_sinfo->client, "wlan0"); + device = nm_client_get_device_by_iface (client, "wlan0"); test_assert (NM_IS_DEVICE_WIFI (device)); - device = nm_client_get_device_by_iface (_sinfo->client, "eth1"); + device = nm_client_get_device_by_iface (client, "eth1"); test_assert (NM_IS_DEVICE_ETHERNET (device)); g_free (paths[0]); g_free (paths[1]); g_free (paths[2]); - service_cleanup (); + + g_object_unref (client); + g_clear_pointer (&sinfo, nm_test_service_cleanup); } /*******************************************************************/ diff --git a/libnm-glib/tests/test-remote-settings-client.c b/libnm-glib/tests/test-remote-settings-client.c index c6912276ed..7b4bdce36b 100644 --- a/libnm-glib/tests/test-remote-settings-client.c +++ b/libnm-glib/tests/test-remote-settings-client.c @@ -33,26 +33,21 @@ #include #include "nm-remote-settings.h" +#include "common.h" -static GPid spid = 0; +static NMTestServiceInfo *sinfo; static NMRemoteSettings *settings = NULL; DBusGConnection *bus = NULL; NMRemoteConnection *remote = NULL; /*******************************************************************/ -static void -cleanup (void) -{ - kill (spid, SIGTERM); -} - #define test_assert(condition) \ do { \ gboolean _condition = !!( condition ); \ \ if (G_UNLIKELY (!_condition)) { \ - cleanup (); \ + nm_test_service_cleanup (sinfo); \ g_assert (!"test_assert() failed for" # condition); \ } \ } while (0) @@ -358,10 +353,8 @@ test_remove_connection (void) int main (int argc, char **argv) { - char *service_argv[2] = { TEST_NM_SERVICE, NULL }; int ret; GError *error = NULL; - int i = 100; #if !GLIB_CHECK_VERSION (2, 35, 0) g_type_init (); @@ -375,21 +368,7 @@ main (int argc, char **argv) g_assert (error == NULL); } - if (!g_spawn_async (NULL, service_argv, NULL, 0, NULL, NULL, &spid, &error)) { - g_warning ("Error spawning %s: %s", TEST_NM_SERVICE, error->message); - g_assert (error == NULL); - } - - /* Wait until the service is registered on the bus */ - while (i > 0) { - g_usleep (G_USEC_PER_SEC / 50); - if (dbus_bus_name_has_owner (dbus_g_connection_get_connection (bus), - "org.freedesktop.NetworkManager", - NULL)) - break; - i--; - } - test_assert (i > 0); + sinfo = nm_test_service_init (); settings = nm_remote_settings_new (bus); test_assert (settings != NULL); @@ -401,7 +380,7 @@ main (int argc, char **argv) ret = g_test_run (); - cleanup (); + nm_test_service_cleanup (sinfo); g_object_unref (settings); dbus_g_connection_unref (bus); From 08b91199fb95d9178feaeaa58381847332f22ad8 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 31 Jul 2014 14:00:22 -0400 Subject: [PATCH 4/6] libnm-glib: make test-networkmanager-service.py automatically exit with its parent test-nm-client.c and test-remote-settings-client.c were using their own assertion macros so they could kill the test service on assertion failure. Except that some new code didn't get the memo and used the g_assert* macros. Not to mention that sometimes the tests would crash outside of an assertion macro. We can make test-networkmanager-service.py notice that its parent has crashed by opening a pipe between them and taking advantage of the fact that the pipe will be automatically closed if the parent crashes. So then test-networkmanager-service.py just has to watch for that, and exit if the pipe closes. Then that lets us drop the test_assert* macros and just use g_assert* instead. --- libnm-glib/tests/common.c | 8 +- libnm-glib/tests/common.h | 1 + libnm-glib/tests/test-nm-client.c | 188 ++++++++---------- .../tests/test-remote-settings-client.c | 79 +++----- tools/test-networkmanager-service.py | 9 +- 5 files changed, 126 insertions(+), 159 deletions(-) diff --git a/libnm-glib/tests/common.c b/libnm-glib/tests/common.c index 0cc5da0130..0dbdac54c2 100644 --- a/libnm-glib/tests/common.c +++ b/libnm-glib/tests/common.c @@ -64,7 +64,12 @@ nm_test_service_init (void) info->bus = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, &error); g_assert_no_error (error); - g_spawn_async (NULL, (char **) args, NULL, 0, NULL, NULL, &info->pid, &error); + /* Spawn the test service. info->keepalive_fd will be a pipe to the service's + * stdin; if it closes, the service will exit immediately. We use this to + * make sure the service exits if the test program crashes. + */ + g_spawn_async_with_pipes (NULL, (char **) args, NULL, 0, NULL, NULL, + &info->pid, &info->keepalive_fd, NULL, NULL, &error); g_assert_no_error (error); /* Wait until the service is registered on the bus */ @@ -107,6 +112,7 @@ nm_test_service_cleanup (NMTestServiceInfo *info) g_assert (i > 0); g_object_unref (info->bus); + close (info->keepalive_fd); memset (info, 0, sizeof (*info)); g_free (info); diff --git a/libnm-glib/tests/common.h b/libnm-glib/tests/common.h index a35274a41d..7c49d2532b 100644 --- a/libnm-glib/tests/common.h +++ b/libnm-glib/tests/common.h @@ -24,6 +24,7 @@ typedef struct { GDBusConnection *bus; GDBusProxy *proxy; GPid pid; + int keepalive_fd; } NMTestServiceInfo; NMTestServiceInfo *nm_test_service_init (void); diff --git a/libnm-glib/tests/test-nm-client.c b/libnm-glib/tests/test-nm-client.c index e34d12516a..dc6126451b 100644 --- a/libnm-glib/tests/test-nm-client.c +++ b/libnm-glib/tests/test-nm-client.c @@ -40,34 +40,6 @@ static NMTestServiceInfo *sinfo; /*******************************************************************/ -#define test_assert(condition) \ -do { \ - if (!G_LIKELY (condition)) \ - nm_test_service_cleanup (sinfo); \ - g_assert (condition); \ -} while (0) - -#define test_assert_cmpint(a, b, c) \ -do { \ - if (!G_LIKELY (a b c)) \ - nm_test_service_cleanup (sinfo); \ - g_assert_cmpint (a, b, c); \ -} while (0) - -#define test_assert_cmpstr(a, b, c) \ -do { \ - if (!G_LIKELY (g_str_hash (a) b g_str_hash (c))) \ - nm_test_service_cleanup (sinfo); \ - g_assert_cmpstr (a, b, c); \ -} while (0) - -#define test_assert_no_error(e) \ -do { \ - if (G_UNLIKELY (e)) \ - nm_test_service_cleanup (sinfo); \ - g_assert_no_error (e); \ -} while (0) - static NMClient * test_client_new (void) { @@ -82,7 +54,7 @@ test_client_new (void) NM_OBJECT_DBUS_CONNECTION, bus, NM_OBJECT_DBUS_PATH, NM_DBUS_PATH, NULL); - test_assert (client != NULL); + g_assert (client != NULL); dbus_g_connection_unref (bus); @@ -114,9 +86,9 @@ add_device (const char *method, const char *ifname, char **out_path) 3000, NULL, &error); - test_assert_no_error (error); - test_assert (ret); - test_assert_cmpstr (g_variant_get_type_string (ret), ==, "(o)"); + g_assert_no_error (error); + g_assert (ret); + g_assert_cmpstr (g_variant_get_type_string (ret), ==, "(o)"); if (out_path) g_variant_get (ret, "(o)", out_path); g_variant_unref (ret); @@ -149,8 +121,8 @@ device_added_cb (NMClient *c, NMDevice *device, DeviceAddedInfo *info) { - test_assert (device); - test_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); + g_assert (device); + g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); info->signaled = TRUE; device_add_check_quit (info); } @@ -164,12 +136,12 @@ devices_notify_cb (NMClient *c, NMDevice *device; devices = nm_client_get_devices (c); - test_assert (devices); - test_assert_cmpint (devices->len, ==, 1); + g_assert (devices); + g_assert_cmpint (devices->len, ==, 1); device = g_ptr_array_index (devices, 0); - test_assert (device); - test_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); + g_assert (device); + g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); info->notified = TRUE; @@ -188,7 +160,7 @@ test_device_added (void) client = test_client_new (); devices = nm_client_get_devices (client); - test_assert (devices == NULL); + g_assert (devices == NULL); /* Tell the test service to add a new device */ add_device ("AddWiredDevice", "eth0", NULL); @@ -209,19 +181,19 @@ test_device_added (void) info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); g_main_loop_run (loop); - test_assert (info.signaled); - test_assert (info.notified); + g_assert (info.signaled); + g_assert (info.notified); g_signal_handlers_disconnect_by_func (client, device_added_cb, &info); g_signal_handlers_disconnect_by_func (client, devices_notify_cb, &info); devices = nm_client_get_devices (client); - test_assert (devices); - test_assert_cmpint (devices->len, ==, 1); + g_assert (devices); + g_assert_cmpint (devices->len, ==, 1); device = g_ptr_array_index (devices, 0); - test_assert (device); - test_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); + g_assert (device); + g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); g_object_unref (client); g_clear_pointer (&sinfo, nm_test_service_cleanup); @@ -257,7 +229,7 @@ wifi_device_added_cb (NMClient *c, NMDevice *device, WifiApInfo *info) { - test_assert_cmpstr (nm_device_get_iface (device), ==, "wlan0"); + g_assert_cmpstr (nm_device_get_iface (device), ==, "wlan0"); info->found = TRUE; wifi_check_quit (info); } @@ -266,7 +238,7 @@ static void got_ap_path (WifiApInfo *info, const char *path) { if (info->ap_path) - test_assert_cmpstr (info->ap_path, ==, path); + g_assert_cmpstr (info->ap_path, ==, path); else info->ap_path = g_strdup (path); } @@ -276,8 +248,8 @@ wifi_ap_added_cb (NMDeviceWifi *w, NMAccessPoint *ap, WifiApInfo *info) { - test_assert (ap); - test_assert_cmpstr (nm_access_point_get_bssid (ap), ==, expected_bssid); + g_assert (ap); + g_assert_cmpstr (nm_access_point_get_bssid (ap), ==, expected_bssid); got_ap_path (info, nm_object_get_path (NM_OBJECT (ap))); info->signaled = TRUE; @@ -293,12 +265,12 @@ wifi_ap_add_notify_cb (NMDeviceWifi *w, NMAccessPoint *ap; aps = nm_device_wifi_get_access_points (w); - test_assert (aps); - test_assert_cmpint (aps->len, ==, 1); + g_assert (aps); + g_assert_cmpint (aps->len, ==, 1); ap = g_ptr_array_index (aps, 0); - test_assert (ap); - test_assert_cmpstr (nm_access_point_get_bssid (ap), ==, "66:55:44:33:22:11"); + g_assert (ap); + g_assert_cmpstr (nm_access_point_get_bssid (ap), ==, "66:55:44:33:22:11"); got_ap_path (info, nm_object_get_path (NM_OBJECT (ap))); info->notified = TRUE; @@ -310,8 +282,8 @@ wifi_ap_removed_cb (NMDeviceWifi *w, NMAccessPoint *ap, WifiApInfo *info) { - test_assert (ap); - test_assert_cmpstr (info->ap_path, ==, nm_object_get_path (NM_OBJECT (ap))); + g_assert (ap); + g_assert_cmpstr (info->ap_path, ==, nm_object_get_path (NM_OBJECT (ap))); info->signaled = TRUE; wifi_check_quit (info); @@ -325,7 +297,7 @@ wifi_ap_remove_notify_cb (NMDeviceWifi *w, const GPtrArray *aps; aps = nm_device_wifi_get_access_points (w); - test_assert (aps == NULL); + g_assert (aps == NULL); info->notified = TRUE; wifi_check_quit (info); @@ -358,11 +330,11 @@ test_wifi_ap_added_removed (void) info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); g_main_loop_run (loop); - test_assert (info.found); + g_assert (info.found); g_signal_handlers_disconnect_by_func (client, wifi_device_added_cb, &info); wifi = (NMDeviceWifi *) nm_client_get_device_by_iface (client, "wlan0"); - test_assert (NM_IS_DEVICE_WIFI (wifi)); + g_assert (NM_IS_DEVICE_WIFI (wifi)); /*************************************/ /* Add the wifi device */ @@ -377,9 +349,9 @@ test_wifi_ap_added_removed (void) 3000, NULL, &error); - test_assert_no_error (error); - test_assert (ret); - test_assert_cmpstr (g_variant_get_type_string (ret), ==, "(o)"); + g_assert_no_error (error); + g_assert (ret); + g_assert_cmpstr (g_variant_get_type_string (ret), ==, "(o)"); g_variant_get (ret, "(o)", &expected_path); g_variant_unref (ret); @@ -399,10 +371,10 @@ test_wifi_ap_added_removed (void) info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); g_main_loop_run (loop); - test_assert (info.signaled); - test_assert (info.notified); - test_assert (info.ap_path); - test_assert_cmpstr (info.ap_path, ==, expected_path); + g_assert (info.signaled); + g_assert (info.notified); + g_assert (info.ap_path); + g_assert_cmpstr (info.ap_path, ==, expected_path); g_signal_handlers_disconnect_by_func (wifi, wifi_ap_added_cb, &info); g_signal_handlers_disconnect_by_func (wifi, wifi_ap_add_notify_cb, &info); @@ -419,7 +391,7 @@ test_wifi_ap_added_removed (void) 3000, NULL, &error); - test_assert_no_error (error); + g_assert_no_error (error); g_clear_pointer (&ret, g_variant_unref); g_signal_connect (wifi, @@ -438,8 +410,8 @@ test_wifi_ap_added_removed (void) info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); g_main_loop_run (loop); - test_assert (info.signaled); - test_assert (info.notified); + g_assert (info.signaled); + g_assert (info.notified); g_signal_handlers_disconnect_by_func (wifi, wifi_ap_removed_cb, &info); g_signal_handlers_disconnect_by_func (wifi, wifi_ap_remove_notify_cb, &info); @@ -480,7 +452,7 @@ wimax_device_added_cb (NMClient *c, NMDevice *device, WimaxNspInfo *info) { - test_assert_cmpstr (nm_device_get_iface (device), ==, "wmx0"); + g_assert_cmpstr (nm_device_get_iface (device), ==, "wmx0"); info->found = TRUE; wimax_check_quit (info); } @@ -489,7 +461,7 @@ static void got_nsp_path (WimaxNspInfo *info, const char *path) { if (info->nsp_path) - test_assert_cmpstr (info->nsp_path, ==, path); + g_assert_cmpstr (info->nsp_path, ==, path); else info->nsp_path = g_strdup (path); } @@ -499,8 +471,8 @@ wimax_nsp_added_cb (NMDeviceWimax *w, NMWimaxNsp *nsp, WimaxNspInfo *info) { - test_assert (nsp); - test_assert_cmpstr (nm_wimax_nsp_get_name (nsp), ==, expected_nsp_name); + g_assert (nsp); + g_assert_cmpstr (nm_wimax_nsp_get_name (nsp), ==, expected_nsp_name); got_nsp_path (info, nm_object_get_path (NM_OBJECT (nsp))); info->signaled = TRUE; @@ -516,12 +488,12 @@ wimax_nsp_add_notify_cb (NMDeviceWimax *w, NMWimaxNsp *nsp; nsps = nm_device_wimax_get_nsps (w); - test_assert (nsps); - test_assert_cmpint (nsps->len, ==, 1); + g_assert (nsps); + g_assert_cmpint (nsps->len, ==, 1); nsp = g_ptr_array_index (nsps, 0); - test_assert (nsp); - test_assert_cmpstr (nm_wimax_nsp_get_name (nsp), ==, expected_nsp_name); + g_assert (nsp); + g_assert_cmpstr (nm_wimax_nsp_get_name (nsp), ==, expected_nsp_name); got_nsp_path (info, nm_object_get_path (NM_OBJECT (nsp))); info->notified = TRUE; @@ -533,8 +505,8 @@ wimax_nsp_removed_cb (NMDeviceWimax *w, NMWimaxNsp *nsp, WimaxNspInfo *info) { - test_assert (nsp); - test_assert_cmpstr (info->nsp_path, ==, nm_object_get_path (NM_OBJECT (nsp))); + g_assert (nsp); + g_assert_cmpstr (info->nsp_path, ==, nm_object_get_path (NM_OBJECT (nsp))); info->signaled = TRUE; wimax_check_quit (info); @@ -548,7 +520,7 @@ wimax_nsp_remove_notify_cb (NMDeviceWimax *w, const GPtrArray *nsps; nsps = nm_device_wimax_get_nsps (w); - test_assert (nsps == NULL); + g_assert (nsps == NULL); info->notified = TRUE; wimax_check_quit (info); @@ -581,11 +553,11 @@ test_wimax_nsp_added_removed (void) info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); g_main_loop_run (loop); - test_assert (info.found); + g_assert (info.found); g_signal_handlers_disconnect_by_func (client, wimax_device_added_cb, &info); wimax = (NMDeviceWimax *) nm_client_get_device_by_iface (client, "wmx0"); - test_assert (NM_IS_DEVICE_WIMAX (wimax)); + g_assert (NM_IS_DEVICE_WIMAX (wimax)); /*************************************/ /* Add the wimax NSP */ @@ -600,9 +572,9 @@ test_wimax_nsp_added_removed (void) 3000, NULL, &error); - test_assert_no_error (error); - test_assert (ret); - test_assert_cmpstr (g_variant_get_type_string (ret), ==, "(o)"); + g_assert_no_error (error); + g_assert (ret); + g_assert_cmpstr (g_variant_get_type_string (ret), ==, "(o)"); g_variant_get (ret, "(o)", &expected_path); g_variant_unref (ret); @@ -622,10 +594,10 @@ test_wimax_nsp_added_removed (void) info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); g_main_loop_run (loop); - test_assert (info.signaled); - test_assert (info.notified); - test_assert (info.nsp_path); - test_assert_cmpstr (info.nsp_path, ==, expected_path); + g_assert (info.signaled); + g_assert (info.notified); + g_assert (info.nsp_path); + g_assert_cmpstr (info.nsp_path, ==, expected_path); g_signal_handlers_disconnect_by_func (wimax, wimax_nsp_added_cb, &info); g_signal_handlers_disconnect_by_func (wimax, wimax_nsp_add_notify_cb, &info); @@ -642,7 +614,7 @@ test_wimax_nsp_added_removed (void) 3000, NULL, &error); - test_assert_no_error (error); + g_assert_no_error (error); g_clear_pointer (&ret, g_variant_unref); g_signal_connect (wimax, @@ -661,8 +633,8 @@ test_wimax_nsp_added_removed (void) info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); g_main_loop_run (loop); - test_assert (info.signaled); - test_assert (info.notified); + g_assert (info.signaled); + g_assert (info.notified); g_signal_handlers_disconnect_by_func (wimax, wimax_nsp_removed_cb, &info); g_signal_handlers_disconnect_by_func (wimax, wimax_nsp_remove_notify_cb, &info); @@ -707,7 +679,7 @@ da_device_removed_cb (NMClient *c, NMDevice *device, DaInfo *info) { - test_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); + g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0"); info->signaled = TRUE; da_check_quit (info); } @@ -723,14 +695,14 @@ da_devices_notify_cb (NMClient *c, const char *iface; devices = nm_client_get_devices (c); - test_assert (devices); - test_assert_cmpint (devices->len, ==, 2); + g_assert (devices); + g_assert_cmpint (devices->len, ==, 2); for (i = 0; i < devices->len; i++) { device = g_ptr_array_index (devices, i); iface = nm_device_get_iface (device); - test_assert (!strcmp (iface, "wlan0") || !strcmp (iface, "eth1")); + g_assert (!strcmp (iface, "wlan0") || !strcmp (iface, "eth1")); } info->notified = TRUE; @@ -767,22 +739,22 @@ test_devices_array (void) info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); g_main_loop_run (loop); - test_assert_cmpint (info.quit_count, ==, 0); + g_assert_cmpint (info.quit_count, ==, 0); g_signal_handlers_disconnect_by_func (client, da_device_added_cb, &info); /* Ensure the devices now exist */ devices = nm_client_get_devices (client); - test_assert (devices); - test_assert_cmpint (devices->len, ==, 3); + g_assert (devices); + g_assert_cmpint (devices->len, ==, 3); device = nm_client_get_device_by_iface (client, "wlan0"); - test_assert (NM_IS_DEVICE_WIFI (device)); + g_assert (NM_IS_DEVICE_WIFI (device)); device = nm_client_get_device_by_iface (client, "eth0"); - test_assert (NM_IS_DEVICE_ETHERNET (device)); + g_assert (NM_IS_DEVICE_ETHERNET (device)); device = nm_client_get_device_by_iface (client, "eth1"); - test_assert (NM_IS_DEVICE_ETHERNET (device)); + g_assert (NM_IS_DEVICE_ETHERNET (device)); /********************************/ /* Now remove the device in the middle */ @@ -793,8 +765,8 @@ test_devices_array (void) 3000, NULL, &error); - test_assert_no_error (error); - test_assert (ret); + g_assert_no_error (error); + g_assert (ret); g_variant_unref (ret); g_signal_connect (client, @@ -812,20 +784,20 @@ test_devices_array (void) info.quit_id = g_timeout_add_seconds (5, loop_quit, loop); g_main_loop_run (loop); - test_assert_cmpint (info.quit_count, ==, 0); + g_assert_cmpint (info.quit_count, ==, 0); g_signal_handlers_disconnect_by_func (client, da_device_removed_cb, &info); g_signal_handlers_disconnect_by_func (client, da_devices_notify_cb, &info); /* Ensure only two are left */ devices = nm_client_get_devices (client); - test_assert (devices); - test_assert_cmpint (devices->len, ==, 2); + g_assert (devices); + g_assert_cmpint (devices->len, ==, 2); device = nm_client_get_device_by_iface (client, "wlan0"); - test_assert (NM_IS_DEVICE_WIFI (device)); + g_assert (NM_IS_DEVICE_WIFI (device)); device = nm_client_get_device_by_iface (client, "eth1"); - test_assert (NM_IS_DEVICE_ETHERNET (device)); + g_assert (NM_IS_DEVICE_ETHERNET (device)); g_free (paths[0]); g_free (paths[1]); diff --git a/libnm-glib/tests/test-remote-settings-client.c b/libnm-glib/tests/test-remote-settings-client.c index 7b4bdce36b..e8dd95596f 100644 --- a/libnm-glib/tests/test-remote-settings-client.c +++ b/libnm-glib/tests/test-remote-settings-client.c @@ -42,18 +42,6 @@ NMRemoteConnection *remote = NULL; /*******************************************************************/ -#define test_assert(condition) \ -do { \ - gboolean _condition = !!( condition ); \ - \ - if (G_UNLIKELY (!_condition)) { \ - nm_test_service_cleanup (sinfo); \ - g_assert (!"test_assert() failed for" # condition); \ - } \ -} while (0) - -/*******************************************************************/ - static void add_cb (NMRemoteSettings *s, NMRemoteConnection *connection, @@ -100,20 +88,20 @@ test_add_connection (void) connection, add_cb, &done); - test_assert (success == TRUE); + g_assert (success == TRUE); start = time (NULL); do { now = time (NULL); g_main_context_iteration (NULL, FALSE); } while ((done == FALSE) && (now - start < 5)); - test_assert (done == TRUE); - test_assert (remote != NULL); + g_assert (done == TRUE); + g_assert (remote != NULL); /* Make sure the connection is the same as what we added */ - test_assert (nm_connection_compare (connection, - NM_CONNECTION (remote), - NM_SETTING_COMPARE_FLAG_EXACT) == TRUE); + g_assert (nm_connection_compare (connection, + NM_CONNECTION (remote), + NM_SETTING_COMPARE_FLAG_EXACT) == TRUE); } /*******************************************************************/ @@ -127,10 +115,8 @@ set_visible_cb (DBusGProxy *proxy, gboolean success; success = dbus_g_proxy_end_call (proxy, call, &error, G_TYPE_INVALID); - if (!success) - g_warning ("Failed to change connection visibility: %s", error->message); - test_assert (success == TRUE); - test_assert (error == NULL); + g_assert_no_error (error); + g_assert (success == TRUE); } static void @@ -158,7 +144,7 @@ test_make_invisible (void) gboolean done = FALSE, has_settings = FALSE; char *path; - test_assert (remote != NULL); + g_assert (remote != NULL); /* Listen for the remove event when the connection becomes invisible */ g_signal_connect (remote, "removed", G_CALLBACK (invis_removed_cb), &done); @@ -168,7 +154,7 @@ test_make_invisible (void) NM_DBUS_SERVICE, path, NM_DBUS_IFACE_SETTINGS_CONNECTION); - test_assert (proxy != NULL); + g_assert (proxy != NULL); /* Bypass the NMRemoteSettings object so we can test it independently */ dbus_g_proxy_begin_call (proxy, "SetVisible", set_visible_cb, NULL, NULL, @@ -180,7 +166,7 @@ test_make_invisible (void) now = time (NULL); g_main_context_iteration (NULL, FALSE); } while ((done == FALSE) && (now - start < 5)); - test_assert (done == TRUE); + g_assert (done == TRUE); g_assert (remote); g_signal_handlers_disconnect_by_func (remote, G_CALLBACK (invis_removed_cb), &done); @@ -190,8 +176,8 @@ test_make_invisible (void) for (iter = list; iter; iter = g_slist_next (iter)) { NMConnection *candidate = NM_CONNECTION (iter->data); - test_assert ((gpointer) remote != (gpointer) candidate); - test_assert (strcmp (path, nm_connection_get_path (candidate)) != 0); + g_assert ((gpointer) remote != (gpointer) candidate); + g_assert (strcmp (path, nm_connection_get_path (candidate)) != 0); } /* And ensure the invisible connection no longer has any settings */ @@ -199,7 +185,7 @@ test_make_invisible (void) nm_connection_for_each_setting_value (NM_CONNECTION (remote), invis_has_settings_cb, &has_settings); - test_assert (has_settings == FALSE); + g_assert (has_settings == FALSE); g_free (path); g_object_unref (proxy); @@ -225,7 +211,7 @@ test_make_visible (void) char *path; NMRemoteConnection *new = NULL; - test_assert (remote != NULL); + g_assert (remote != NULL); /* Wait for the new-connection signal when the connection is visible again */ g_signal_connect (settings, NM_REMOTE_SETTINGS_NEW_CONNECTION, @@ -236,7 +222,7 @@ test_make_visible (void) NM_DBUS_SERVICE, path, NM_DBUS_IFACE_SETTINGS_CONNECTION); - test_assert (proxy != NULL); + g_assert (proxy != NULL); /* Bypass the NMRemoteSettings object so we can test it independently */ dbus_g_proxy_begin_call (proxy, "SetVisible", set_visible_cb, NULL, NULL, @@ -251,8 +237,8 @@ test_make_visible (void) } while ((new == NULL) && (now - start < 5)); /* Ensure the new connection is the same as the one we made visible again */ - test_assert (new); - test_assert (new == remote); + g_assert (new); + g_assert (new == remote); g_signal_handlers_disconnect_by_func (settings, G_CALLBACK (vis_new_connection_cb), &new); @@ -262,13 +248,13 @@ test_make_visible (void) NMConnection *candidate = NM_CONNECTION (iter->data); if ((gpointer) remote == (gpointer) candidate) { - test_assert (strcmp (path, nm_connection_get_path (candidate)) == 0); - test_assert (strcmp (TEST_CON_ID, nm_connection_get_id (candidate)) == 0); + g_assert_cmpstr (path, ==, nm_connection_get_path (candidate)); + g_assert_cmpstr (TEST_CON_ID, ==, nm_connection_get_id (candidate)); found = TRUE; break; } } - test_assert (found == TRUE); + g_assert (found == TRUE); g_free (path); g_object_unref (proxy); @@ -285,10 +271,8 @@ deleted_cb (DBusGProxy *proxy, gboolean success; success = dbus_g_proxy_end_call (proxy, call, &error, G_TYPE_INVALID); - if (!success) - g_warning ("Failed to delete connection: %s", error->message); - test_assert (success == TRUE); - test_assert (error == NULL); + g_assert_no_error (error); + g_assert (success == TRUE); } static void @@ -309,7 +293,7 @@ test_remove_connection (void) /* Find a connection to delete */ list = nm_remote_settings_list_connections (settings); - test_assert (g_slist_length (list) > 0); + g_assert_cmpint (g_slist_length (list), >, 0); connection = NM_REMOTE_CONNECTION (list->data); g_assert (connection); @@ -321,7 +305,7 @@ test_remove_connection (void) NM_DBUS_SERVICE, path, NM_DBUS_IFACE_SETTINGS_CONNECTION); - test_assert (proxy != NULL); + g_assert (proxy != NULL); /* Bypass the NMRemoteSettings object so we can test it independently */ dbus_g_proxy_begin_call (proxy, "Delete", deleted_cb, NULL, NULL, G_TYPE_INVALID); @@ -331,7 +315,7 @@ test_remove_connection (void) now = time (NULL); g_main_context_iteration (NULL, FALSE); } while ((done == FALSE) && (now - start < 5)); - test_assert (done == TRUE); + g_assert (done == TRUE); g_assert (!remote); @@ -340,8 +324,8 @@ test_remove_connection (void) for (iter = list; iter; iter = g_slist_next (iter)) { NMConnection *candidate = NM_CONNECTION (iter->data); - test_assert ((gpointer) connection != (gpointer) candidate); - test_assert (strcmp (path, nm_connection_get_path (candidate)) != 0); + g_assert ((gpointer) connection != (gpointer) candidate); + g_assert_cmpstr (path, ==, nm_connection_get_path (candidate)); } g_free (path); @@ -363,15 +347,12 @@ main (int argc, char **argv) g_test_init (&argc, &argv, NULL); bus = dbus_g_bus_get (DBUS_BUS_SESSION, &error); - if (!bus) { - g_warning ("Error connecting to D-Bus: %s", error->message); - g_assert (error == NULL); - } + g_assert_no_error (error); sinfo = nm_test_service_init (); settings = nm_remote_settings_new (bus); - test_assert (settings != NULL); + g_assert (settings != NULL); g_test_add_func ("/remote_settings/add_connection", test_add_connection); g_test_add_func ("/remote_settings/make_invisible", test_make_invisible); diff --git a/tools/test-networkmanager-service.py b/tools/test-networkmanager-service.py index 5f1f613766..437381eb06 100755 --- a/tools/test-networkmanager-service.py +++ b/tools/test-networkmanager-service.py @@ -858,6 +858,9 @@ class Settings(dbus.service.Object): ################################################################### +def stdin_cb(io, condition): + mainloop.quit() + def quit_cb(user_data): mainloop.quit() @@ -872,7 +875,11 @@ def main(): if not bus.request_name("org.freedesktop.NetworkManager"): sys.exit(1) - # quit after inactivity to ensure we don't stick around if tests fail + # Watch stdin; if it closes, assume our parent has crashed, and exit + io = GLib.IOChannel.unix_new(0) + io.add_watch(GLib.IOCondition.HUP, stdin_cb) + + # also quit after inactivity to ensure we don't stick around if the above fails somehow GLib.timeout_add_seconds(20, quit_cb, None) try: From fe264a2d01245d1cc814f5df6f86623bb8b35aef Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 31 Jul 2014 09:21:05 -0400 Subject: [PATCH 5/6] libnm-glib: fix a crash when using multiple NMClients NMObjectCache was assuming there would never be more than one object with the same path, but since NMClient is an NMObject, it was getting cached too, so if you created two clients and then unreffed one of them, it's possible the wrong one could get left in the cache, causing a crash the next time the other one called nm_object_cache_clear(). Fix this by only adding NMObjects to the cache in the codepaths where we also check to see if the object was already in the cache. (This also means we can remove the "except" argument to nm_object_cache_clear(), since the NMClient won't be cached any more.) --- libnm-glib/nm-client.c | 2 +- libnm-glib/nm-object-cache.c | 26 +++++++++++++------------- libnm-glib/nm-object-cache.h | 2 +- libnm-glib/nm-object.c | 6 ++++-- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/libnm-glib/nm-client.c b/libnm-glib/nm-client.c index c020f9ffdb..711da7f4a0 100644 --- a/libnm-glib/nm-client.c +++ b/libnm-glib/nm-client.c @@ -1383,7 +1383,7 @@ proxy_name_owner_changed (DBusGProxy *proxy, /* Clear object cache to ensure bad refcounting by clients doesn't * keep objects in the cache. */ - _nm_object_cache_clear (NM_OBJECT (client)); + _nm_object_cache_clear (); } else { _nm_object_suppress_property_updates (NM_OBJECT (client), FALSE); _nm_object_reload_properties_async (NM_OBJECT (client), updated_properties, client); diff --git a/libnm-glib/nm-object-cache.c b/libnm-glib/nm-object-cache.c index 833ee24092..fe388803e7 100644 --- a/libnm-glib/nm-object-cache.c +++ b/libnm-glib/nm-object-cache.c @@ -63,26 +63,26 @@ _nm_object_cache_get (const char *path) } void -_nm_object_cache_clear (NMObject *except) +_nm_object_cache_clear (void) { GHashTableIter iter; - NMObject *obj; + GObject *obj; const char *path; char *foo; - _init_cache (); + if (!cache) + return; + g_hash_table_iter_init (&iter, cache); while (g_hash_table_iter_next (&iter, (gpointer) &path, (gpointer) &obj)) { - if (obj != except) { - /* Remove the callback so that if the object isn't yet released - * by a client, when it does finally get unrefed, it won't trigger - * the cache removal for a new object with the same path as the - * one being released. - */ - foo = g_object_steal_data (G_OBJECT (obj), "nm-object-cache-tag"); - g_free (foo); + /* Remove the callback so that if the object isn't yet released + * by a client, when it does finally get unrefed, it won't trigger + * the cache removal for a new object with the same path as the + * one being released. + */ + foo = g_object_steal_data (obj, "nm-object-cache-tag"); + g_free (foo); - g_hash_table_iter_remove (&iter); - } + g_hash_table_iter_remove (&iter); } } diff --git a/libnm-glib/nm-object-cache.h b/libnm-glib/nm-object-cache.h index 87b4689617..7aca3b4fba 100644 --- a/libnm-glib/nm-object-cache.h +++ b/libnm-glib/nm-object-cache.h @@ -30,7 +30,7 @@ G_BEGIN_DECLS /* Returns referenced object from the cache */ NMObject *_nm_object_cache_get (const char *path); void _nm_object_cache_add (NMObject *object); -void _nm_object_cache_clear (NMObject *except); +void _nm_object_cache_clear (void); G_END_DECLS diff --git a/libnm-glib/nm-object.c b/libnm-glib/nm-object.c index e823baf43d..3550677df6 100644 --- a/libnm-glib/nm-object.c +++ b/libnm-glib/nm-object.c @@ -159,8 +159,6 @@ constructor (GType type, return NULL; } - _nm_object_cache_add (NM_OBJECT (object)); - return object; } @@ -573,6 +571,8 @@ _nm_object_create (GType type, DBusGConnection *connection, const char *path) NM_OBJECT_DBUS_CONNECTION, connection, NM_OBJECT_DBUS_PATH, path, NULL); + if (NM_IS_OBJECT (object)) + _nm_object_cache_add (NM_OBJECT (object)); if (!g_initable_init (G_INITABLE (object), NULL, &error)) { dbgmsg ("Could not create object for %s: %s", path, error->message); g_error_free (error); @@ -656,6 +656,8 @@ async_got_type (GType type, gpointer user_data) NM_OBJECT_DBUS_PATH, async_data->path, NULL); g_warn_if_fail (object != NULL); + if (NM_IS_OBJECT (object)) + _nm_object_cache_add (NM_OBJECT (object)); g_async_initable_init_async (G_ASYNC_INITABLE (object), G_PRIORITY_DEFAULT, NULL, async_inited, async_data); } From 48ad58620965a31d7e7d4811c4189f38675fe2ae Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Thu, 31 Jul 2014 09:24:45 -0400 Subject: [PATCH 6/6] libnm-glib: add tests of NMClient:manager-running and NMRemoteSettings:service-running Test that the code to track whether NetworkManager is running works correctly. --- libnm-glib/tests/test-nm-client.c | 68 +++++++++++++++ .../tests/test-remote-settings-client.c | 86 +++++++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/libnm-glib/tests/test-nm-client.c b/libnm-glib/tests/test-nm-client.c index dc6126451b..eebf8358c7 100644 --- a/libnm-glib/tests/test-nm-client.c +++ b/libnm-glib/tests/test-nm-client.c @@ -807,6 +807,73 @@ test_devices_array (void) g_clear_pointer (&sinfo, nm_test_service_cleanup); } +static void +manager_running_changed (GObject *client, + GParamSpec *pspec, + gpointer user_data) +{ + int *running_changed = user_data; + + (*running_changed)++; + g_main_loop_quit (loop); +} + +static void +test_client_manager_running (void) +{ + NMClient *client1, *client2; + guint quit_id; + int running_changed = 0; + GError *error = NULL; + + client1 = test_client_new (); + + g_assert (!nm_client_get_manager_running (client1)); + g_assert_cmpstr (nm_client_get_version (client1), ==, NULL); + + g_assert (!nm_client_networking_get_enabled (client1)); + /* This will have no effect, but it shouldn't cause any warnings either. */ + nm_client_networking_set_enabled (client1, TRUE); + g_assert (!nm_client_networking_get_enabled (client1)); + + /* OTOH, this should result in an error */ + nm_client_set_logging (client1, "DEFAULT", "INFO", &error); + g_assert_error (error, NM_CLIENT_ERROR, NM_CLIENT_ERROR_MANAGER_NOT_RUNNING); + g_clear_error (&error); + + /* Now start the test service. */ + sinfo = nm_test_service_init (); + client2 = test_client_new (); + + /* client2 should know that NM is running, but the previously-created + * client1 hasn't gotten the news yet. + */ + g_assert (!nm_client_get_manager_running (client1)); + g_assert (nm_client_get_manager_running (client2)); + + g_signal_connect (client1, "notify::" NM_CLIENT_MANAGER_RUNNING, + G_CALLBACK (manager_running_changed), &running_changed); + quit_id = g_timeout_add_seconds (5, loop_quit, loop); + g_main_loop_run (loop); + g_assert_cmpint (running_changed, ==, 1); + g_assert (nm_client_get_manager_running (client1)); + g_source_remove (quit_id); + + /* And kill it */ + g_clear_pointer (&sinfo, nm_test_service_cleanup); + + g_assert (nm_client_get_manager_running (client1)); + + quit_id = g_timeout_add_seconds (5, loop_quit, loop); + g_main_loop_run (loop); + g_assert_cmpint (running_changed, ==, 2); + g_assert (!nm_client_get_manager_running (client1)); + g_source_remove (quit_id); + + g_object_unref (client1); + g_object_unref (client2); +} + /*******************************************************************/ int @@ -824,6 +891,7 @@ main (int argc, char **argv) g_test_add_func ("/libnm-glib/wifi-ap-added-removed", test_wifi_ap_added_removed); g_test_add_func ("/libnm-glib/wimax-nsp-added-removed", test_wimax_nsp_added_removed); g_test_add_func ("/libnm-glib/devices-array", test_devices_array); + g_test_add_func ("/libnm-glib/client-manager-running", test_client_manager_running); return g_test_run (); } diff --git a/libnm-glib/tests/test-remote-settings-client.c b/libnm-glib/tests/test-remote-settings-client.c index e8dd95596f..987845c1bb 100644 --- a/libnm-glib/tests/test-remote-settings-client.c +++ b/libnm-glib/tests/test-remote-settings-client.c @@ -334,6 +334,88 @@ test_remove_connection (void) /*******************************************************************/ +static GMainLoop *loop; + +static gboolean +loop_quit (gpointer user_data) +{ + g_main_loop_quit (loop); + return G_SOURCE_REMOVE; +} + +static void +settings_service_running_changed (GObject *client, + GParamSpec *pspec, + gpointer user_data) +{ + int *running_changed = user_data; + + (*running_changed)++; + g_main_loop_quit (loop); +} + +static void +test_service_running (void) +{ + NMRemoteSettings *settings2; + guint quit_id; + int running_changed = 0; + gboolean running; + + loop = g_main_loop_new (NULL, FALSE); + + g_object_get (G_OBJECT (settings), + NM_REMOTE_SETTINGS_SERVICE_RUNNING, &running, + NULL); + g_assert (running == TRUE); + + /* Now kill the test service. */ + nm_test_service_cleanup (sinfo); + + settings2 = nm_remote_settings_new (bus); + + /* settings2 should know that NM is running, but the previously-created + * settings hasn't gotten the news yet. + */ + g_object_get (G_OBJECT (settings2), + NM_REMOTE_SETTINGS_SERVICE_RUNNING, &running, + NULL); + g_assert (running == FALSE); + g_object_get (G_OBJECT (settings), + NM_REMOTE_SETTINGS_SERVICE_RUNNING, &running, + NULL); + g_assert (running == TRUE); + + g_signal_connect (settings, "notify::" NM_REMOTE_SETTINGS_SERVICE_RUNNING, + G_CALLBACK (settings_service_running_changed), &running_changed); + quit_id = g_timeout_add_seconds (5, loop_quit, loop); + g_main_loop_run (loop); + g_assert_cmpint (running_changed, ==, 1); + g_source_remove (quit_id); + + g_object_get (G_OBJECT (settings2), + NM_REMOTE_SETTINGS_SERVICE_RUNNING, &running, + NULL); + g_assert (running == FALSE); + + /* Now restart it */ + sinfo = nm_test_service_init (); + + quit_id = g_timeout_add_seconds (5, loop_quit, loop); + g_main_loop_run (loop); + g_assert_cmpint (running_changed, ==, 2); + g_source_remove (quit_id); + + g_object_get (G_OBJECT (settings2), + NM_REMOTE_SETTINGS_SERVICE_RUNNING, &running, + NULL); + g_assert (running == TRUE); + + g_object_unref (settings2); +} + +/*******************************************************************/ + int main (int argc, char **argv) { @@ -354,10 +436,14 @@ main (int argc, char **argv) settings = nm_remote_settings_new (bus); g_assert (settings != NULL); + /* FIXME: these tests assume that they get run in order, but g_test_run() + * does not actually guarantee that! + */ g_test_add_func ("/remote_settings/add_connection", test_add_connection); g_test_add_func ("/remote_settings/make_invisible", test_make_invisible); g_test_add_func ("/remote_settings/make_visible", test_make_visible); g_test_add_func ("/remote_settings/remove_connection", test_remove_connection); + g_test_add_func ("/remote_settings/service_running", test_service_running); ret = g_test_run ();