Bug 69283 - tests/dbus/cm.c: un-fork test_dbus_ready() and test_dbus_fallback()
Summary: tests/dbus/cm.c: un-fork test_dbus_ready() and test_dbus_fallback()
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: tp-glib (show other bugs)
Version: git master
Hardware: Other All
: medium normal
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2013-09-12 18:05 UTC by Simon McVittie
Modified: 2013-09-13 16:28 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
1/2] tp_dbus_properties_mixin_dup_all: make public (7.65 KB, patch)
2013-09-12 18:07 UTC, Simon McVittie
Details | Splinter Review
2/2] cm test: test "drop name on get" on a modern CM (11.15 KB, patch)
2013-09-12 18:07 UTC, Simon McVittie
Details | Splinter Review
[next] TpConnectionManager: retry introspection after CM exits, up to once (9.54 KB, patch)
2013-09-12 18:39 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2013-09-12 18:05:12 UTC
When I tried to merge telepathy-glib 0.20.4 via master into next, I ran into the problem that in the regression test for Bug #67183, I added test coverage that only works for archaic (no Properties) ConnectionManager objects.
Comment 1 Simon McVittie 2013-09-12 18:07:03 UTC
Created attachment 85732 [details] [review]
1/2] tp_dbus_properties_mixin_dup_all: make public

There's no real reason not to - anything that implements D-Bus
properties is clearly going to have this method in some form.
Also, my next commit needs it.
Comment 2 Simon McVittie 2013-09-12 18:07:17 UTC
Created attachment 85733 [details] [review]
2/2] cm test: test "drop name on get" on a modern CM

We were only testing this case on the archaic CM, but that's
undesirable when converting to Telepathy 1.0, since a CM with no
D-Bus properties will cease to be a valid CM.

The diff is a little confusing because I'm basically turning
test_dbus_fallback() and its fork test_dbus_ready() back into
the same function. The merged version does everything that
either of its precursors did.
Comment 3 Simon McVittie 2013-09-12 18:39:57 UTC
Created attachment 85739 [details] [review]
[next] TpConnectionManager: retry introspection after CM exits, up  to once

Many connection managers automatically exit after 5 seconds of
inactivity. If the CM has no .manager file *and* exits in this way
while we are introspecting it, we would previously consider it to have
failed introspection - but with sufficiently unfortunate timing,
that can result in empathy-accounts not considering Haze to exist.

To avoid this, without going into an infinite loop if the CM fails to
introspect, retry once, but only once.

[This is a forward-port to Telepathy 1.0.]

---

This is commit 60120429 (Bug #67183) squashed into Attachment #85733 [details] and manually applied to next, to be applied on top of a cherry-pick of Attachment #85732 [details] (which is itself non-trivial because there is something else in the -internal.h on next).

It also contains a couple of irrelevant patch-bands:

* cope with the protocols coming out in reversed order
* reinstate the ACTIVATE_CM test

because they were in master, and I suspect they were previously mis-merged into next. Yay conflicts.

I'll put commit hash references in when I've committed the original.
Comment 4 Guillaume Desmottes 2013-09-13 12:08:16 UTC
Comment on attachment 85732 [details] [review]
1/2] tp_dbus_properties_mixin_dup_all: make public

Review of attachment 85732 [details] [review]:
-----------------------------------------------------------------

++
Comment 5 Guillaume Desmottes 2013-09-13 12:11:11 UTC
Comment on attachment 85733 [details] [review]
2/2] cm test: test "drop name on get" on a modern CM

Review of attachment 85733 [details] [review]:
-----------------------------------------------------------------

++
Comment 6 Guillaume Desmottes 2013-09-13 12:14:14 UTC
Comment on attachment 85739 [details] [review]
[next] TpConnectionManager: retry introspection after CM exits, up  to once

Review of attachment 85739 [details] [review]:
-----------------------------------------------------------------

++
Comment 7 Simon McVittie 2013-09-13 16:28:25 UTC
Fixed, and 'next' is now up to date with master again.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.