GNOME.org
 

[glib] Rewrite apps test to ensure children are killed

commit cc19922183b18683da192cc7371a510cf648ec64
Author: Colin Walters <walters verbum org>
Date:   Tue Jun 8 21:43:16 2010 -0400

    Rewrite apps test to ensure children are killed
    
    Create a function run_with_application that both ensures the
    app is running exactly while the test is running, which most
    of the tests use.  We start it beforehand, and kill it after.
    This avoids having any interdependence between the tests (and
    there definitely was before, because we didn't wait for
    the process to actually terminate after a kill() call).
    
    Also, open a pipe between the two, and have the child app
    monitor that pipe.  If it gets closed (e.g. because the parent
    died), the child exits.  This is the most reliable way to
    avoid stale children; before, if we failed an assertion, the
    parent would abort, and not run kill().
    
    https://bugzilla.gnome.org/show_bug.cgi?id=621034

 gio/tests/testapp.c  |   50 +++---
 gio/tests/testapps.c |  512 ++++++++++++++++++++++++++------------------------
 2 files changed, 298 insertions(+), 264 deletions(-)
---
diff --git a/gio/tests/testapp.c b/gio/tests/testapp.c
index 8ed3091..85f93b7 100644
--- a/gio/tests/testapp.c
+++ b/gio/tests/testapp.c
@@ -1,8 +1,12 @@
-#include <stdlib.h>
 #include <gio.h>
 #include <gstdio.h>
 #include <string.h>
 
+#ifdef G_OS_UNIX
+#include <stdlib.h>
+#include <fcntl.h>
+#endif
+
 static gboolean action3_added = FALSE;
 
 static void
@@ -22,37 +26,39 @@ on_app_action (GApplication *application,
     }
 }
 
-static gboolean
-invoke_action1 (gpointer data)
-{
-  GApplication *app = data;
-
-  g_application_invoke_action (app, "action1", 0);
-
-  return FALSE;
-}
-
 static void
 on_app_activated (GApplication  *application,
 		  GVariant      *args,
 		  GVariant      *platform_data)
 {
-  char *str;
+}
 
