Bug 55761

Here is a patch: http://cgit.collabora.com/git/user/xclaesse/telepathy-glib.git/log/?h=g-test-dbus

I got 3 tests failing with glib 2.34.0-1ubuntu1 but they all pass with current glib master. With 2.34.0 the test actually succeed, but then in g_test_dbus_down() something does an exit() that makes the process return failure. This is weird because g_test_dbus_down() calls g_dbus_connection_set_exit_on_close (connection, FALSE); to make sure that does not happen.

actually tested with glib 2.34.0 upstream and it works nicely. I only fails with ubuntu package... Maybe a patch on their side...

> +tp_test_run (void)

The C namespace for this code is tp_tests. tp_tests_run_with_dbus_session() or tp_tests_run_with_bus() maybe?

> + TP_TESTS_SERVICES_DIR=@abs_srcdir@/services \

I would prefer these to return to @abs_srcdir@/dbus-1/services to mirror their location on real systems (/usr/share/dbus-1/services).

run-test.sh.in needs to set this environment variable, too (it should be @tpglibtestsdir@/dbus-1/services there, with dbusservicedir set accordingly).

> tp_tests_dbus_daemon_dup_or_die (void)
> {
> - TpDBusDaemon *d = tp_dbus_daemon_dup (NULL);
> + TpDBusDaemon *d;
> +
> + if (test_dbus == NULL)
> + {
> + start_dbus_session ();

I'm unsure about this function getting a non-obvious side-effect... I suppose it's not too bad, but I think I'd prefer it if you exposed tp_tests_start_dbus_session() and tp_tests_stop_dbus_session(), and called them from main() in each test that doesn't use tp_tests_run_with_dbus_session(). That way, leaking a reference to the TpDBusDaemon (which I'm sure we do, despite our best efforts) wouldn't mean we leaked a dbus-daemon process.

> With 2.34.0 the test actually succeed, but then in
> g_test_dbus_down() something does an exit() that makes the process return
> failure.

My guess would be that this is the libdbus shared connection doing its exit-on-close behaviour.

> This is weird because g_test_dbus_down() calls
> g_dbus_connection_set_exit_on_close (connection, FALSE); to make sure that
> does not happen

... but that doesn't affect libdbus.

Perhaps tp_tests_stop_dbus_session() should call dbus_g_bus_get() and turn off its exit-on-disconnect flag before terminating the session? (In practice we always leak the dbus-glib shared connection anyway.)

Then the only tests that would have to turn off exit-on-disconnect themselves would be those that create private bus connections, which seems fair.

(In reply to comment #3)
> > +tp_test_run (void)
> 
> The C namespace for this code is tp_tests. tp_tests_run_with_dbus_session()
> or tp_tests_run_with_bus() maybe?

Right, changed to tp_tests_run_with_bus().

> > + TP_TESTS_SERVICES_DIR=@abs_srcdir@/services \
> 
> I would prefer these to return to @abs_srcdir@/dbus-1/services to mirror
> their location on real systems (/usr/share/dbus-1/services).

done

> run-test.sh.in needs to set this environment variable, too (it should be
> @tpglibtestsdir@/dbus-1/services there, with dbusservicedir set accordingly).

Did not know about that run-test.sh, it was still using with-session-bus.sh.
Fixed.

> > tp_tests_dbus_daemon_dup_or_die (void)
> > {
> > - TpDBusDaemon *d = tp_dbus_daemon_dup (NULL);
> > + TpDBusDaemon *d;
> > +
> > + if (test_dbus == NULL)
> > + {
> > + start_dbus_session ();
> 
> I'm unsure about this function getting a non-obvious side-effect... I
> suppose it's not too bad, but I think I'd prefer it if you exposed
> tp_tests_start_dbus_session() and tp_tests_stop_dbus_session(), and called
> them from main() in each test that doesn't use
> tp_tests_run_with_dbus_session(). That way, leaking a reference to the
> TpDBusDaemon (which I'm sure we do, despite our best efforts) wouldn't mean
> we leaked a dbus-daemon process.

Yeah, I've done this to make change as less intrusive as possible, it's a pain to edit each unit test... ideally they should all use tp_tests_run_with_bus() but some outdated tests still does code in main().

GTestDBus fork() a cleanup process that takes care of killing the dbus-daemon if unit test doesn't.

g_test_dbus_down() also wait until the last ref on the GDBusConnection session singleton is dropped (with a timeout). I think we should do the same to ensure TpDBusDaemon isn't leaked, but that can be added later.

> > With 2.34.0 the test actually succeed, but then in
> > g_test_dbus_down() something does an exit() that makes the process return
> > failure.
> 
> My guess would be that this is the libdbus shared connection doing its
> exit-on-close behaviour.
> 
> > This is weird because g_test_dbus_down() calls
> > g_dbus_connection_set_exit_on_close (connection, FALSE); to make sure that
> > does not happen
> 
> ... but that doesn't affect libdbus.
> 
> Perhaps tp_tests_stop_dbus_session() should call dbus_g_bus_get() and turn
> off its exit-on-disconnect flag before terminating the session? (In practice
> we always leak the dbus-glib shared connection anyway.)
> 
> Then the only tests that would have to turn off exit-on-disconnect
> themselves would be those that create private bus connections, which seems
> fair.

Turned out this was not needed. It is dconf/gvfs that uses the bus session and don't release it. I've reported https://bugzilla.gnome.org/show_bug.cgi?id=685734

Fixed \o/