[telepathy-gabble-0.16] telepathy-gabble: roster/groups: don' t rely on an implementation detail

[Simon McVittie]

Module: telepathy-gabble
Branch: telepathy-gabble-0.16
Commit: 08ac84725bdc135e199146f180be2ea474b452c9
URL:    http://cgit.freedesktop.org/telepathy/telepathy-gabble/commit/?id=08ac84725bdc135e199146f180be2ea474b452c9

Author: Simon McVittie <simon.mcvittie at collabora.co.uk>
Date:   Tue Aug  6 15:28:24 2013 +0100

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 at collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=67828
Reviewed-by: Guillaume Desmottes <guillaume.desmottes at 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 at foo.com', 'bob at foo.com', 'che at 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,