ayatana-commits team mailing list archive

[Ted Gould <ted@xxxxxxxx>]

Ted Gould has proposed merging lp:~ted/dbus-test-runner/reftasks into lp:dbus-test-runner.

    Requested reviews:
    Indicator Applet Developers (indicator-applet-developers)


Makes it so that we have better tracking of the sources and the tasks so that everything gets cleaned up nicely and there are no memory access errors.  Also, adds a test to ensure the output is all correct.
-- 
https://code.launchpad.net/~ted/dbus-test-runner/reftasks/+merge/15623
Your team ayatana-commits is subscribed to branch lp:dbus-test-runner.

=== modified file '.bzrignore'
--- .bzrignore	2009-12-02 23:15:30 +0000
+++ .bzrignore	2009-12-03 19:15:21 +0000
@@ -9,3 +9,9 @@
 tests/test-manytask
 tests/test-param
 tests/test-simple
+tests/test-output
+tests/testcat.output.cat1.filtered.txt
+tests/testcat.output.cat1.txt
+tests/testcat.output.cat2.filtered.txt
+tests/testcat.output.cat2.txt
+tests/testcat.output.txt

=== modified file 'src/dbus-test-runner.c'
--- src/dbus-test-runner.c	2009-12-03 16:18:08 +0000
+++ src/dbus-test-runner.c	2009-12-03 19:15:21 +0000
@@ -19,8 +19,57 @@
 	guint watcher;
 	guint number;
 	GList * parameters;
+	gboolean task_die;
+	gboolean text_die;
+	guint idle;
+	guint io_watch;
 } task_t;
 
+static void check_task_cleanup (task_t * task, gboolean force);
+
+static gboolean
+force_kill_in_a_bit (gpointer data)
+{
+	g_debug("Forcing cleanup of: %s", ((task_t *)data)->name);
+	check_task_cleanup((task_t *)data, TRUE);
+	return FALSE;
+}
+
+static void
+check_task_cleanup (task_t * task, gboolean force)
+{
+	if (!force) {
+		if (task->task_die) {
+			if (!task->text_die) {
+				task->idle = g_idle_add(force_kill_in_a_bit, task);
+			}
+		} else  {
+			return;
+		}
+	}
+
+	if (task->idle) {
+		g_source_remove(task->idle);
+	}
+	if (task->io_watch) {
+		g_source_remove(task->io_watch);
+	}
+	if (task->watcher) {
+		g_source_remove(task->watcher);
+	}
+
+	tasks = g_list_remove(tasks, task);
+	g_free(task->executable);
+	g_free(task->name);
+	g_free(task);
+
+	if (g_list_length(tasks) == 0) {
+		g_main_loop_quit(global_mainloop);
+	}
+
+	return;
+}
+
 static void
 proc_watcher (GPid pid, gint status, gpointer data)
 {
@@ -36,13 +85,9 @@
 
 	g_spawn_close_pid(pid);
 
-	tasks = g_list_remove(tasks, task);
-	g_free(task->executable);
-	g_free(task->name);
+	task->task_die = TRUE;
 
-	if (g_list_length(tasks) == 0) {
-		g_main_loop_quit(global_mainloop);
-	}
+	check_task_cleanup(task, FALSE);
 
 	return;
 }
@@ -53,12 +98,25 @@
 	task_t * task = (task_t *)data;
 	gchar * line;
 	gsize termloc;
-	GIOStatus status = g_io_channel_read_line (channel, &line, NULL, &termloc, NULL);
-	g_return_val_if_fail(status == G_IO_STATUS_NORMAL, FALSE);
-	line[termloc] = '\0';
-
-	g_print("%s: %s\n", task->name, line);
-	g_free(line);
+
+	do {
+		GIOStatus status = g_io_channel_read_line (channel, &line, NULL, &termloc, NULL);
+
+		if (status == G_IO_STATUS_EOF) {
+			task->text_die = TRUE;
+			check_task_cleanup(task, FALSE);
+			continue;
+		}
+
+		if (status != G_IO_STATUS_NORMAL) {
+			continue;
+		}
+
+		line[termloc] = '\0';
+
+		g_print("%s: %s\n", task->name, line);
+		g_free(line);
+	} while (G_IO_IN & g_io_channel_get_buffer_condition(channel));
 
 	return TRUE;
 }
@@ -92,10 +150,11 @@
 	g_free(argv);
 
 	GIOChannel * iochan = g_io_channel_unix_new(proc_stdout);
-	g_io_add_watch(iochan,
-	               G_IO_IN, /* conditions */
-	               proc_writes, /* func */
-	               task); /* func data */
+	g_io_channel_set_buffer_size(iochan, 10 * 1024 * 1024); /* 10 MB should be enough for anyone */
+	task->io_watch = g_io_add_watch(iochan,
+	                                G_IO_IN, /* conditions */
+	                                proc_writes, /* func */
+	                                task); /* func data */
 
 	task->watcher = g_child_watch_add(task->pid, proc_watcher, task);
 
@@ -116,14 +175,21 @@
 	g_return_val_if_fail(status == G_IO_STATUS_NORMAL, FALSE);
 	line[termloc] = '\0';
 
