[Bug 23684] Gabble advertize an avatar's sha1 in presence stanza without following XEP-0153

[bugzilla-daemon at freedesktop.org]

http://bugs.freedesktop.org/show_bug.cgi?id=23684





--- Comment #2 from Alban Crequy <alban.crequy at collabora.co.uk>  2009-09-09 11:56:28 PST ---
(In reply to comment #1)
> The actual code looks good to me for 0.8 and 0.9, with some stylistic
> complaints. The regression test, not so much.

Do you mean the regression test is not good for 0.8 and 0.9, or that your
complaints on the regression test are not stylistic?

> > +        }
> > +      else
> > +        presence->avatar_sha1 = g_strdup (sha1);
> 
> Put the else clause in {} to be consistent with the if clause, please.

Fixed.

> > priv->conn->parent.self_handle
> 
> I'd prefer to have a local, "TpBaseConnection *base_conn = (TpBaseConnection *)
> priv->conn;", and use base_conn->self_handle. I've been convinced that this is
> substantially better style.

Fixed.

> > +    event = q.expect('stream-presence')
> > +    event = q.expect('dbus-signal', signal='AvatarUpdated')
> > +    assert event.args[0] == 1, event.args
> > +    assert event.args[1] == "", event.args
> > +    event = q.expect('stream-iq', to=None, query_ns='vcard-temp',
> > +            query_name='vCard')
> 
> This is a race - the tests read from the D-Bus socket and the stream socket in
> a non-deterministic order. You'll have to use:
> 
> stream_presence, avatar_update, stream_iq = q.expect_many(EventPattern(...

Indeed. However, I wanted to check that stream-presence arrives before
stream-iq as it is required by the XEP-0153 section 4.4 bullet 1 and 2.

I don't know whether it is possible to do with the test framework, so I just
fixed the race, and thus remove that check.

Branch pushed again.


-- 
Configure bugmail: http://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.