GNOME Bugzilla – Bug 697348
GTestDBus should unset DBUS_STARTER_ADDRESS, DBUS_STARTER_BUS_TYPE
Last modified: 2013-10-29 17:46:03 UTC
The actual terminal in gnome-terminal 3.8 is an activatable service, so it has DBUS_STARTER_ADDRESS in its environment. telepathy-glib (and possibly other libraries) prefers DBUS_STARTER_ADDRESS over DBUS_SESSION_BUS_ADDRESS when looking for "the bus with Telepathy on it", but GTestDBus doesn't clear or set the former, only the latter; so we end up trying to use the real bus when run under a recent gnome-terminal. See Bug #697261 and <https://bugs.freedesktop.org/show_bug.cgi?id=63119>. This affects both g_test_dbus_unset() and g_test_dbus_up().
Created attachment 258332 [details] [review] GTestDBus: Make sure it replaces the STARTER bus as well This breaks dbus-appinfo unit test for some reason. That test now waits undefinitly on the "activate" action that never happens. I don't understand why.
Created attachment 258333 [details] [review] Tests: fix indentation in dbus-appinfo code
Review of attachment 258332 [details] [review]: > This breaks dbus-appinfo unit test for some reason Well, presumably that's not desirable :-) ::: gio/gtestdbus.c @@ +761,3 @@ g_setenv ("DBUS_SESSION_BUS_ADDRESS", self->priv->bus_address, TRUE); + g_setenv ("DBUS_STARTER_ADDRESS", self->priv->bus_address, TRUE); + g_setenv ("DBUS_STARTER_BUS_TYPE", "session", TRUE); If you make sure g_test_dbus_unset() has been called instead, does that work any better? I think the environment that tests normally ought to use is: DBUS_SESSION_BUS_ADDRESS set, DISPLAY cleared, DBUS_STARTER_* cleared.
Created attachment 258342 [details] [review] GTestDBus: Make sure only DBUS_SESSION_BUS_ADDRESS is set by default g_test_dbus_unset() now also unset DBUS_STARTER_ADDRESS and DBUS_STARTER_BUS_TYPE. This breaks dbus-appinfo unit test because unsetting DISPLAY makes dbus-launch fail to init x11. I don't understand why all other unit tests still pass, though.
and if I comment out the g_unsetenv ("DISPLAY"); some other unit tests then fails... Simon: I'm counting on your expert eyes to understand what's going on here :p
Created attachment 258345 [details] [review] Tests: fix indentation in dbus-appinfo code
Created attachment 258346 [details] [review] Tests: add _g_test_dbus_run() and use it where possible This is to avoid having again the subtil bug in dbus-appinfo.c: session_bus_down() was called before g_test_run() so the test was running on the user's dbus session.
Created attachment 258347 [details] [review] Tests: It is useless to unset "DBUS_SESSION_BUS_ADDRESS" manually If the goal is to make sure we don't have a dbus connection, it has to call g_test_dbus_unset() instead which is much more complete. In this case, g_test_dbus_unset() is called already, so it should be fine.
Created attachment 258348 [details] [review] GTestDBus: Make sure only DBUS_SESSION_BUS_ADDRESS is set by default g_test_dbus_unset() now also unset DBUS_STARTER_ADDRESS and DBUS_STARTER_BUS_TYPE.
I'm now pondering adding g_test_dbus_unset() inside g_test_init() to make damn sure we'll never run a test that use user's session. I know g_test_init() is not part of libgio, but g_test_dbus_unset() does not use any of libgio, so it's doable. It will probably have side effects for tests that use dbus indirectly, but I think it is actually good to see all those tests messing with user's session failing.
(In reply to comment #10) > I'm now pondering adding g_test_dbus_unset() inside g_test_init() to make damn > sure we'll never run a test that use user's session. I know g_test_init() is > not part of libgio, but g_test_dbus_unset() does not use any of libgio, so it's > doable. > > It will probably have side effects for tests that use dbus indirectly, but I > think it is actually good to see all those tests messing with user's session > failing. Actually no, that would break tests that use wrapper like with-session-bus.sh. We probably can't force them to port their tests to GTestDBus.
Review of attachment 258345 [details] [review]: This is a matter of taste, but I prefer Xavier's version.
Review of attachment 258346 [details] [review]: I like this.
Review of attachment 258347 [details] [review]: Looks good to me.
Review of attachment 258348 [details] [review]: Looks good to me, and should fix the "Telepathy in gnome-terminal" use-case.
Review of attachment 258345 [details] [review]: I prefer mine :)
Review of attachment 258346 [details] [review]: This is a nice improvement. Thanks!
Review of attachment 258348 [details] [review]: Looks good as well. Go ahead.
Review of attachment 258347 [details] [review]: Agreed. This testcase is both good and bad in the sense that it's doing the up/down per test (good), but that it is having to do it manually (bad).... Having a support for this in gtester in some form or another would be nice...
Review of attachment 258346 [details] [review]: Maybe call it session_bus_run_tests() to match the rest of the stuff in that file... You'll need to rebase this patch anyway since it no longer applies.
Thanks. I skipped attachment #258345 [details], renamed _g_test_dbus_run() to session_bus_run() and pushed to master.