-  g_print ("got args: ");
-  str = g_variant_print (args, TRUE);
-  g_print ("%s ", str);
-  g_free (str);
-  str = g_variant_print (platform_data, TRUE);
-  g_print ("%s\n", str);
-  g_free (str);
+static gboolean
+on_monitor_fd_io (GIOChannel *source,
+		  GIOCondition condition,
+		  gpointer data)
+{
+  exit (0);
+  return FALSE;
 }
 
 int
 main (int argc, char *argv[])
 {
   GApplication *app;
-  GMainLoop *loop;
+
+#ifdef G_OS_UNIX
+  {
+    const char *slave_fd_env = g_getenv ("_G_TEST_SLAVE_FD");
+    if (slave_fd_env)
+      {
+	int slave_fd = atoi (slave_fd_env);
+	fcntl (slave_fd, F_SETFD, FD_CLOEXEC);
+	g_io_add_watch (g_io_channel_unix_new (slave_fd), G_IO_HUP | G_IO_ERR,
+			on_monitor_fd_io, NULL);
+      }
+  }
+#endif
 
   app = g_application_new ("org.gtk.test.app");
   if (!(argc > 1 && strcmp (argv[1], "--non-unique") == 0))
@@ -60,9 +66,7 @@ main (int argc, char *argv[])
 
   if (g_application_is_remote (app))
     {
-      g_timeout_add (1000, invoke_action1, app);
-      loop = g_main_loop_new (NULL, FALSE);
-      g_main_loop_run (loop);
+      g_application_invoke_action (app, "action1", 0);
     }
   else
     {
diff --git a/gio/tests/testapps.c b/gio/tests/testapps.c
index 423e808..b83c5c6 100644
--- a/gio/tests/testapps.c
+++ b/gio/tests/testapps.c
@@ -1,16 +1,16 @@
 #include <gio/gio.h>
+#ifdef G_OS_UNIX
+#include <fcntl.h>
 #include <sys/types.h>
 #include <sys/wait.h>
 #include <signal.h>
+#endif
 
 #include "gdbus-sessionbus.h"
 
 static gint appeared;
 static gint disappeared;
 static gint changed;
-static gboolean died;
-static gboolean timed_out;
-GPid pid;
 
 static void
 name_appeared (GDBusConnection *connection,
@@ -39,174 +39,301 @@ name_disappeared (GDBusConnection *connection,
     g_main_loop_quit (loop);
 }
 
-static gboolean
-start_application (gpointer data)
+#ifdef G_OS_UNIX
+void
+child_setup_pipe (gpointer user_data)
 {
-  gchar *argv[] = { "./testapp", NULL };
+  int *fds = user_data;
 
-  g_assert (g_spawn_async (NULL, argv, NULL, 0, NULL, NULL, &pid, NULL));
+  close (fds[0]);
+  dup2 (fds[1], 3);
+  g_setenv ("_G_TEST_SLAVE_FD", "3", TRUE);
+  close (fds[1]);
+}
+#endif
 
-  return FALSE;
+static gboolean
+spawn_async_with_monitor_pipe (const gchar *argv[], GPid *pid, int *fd)
+{
+#ifdef G_OS_UNIX
+  int fds[2];
+  gboolean result;
+
+  pipe (fds);
+
+  result = g_spawn_async (NULL, (char**)argv, NULL, G_SPAWN_DO_NOT_REAP_CHILD, child_setup_pipe, &fds, pid, NULL);
+  close (fds[1]);
+  *fd = fds[0];
+  return result;
+#else
+  *fd = -1;
+  return g_spawn_async (NULL, argv, 0, NULL, NULL, pid, NULL);
+#endif
 }
 
 static gboolean
-run_application_sync (gpointer data)
+start_application (GPid *pid, int *fd)
 {
-  GMainLoop *loop = data;
+  const gchar *argv[] = { "./testapp", NULL };
 
-  g_assert (g_spawn_command_line_sync ("./testapp", NULL, NULL, NULL, NULL));
+  g_assert (spawn_async_with_monitor_pipe (argv, pid, fd));
 
-  if (loop)
-    g_main_loop_quit (loop);
+  return FALSE;
+}
+
+typedef struct {
+  GMainContext *context;
+  GSource *child_watch;
+  GSource *timeout;
+  GPid pid;
+  int fd;
+
+  gboolean child_exited;
+  GMainLoop *loop;
+} AwaitChildTerminationData;
+
+static void
+on_child_termination_exited (GPid pid,
+			     gint status,
+			     gpointer user_data)
+{
+  AwaitChildTerminationData *data = user_data;
+  data->child_exited = TRUE;
+  g_spawn_close_pid (data->pid);
+  g_main_loop_quit (data->loop);
+}
 
+static gboolean
+on_child_termination_timeout (gpointer user_data)
+{
+  AwaitChildTerminationData *data = user_data;
+  g_main_loop_quit (data->loop);
   return FALSE;
 }
 
+static void
+await_child_termination_init (AwaitChildTerminationData *data,
+			      GPid                       pid,
+			      int                        fd)
+{
+  data->context = g_main_context_get_thread_default ();
+  data->child_exited = FALSE;
+  data->pid = pid;
+  data->fd = fd;
+}
+
+static void
+await_child_termination_terminate (AwaitChildTerminationData *data)
+{
+#ifdef G_OS_UNIX
+  kill (data->pid, SIGTERM);
+  close (data->fd);
+#endif
+}
+
 static gboolean
-timeout (gpointer data)
+await_child_termination_run (AwaitChildTerminationData *data)
 {
-  GMainLoop *loop = data;
+  GSource *timeout_source;
+  GSource *child_watch_source;
 
-  timed_out = TRUE;
+  data->loop = g_main_loop_new (data->context, FALSE);
 
-  g_main_loop_quit (loop);
+  child_watch_source = g_child_watch_source_new (data->pid);
+  g_source_set_callback (child_watch_source, (GSourceFunc) on_child_termination_exited, data, NULL);
+  g_source_attach (child_watch_source, data->context);
+  g_source_unref (child_watch_source);
+
+  timeout_source = g_timeout_source_new_seconds (5);
+  g_source_set_callback (timeout_source, on_child_termination_timeout, data, NULL);
+  g_source_attach (timeout_source, data->context);
+  g_source_unref (timeout_source);
+
+  g_main_loop_run (data->loop);
+
+  g_source_destroy (child_watch_source);
+  g_source_destroy (timeout_source);
+
+  g_main_loop_unref (data->loop);
 
-  return TRUE;
+  return data->child_exited;
 }
 
-/* This test starts an application, checks that its name appears
- * on the bus, then starts it again and checks that the second
- * instance exits right away.
- */
 static void
-test_unique (void)
+terminate_child_sync (GPid pid, int fd)
 {
+  AwaitChildTerminationData data;
+
+  await_child_termination_init (&data, pid, fd);
+  await_child_termination_terminate (&data);
+  await_child_termination_run (&data);
+}
+
+typedef void (*RunWithApplicationFunc) (void);
+
+typedef struct {
   GMainLoop *loop;
-  gint watch;
-  guint id1, id2, id3;
+  RunWithApplicationFunc func;
+  guint timeout_id;
+} RunWithAppNameAppearedData;
 
-  appeared = 0;
-  timed_out = FALSE;
+static void
+on_run_with_application_name_appeared (GDBusConnection *connection,
+				       const gchar     *name,
+				       const gchar     *name_owner,
+				       gpointer         user_data)
+{
+  RunWithAppNameAppearedData *data = user_data;
+
+  data->func ();
+
+  g_main_loop_quit (data->loop);
+}
+
+static gboolean
+on_run_with_application_timeout (gpointer user_data)
+{
+  RunWithAppNameAppearedData *data = user_data;
+  data->timeout_id = 0;
+  g_error ("Timed out starting testapp");
+  return FALSE;
+}
+
+static void
+run_with_application (RunWithApplicationFunc test_func)
+{
+  GMainLoop *loop;
+  RunWithAppNameAppearedData data;
+  gint watch;
+  GPid main_pid;
+  gint main_fd;
 
   loop = g_main_loop_new (NULL, FALSE);
-  id1 = g_timeout_add (5000, timeout, loop);
+
+  data.timeout_id = 0;
+  data.func = test_func;
+  data.loop = loop;
 
   watch = g_bus_watch_name (G_BUS_TYPE_SESSION,
                             "org.gtk.test.app",
                             0,
-                            name_appeared,
+                            on_run_with_application_name_appeared,
                             NULL,
-                            loop,
+                            &data,
                             NULL);
 
-  id2 = g_timeout_add (0, start_application, loop);
+  data.timeout_id = g_timeout_add_seconds (5, on_run_with_application_timeout, &data);
 
-  g_main_loop_run (loop);
-
-  g_assert_cmpint (appeared, ==, 1);
-
-  id3 = g_timeout_add (0, run_application_sync, loop);
+  start_application (&main_pid, &main_fd);
 
   g_main_loop_run (loop);
 
-  g_assert_cmpint (appeared, ==, 1);
-  g_assert_cmpint (timed_out, ==, FALSE);
+  if (data.timeout_id)
+    {
+      g_source_remove (data.timeout_id);
+      data.timeout_id = 0;
+    }
 
-  g_bus_unwatch_name (watch);
+  g_main_loop_unref (loop);
 
-  kill (pid, SIGTERM);
+  g_bus_unwatch_name (watch);
 
-  g_main_loop_unref (loop);
-  g_source_remove (id1);
-  g_source_remove (id2);
-  g_source_remove (id3);
+  terminate_child_sync (main_pid, main_fd);
 }
 
-static gboolean
-quit_app (gpointer data)
+/* This test starts an application, checks that its name appears
+ * on the bus, then starts it again and checks that the second
+ * instance exits right away.
+ */
+static void
+test_unique_on_app_appeared (void)
 {
-  GDBusConnection *connection;
-  GVariant *res;
+  GPid sub_pid;
+  int sub_fd;
+  int watch;
+  AwaitChildTerminationData data;
 
-  connection = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL);
-  res = g_dbus_connection_call_sync (connection,
-                                     "org.gtk.test.app",
-                                     "/org/gtk/test/app",
-                                     "org.gtk.Application",
-                                     "Quit",
-                                     g_variant_new ("(u)", 0),
-				     NULL,
-                                     G_DBUS_CALL_FLAGS_NONE,
-                                     -1,
-                                     NULL,
-                                     NULL);
-  if (res)
-    g_variant_unref (res);
+  appeared = 0;
+  disappeared = 0;
 
-  return FALSE;
+  watch = g_bus_watch_name (G_BUS_TYPE_SESSION,
+                            "org.gtk.test.app",
+                            0,
+                            name_appeared,
+                            name_disappeared,
+                            NULL,
+                            NULL);
+
+  start_application (&sub_pid, &sub_fd);
+  await_child_termination_init (&data, sub_pid, sub_fd);
+  await_child_termination_run (&data);
+
+  g_bus_unwatch_name (watch);
+
+  g_assert_cmpint (appeared, ==, 1);
+  g_assert_cmpint (disappeared, ==, 0);
 }
 
 static void
-child_is_dead (GPid     pid,
-               gint     status,
-               gpointer data)
+test_unique (void)
 {
-  GMainLoop *loop = data;
-
-  died++;
+  run_with_application (test_unique_on_app_appeared);
+}
 
-  g_assert (WIFEXITED (status) && WEXITSTATUS(status) == 0);
+static void
+on_name_disappeared_quit (GDBusConnection *connection,
+			  const gchar     *name,
+			  gpointer         user_data)
+{
+  GMainLoop *loop = user_data;
 
-  if (loop)
-    g_main_loop_quit (loop);
+  g_main_loop_quit (loop);
 }
 
-/* This test start an application, checks that its name appears on
- * the bus, then calls Quit, and verifies that the name disappears
- * and the application exits.
+/* This test starts an application, checks that its name appears on
+ * the bus, then calls Quit, and verifies that the name disappears and
+ * the application exits.
  */
 static void
-test_quit (void)
+test_quit_on_app_appeared (void)
 {
   GMainLoop *loop;
-  gint watch;
-  guint id1, id2, id3;
-  gchar *argv[] = { "./testapp", NULL };
-
-  appeared = 0;
-  disappeared = 0;
-  died = FALSE;
-  timed_out = FALSE;
+  int quit_disappeared_watch;
+  GDBusConnection *connection;
 
   loop = g_main_loop_new (NULL, FALSE);
-  watch = g_bus_watch_name (G_BUS_TYPE_SESSION,
+  quit_disappeared_watch = g_bus_watch_name (G_BUS_TYPE_SESSION,
                             "org.gtk.test.app",
                             0,
-                            name_appeared,
-                            name_disappeared,
                             NULL,
+                            on_name_disappeared_quit,
+                            loop,
                             NULL);
 
-  g_assert (g_spawn_async (NULL, argv, NULL, G_SPAWN_DO_NOT_REAP_CHILD, NULL, NULL, &pid, NULL));
-
-  id1 = g_child_watch_add (pid, child_is_dead, loop);
-
-  id2 = g_timeout_add (500, quit_app, NULL);
-
-  id3 = g_timeout_add (5000, timeout, loop);
+  connection = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL);
+  g_dbus_connection_call (connection,
+			  "org.gtk.test.app",
+			  "/org/gtk/test/app",
+			  "org.gtk.Application",
+			  "Quit",
+			  g_variant_new ("(u)", 0),
+			  NULL,
+			  G_DBUS_CALL_FLAGS_NONE,
+			  -1,
+			  NULL,
+			  NULL, NULL);
 
   g_main_loop_run (loop);
-  g_assert_cmpint (timed_out, ==, FALSE);
-  g_assert_cmpint (appeared, ==, 1);
-  g_assert_cmpint (disappeared, >=, 1);
-  g_assert_cmpint (died, ==, TRUE);
 
-  g_bus_unwatch_name (watch);
+  g_bus_unwatch_name (quit_disappeared_watch);
 
   g_main_loop_unref (loop);
-  g_source_remove (id1);
-  g_source_remove (id2);
-  g_source_remove (id3);
+}
+
+static void
+test_quit (void)
+{
+  run_with_application (test_quit_on_app_appeared);
 }
 
 static gboolean
@@ -265,33 +392,10 @@ list_actions (void)
   return strv;
 }
 
-/* This test start an application, waits for its name to appear on
- * the bus, then calls ListActions, and verifies that it gets the expected
- * actions back.
- */
 static void
-test_list_actions (void)
+test_list_actions_on_app_appeared (void)
 {
-  GMainLoop *loop;
-  gchar *argv[] = { "./testapp", NULL };
   gchar **actions;
-  gint watch;
-
-  appeared = 0;
-
-  loop = g_main_loop_new (NULL, FALSE);
-  watch = g_bus_watch_name (G_BUS_TYPE_SESSION,
-                            "org.gtk.test.app",
-                            0,
-                            name_appeared,
-                            NULL,
-                            loop,
-                            NULL);
-
-  g_assert (g_spawn_async (NULL, argv, NULL, 0, NULL, NULL, &pid, NULL));
-  if (!appeared)
-    g_main_loop_run (loop);
-  g_main_loop_unref (loop);
 
   actions = list_actions ();
 
@@ -300,16 +404,22 @@ test_list_actions (void)
   g_assert (_g_strv_has_string ((const char *const *)actions, "action2"));
 
   g_strfreev (actions);
+}
 
-  kill (pid, SIGTERM);
-
-  g_bus_unwatch_name (watch);
+/* This test start an application, waits for its name to appear on
+ * the bus, then calls ListActions, and verifies that it gets the expected
+ * actions back.
+ */
+static void
+test_list_actions (void)
+{
+  run_with_application (test_list_actions_on_app_appeared);
 }
 
 static gboolean
-invoke_action (gpointer data)
+invoke_action (gpointer user_data)
 {
-  const gchar *action = data;
+  const gchar *action = user_data;
   GDBusConnection *connection;
   GVariant *res;
 
@@ -334,21 +444,6 @@ invoke_action (gpointer data)
   return FALSE;
 }
 
-static void
-exit_with_code_1 (GPid     pid,
-                  gint     status,
-                  gpointer data)
-{
-  GMainLoop *loop = data;
-
-  died++;
-
-  g_assert (WIFEXITED (status) && WEXITSTATUS(status) == 1);
-
-  if (loop)
-    g_main_loop_quit (loop);
-}
-
 /* This test starts an application, waits for it to appear,
  * then invokes 'action1' and checks that it causes the application
  * to exit with an exit code of 1.
@@ -357,89 +452,43 @@ static void
 test_invoke (void)
 {
   GMainLoop *loop;
-  gint watch;
-  gchar *argv[] = { "./testapp", NULL };
-  guint id1, id2, id3;
-
-  appeared = 0;
-  disappeared = 0;
-  died = FALSE;
-  timed_out = FALSE;
+  int quit_disappeared_watch;
 
   loop = g_main_loop_new (NULL, FALSE);
-  watch = g_bus_watch_name (G_BUS_TYPE_SESSION,
-                            "org.gtk.test.app",
-                            0,
-                            name_appeared,
-                            name_disappeared,
-                            NULL,
-                            NULL);
 
-  g_assert (g_spawn_async (NULL, argv, NULL, G_SPAWN_DO_NOT_REAP_CHILD, NULL, NULL, &pid, NULL));
+  quit_disappeared_watch = g_bus_watch_name (G_BUS_TYPE_SESSION,
+					     "org.gtk.test.app",
+					     0,
+					     NULL,
+					     on_name_disappeared_quit,
+					     loop,
+					     NULL);
 
-  id1 = g_child_watch_add (pid, exit_with_code_1, loop);
-
-  id2 = g_timeout_add (500, invoke_action, "action1");
-
-  id3 = g_timeout_add (5000, timeout, loop);
+  g_timeout_add (0, invoke_action, "action1");
 
   g_main_loop_run (loop);
-  g_assert_cmpint (timed_out, ==, FALSE);
-  g_assert_cmpint (appeared, >=, 1);
-  g_assert_cmpint (disappeared, >=, 1);
-  g_assert_cmpint (died, ==, TRUE);
 
-  g_bus_unwatch_name (watch);
-  g_main_loop_unref (loop);
-  g_source_remove (id1);
-  g_source_remove (id2);
-  g_source_remove (id3);
-
-  kill (pid, SIGTERM);
+  g_bus_unwatch_name (quit_disappeared_watch);
 }
 
 static void
-test_remote (void)
+test_remote_on_application_appeared (void)
 {
-  GMainLoop *loop;
-  gint watch;
-  GPid pid1, pid2;
-  gchar *argv[] = { "./testapp", NULL, NULL };
-
-  appeared = 0;
-  timed_out = FALSE;
-
-  loop = g_main_loop_new (NULL, FALSE);
-  g_timeout_add (5000, timeout, loop);
-
-  watch = g_bus_watch_name (G_BUS_TYPE_SESSION,
-                            "org.gtk.test.app",
-                            0,
-                            name_appeared,
-                            NULL,
-                            loop,
-                            NULL);
-
-  g_assert (g_spawn_async (NULL, argv, NULL, G_SPAWN_DO_NOT_REAP_CHILD, NULL, NULL, &pid1, NULL));
-  g_child_watch_add (pid1, exit_with_code_1, loop);
-
-  g_main_loop_run (loop);
-
-  g_assert_cmpint (appeared, ==, 1);
-
-  argv[1] = "--non-unique";
-  g_assert (g_spawn_async (NULL, argv, NULL, G_SPAWN_DO_NOT_REAP_CHILD, NULL, NULL, &pid2, NULL));
+  GPid sub_pid;
+  int sub_fd;
+  AwaitChildTerminationData data;
+  gchar *argv[] = { "./testapp", "--non-unique", NULL };
 
-  g_main_loop_run (loop);
-
-  g_assert_cmpint (appeared, ==, 1);
-  g_assert_cmpint (timed_out, ==, FALSE);
+  spawn_async_with_monitor_pipe ((const char **) argv, &sub_pid, &sub_fd);
 
-  g_main_loop_unref (loop);
-  g_bus_unwatch_name (watch);
+  await_child_termination_init (&data, sub_pid, sub_fd);
+  await_child_termination_run (&data);
+}
 
-  kill (pid1, SIGTERM);
-  kill (pid2, SIGTERM);
+static void
+test_remote (void)
+{
+  run_with_application (test_remote_on_application_appeared);
 }
 
 static void
@@ -462,34 +511,13 @@ actions_changed (GDBusConnection *connection,
 }
 
 static void
-test_change_action (void)
+test_change_action_on_application_appeared (void)
 {
   GMainLoop *loop;
-  gint watch;
   guint id;
-  GPid pid1;
-  gchar *argv[] = { "./testapp", NULL, NULL };
   GDBusConnection *connection;
 
-  appeared = 0;
-  changed = 0;
-  timed_out = FALSE;
-
   loop = g_main_loop_new (NULL, FALSE);
-  g_timeout_add (5000, timeout, loop);
-
-  watch = g_bus_watch_name (G_BUS_TYPE_SESSION,
-                            "org.gtk.test.app",
-                            0,
-                            name_appeared,
-                            NULL,
-                            loop,
-                            NULL);
-
-  g_assert (g_spawn_async (NULL, argv, NULL, G_SPAWN_DO_NOT_REAP_CHILD, NULL, NULL, &pid1, NULL));
-  g_main_loop_run (loop);
-
-  g_assert_cmpint (appeared, ==, 1);
 
   connection = g_bus_get_sync (G_BUS_TYPE_SESSION, NULL, NULL);
   id = g_dbus_connection_signal_subscribe (connection,
@@ -502,19 +530,21 @@ test_change_action (void)
                                            loop,
                                            NULL);
 
-  g_timeout_add (1000, invoke_action, "action2");
+  g_timeout_add (0, invoke_action, "action2");
 
   g_main_loop_run (loop);
 
   g_assert_cmpint (changed, >, 0);
-  g_assert_cmpint (timed_out, ==, FALSE);
 
   g_dbus_connection_signal_unsubscribe (connection, id);
   g_object_unref (connection);
   g_main_loop_unref (loop);
-  g_bus_unwatch_name (watch);
+}
 
-  kill (pid1, SIGTERM);
+static void
+test_change_action (void)
+{
+  run_with_application (test_change_action_on_application_appeared);
 }
 
 int
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]