-	g_print("DBus address: %s\n", line);
-	g_setenv("DBUS_SESSION_BUS_ADDRESS", line, TRUE);
-	g_setenv("DBUS_STARTER_ADDRESS", line, TRUE);
-	g_setenv("DBUS_STARTER_BUS_TYPE", "session", TRUE);
+	static gboolean first_time = TRUE;
+	g_print("DBus daemon: %s\n", line);
+
+	if (first_time) {
+		first_time = FALSE;
+
+		g_setenv("DBUS_SESSION_BUS_ADDRESS", line, TRUE);
+		g_setenv("DBUS_STARTER_ADDRESS", line, TRUE);
+		g_setenv("DBUS_STARTER_BUS_TYPE", "session", TRUE);
+
+		g_list_foreach(tasks, start_task, NULL);
+	}
+
 	g_free(line);
 
-	g_list_foreach(tasks, start_task, NULL);
-
 	return TRUE;
 }
 
@@ -134,6 +200,8 @@
 	task->executable = g_strdup(value);
 	task->number = g_list_length(tasks);
 	task->parameters = NULL;
+	task->task_die = FALSE;
+	task->text_die = FALSE;
 	tasks = g_list_prepend(tasks, task);
 	return TRUE;
 }

=== modified file 'tests/Makefile.am'
--- tests/Makefile.am	2009-12-02 23:14:10 +0000
+++ tests/Makefile.am	2009-12-03 19:15:21 +0000
@@ -4,46 +4,65 @@
 DISTCLEANFILES = $(TESTS)
 
 TESTS += test-simple
-test-simple:
+test-simple: Makefile.am
 	@echo "#!/bin/sh" > $@
 	@echo $(DBUS_RUNNER) --task true >> $@
 	@chmod +x $@
 
 TESTS += test-manytask
-test-manytask:
+test-manytask: Makefile.am
 	@echo "#!/bin/sh" > $@
 	@echo $(DBUS_RUNNER) --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true --task true >> $@
 	@chmod +x $@
 
 TESTS += test-ignore
-test-ignore:
+test-ignore: Makefile.am
 	@echo "#!/bin/sh" > $@
 	@echo $(DBUS_RUNNER) --task false --ignore-return >> $@
 	@chmod +x $@
 
 TESTS += test-ignore-second
-test-ignore-second:
+test-ignore-second: Makefile.am
 	@echo "#!/bin/sh" > $@
 	@echo $(DBUS_RUNNER) --task true --task false --ignore-return >> $@
 	@chmod +x $@
 
 TESTS += test-invert
-test-invert:
+test-invert: Makefile.am
 	@echo "#!/bin/sh" > $@
 	@echo $(DBUS_RUNNER) --task false --invert-return >> $@
 	@chmod +x $@
 
 TESTS += test-invert-second
-test-invert-second:
+test-invert-second: Makefile.am
 	@echo "#!/bin/sh" > $@
 	@echo $(DBUS_RUNNER) --task true --task false --invert-return >> $@
 	@chmod +x $@
 
 TESTS += test-param
-test-param:
+test-param: Makefile.am
 	@echo "#!/bin/sh" > $@
 	@echo $(DBUS_RUNNER) --task \"$(srcdir)/delayrm.sh\" --parameter \"$(builddir)/bob\" --task touch --parameter \"$(builddir)/bob\" --ignore-return >> $@
 	@chmod +x $@
 
+TESTS += test-output
+test-output: Makefile.am
+	@echo "#!/bin/sh -e" > $@
+	@echo "$(DBUS_RUNNER) --task cat --parameter \"$(top_srcdir)/src/dbus-test-runner.c\" --task-name \"cat1\" --task cat --parameter \"$(top_srcdir)/src/dbus-test-runner.c\" --task-name \"cat2\" > testcat.output.txt" >> $@
+	@echo "echo Finding cat1 data" >> $@
+	@echo "grep ^cat1: testcat.output.txt > testcat.output.cat1.txt" >> $@
+	@echo "echo Finding cat2 data" >> $@
+	@echo "grep ^cat2: testcat.output.txt > testcat.output.cat2.txt" >> $@
+	@echo "echo Filtering cat1 data" >> $@
+	@echo "sed -e s/cat1:\\ //g testcat.output.cat1.txt > testcat.output.cat1.filtered.txt" >> $@
+	@echo "echo Filtering cat2 data" >> $@
+	@echo "sed -e s/cat2:\\ //g testcat.output.cat2.txt > testcat.output.cat2.filtered.txt" >> $@
+	@echo "echo Verifying cat 1" >> $@
+	@echo "diff testcat.output.cat1.filtered.txt \"$(top_srcdir)/src/dbus-test-runner.c\" > /dev/null" >> $@
+	@echo "echo Verifying cat 2" >> $@
+	@echo "diff testcat.output.cat2.filtered.txt \"$(top_srcdir)/src/dbus-test-runner.c\" > /dev/null" >> $@
+	@chmod +x $@
+DISTCLEANFILES += testcat.output.txt testcat.output.cat1.txt testcat.output.cat2.txt testcat.output.cat1.filtered.txt testcat.output.cat2.filtered.txt
+
 EXTRA_DIST = \
 	delayrm.sh