Attachment 83723 Details for Bug 67828 – roster/groups: don't rely on an implementation detail

[patch] roster/groups: don't rely on an implementation detail

0001-roster-groups-don-t-rely-on-an-implementation-detail.patch (text/plain), 4.87 KB, created by Simon McVittie on 2013-08-06 14:21:31 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-08-06 14:21:31 UTC

Size: 4.87 KB

patch

obsolete

>From deb23eb2b441994bf6a4666507b4ad725f75c46f Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Tue, 6 Aug 2013 15:13:25 +0100
>Subject: [PATCH] roster/groups: don't rely on an implementation detail
>
>Before telepathy-glib 0.20.3 and 0.21.1, we had this incorrect sequence
>(pseudocode) for each group:
>
>* NewChannels(the group)
>* GroupsChanged([the group], added: [...], removed: [])
>* NewChannels(the group)
>
>In 0.20.3 and 0.20.1, we removed the second emission of NewChannels.
>Unfortunately, that broke this test, which was specifically expecting
>GroupsChanged followed by NewChannels.
>
>Rather than reversing the assumption, I'm doing it properly, by
>expecting the events in no particular order.
>
>Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
>---
> tests/twisted/roster/groups.py | 39 +++++++++++++++++++++++++--------------
> tests/twisted/rostertest.py    | 17 +++++++++++++++--
> 2 files changed, 40 insertions(+), 16 deletions(-)
>
>diff --git a/tests/twisted/roster/groups.py b/tests/twisted/roster/groups.py
>index d9fd67d..a0ba99b 100644
>--- a/tests/twisted/roster/groups.py
>+++ b/tests/twisted/roster/groups.py
>@@ -53,24 +53,35 @@ def test(q, bus, conn, stream):
> 
>     stream.send(event.stanza)
> 
>-    # slight implementation detail: TpBaseContactList emits ContactsChanged
>-    # etc. before it announces its channels, and it emits one CGC per group.
>-    s1, s2 = q.expect_many(
>-        EventPattern('dbus-signal', signal='GroupsChanged',
>-            interface=cs.CONN_IFACE_CONTACT_GROUPS, path=conn.object_path,
>-            predicate=lambda e: 'women' in e.args[1]),
>-        EventPattern('dbus-signal', signal='GroupsChanged',
>-            interface=cs.CONN_IFACE_CONTACT_GROUPS, path=conn.object_path,
>-            predicate=lambda e: 'men' in e.args[1]),
>-        )
>+    # Avoid relying on the implementation detail of exactly when
>+    # TpBaseContactList emits ContactsChanged, relative to when it
>+    # announces its channels. Prior to 0.20.3, 0.21.1 it would
>+    # announce the channels, emit GroupsChanged, then announce the channels
>+    # again... which was a bug, but it turned out this test relied on it.
>+    #
>+    # We do still rely on the implementation detail that we emit GroupsChanged
>+    # once per group with all of its members, not once per contact with all
>+    # of their groups. On a typical contact list, there are more contacts
>+    # than groups, so that'll work out smaller.
>+
>+    pairs, groups_changed = expect_contact_list_signals(q, bus, conn, [],
>+            ['men', 'women'],
>+            [
>+                EventPattern('dbus-signal', signal='GroupsChanged',
>+                    interface=cs.CONN_IFACE_CONTACT_GROUPS,
>+                    path=conn.object_path,
>+                    predicate=lambda e: 'women' in e.args[1]),
>+                EventPattern('dbus-signal', signal='GroupsChanged',
>+                    interface=cs.CONN_IFACE_CONTACT_GROUPS,
>+                    path=conn.object_path,
>+                    predicate=lambda e: 'men' in e.args[1]),
>+            ])
> 
>     amy, bob, che = conn.RequestHandles(cs.HT_CONTACT,
>             ['amy@foo.com', 'bob@foo.com', 'che@foo.com'])
> 
>-    assertEquals([[amy], ['women'], []], s1.args)
>-    assertEquals([[bob, che], ['men'], []], s2.args)
>-
>-    pairs = expect_contact_list_signals(q, bus, conn, [], ['men', 'women'])
>+    assertEquals([[amy], ['women'], []], groups_changed[0].args)
>+    assertEquals([[bob, che], ['men'], []], groups_changed[1].args)
> 
>     q.expect('dbus-signal', signal='ContactListStateChanged',
>             args=[cs.CONTACT_LIST_STATE_SUCCESS])
>diff --git a/tests/twisted/rostertest.py b/tests/twisted/rostertest.py
>index 3d31ad4..7e9f121 100644
>--- a/tests/twisted/rostertest.py
>+++ b/tests/twisted/rostertest.py
>@@ -68,10 +68,14 @@ def get_contact_list_event_patterns(q, bus, conn, expected_handle_type, name):
>                 predicate=new_channels_predicate)
>             )
> 
>-def expect_contact_list_signals(q, bus, conn, lists, groups=[]):
>+def expect_contact_list_signals(q, bus, conn, lists, groups=[],
>+        expect_more=None):
>     assert lists or groups
> 
>-    eps = []
>+    if expect_more is None:
>+        eps = []
>+    else:
>+        eps = expect_more[:]
> 
>     for name in lists:
>         eps.extend(get_contact_list_event_patterns(q, bus, conn,
>@@ -83,6 +87,11 @@ def expect_contact_list_signals(q, bus, conn, lists, groups=[]):
> 
>     events = q.expect_many(*eps)
>     ret = []
>+    more = []
>+
>+    if expect_more is not None:
>+        for ep in expect_more:
>+            more.append(events.pop(0))
> 
>     for name in lists:
>         old_signal = events.pop(0)
>@@ -95,6 +104,10 @@ def expect_contact_list_signals(q, bus, conn, lists, groups=[]):
>         ret.append((old_signal, new_signal))
> 
>     assert len(events) == 0
>+
>+    if expect_more is not None:
>+        return ret, more
>+
>     return ret
> 
> def check_contact_list_signals(q, bus, conn, signals,
>-- 
>1.8.4.rc1
>

Actions: View | Splinter Review

Attachments on bug 67828: 83723

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.