[Bug 38206] Tp::StreamTubeChannel (and subclasses) don't have unit tests

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Thu Jun 30 12:53:35 CEST 2011


https://bugs.freedesktop.org/show_bug.cgi?id=38206

--- Comment #9 from Olli Salli <ollisal at gmail.com> 2011-06-30 03:53:35 PDT ---
The offer tests and specifically the checks for the map contents look okay to
me.

An API issue:

+ * This method is only useful if TubeChannel::addressType() is either
#SocketAddressTypeUnix or
+ * #SocketAddressTypeAbstractUnix and TubeChannel::accessControl() is
+ * #SocketAccessControlCredentials.
+ *

The user doesn't set the addressType() or accessControl(). The user passes true
or false as the requiresCredentials param to offerUnixSocket() which they're
using. Hence, you must refer to those concepts rather than the lower-level
accessors - the user doesn't have any idea how to make this method "useful"
otherwise!

The same goes for the warning messages.

For that reason, I don't think it's necessarily a great idea to have a
accessControl() public accessor rather than e.g. some passesCredentials()
accessor (with the logic that because you told offerUnixSocket() that your
socket requires credentials, the CM will pass you credentials when connecting).

In a word, *consistency*.

The above method's description should also follow the general short descr -
what it does - when can it be used pattern. Currently the long description
begins with the quoted chapter, which I'd like moved together with the features
it needs etc, after the actual description of the mapping.

> The test to check whether accept fails if the channel is invalidated will come
> next. Any other tests I should consider?

That's the most important outstanding bit, with the channel invalidated at
various different stages of the process.

I'd also like a version of the onNewConnection() slot which checks that when
that signal is emitted, the connectionsFor*..., contactsFor.. maps are
populated with the information for the new connection. The current test only
checks the map contents potentially significantly later 

Oh, you should also probably check that ConnectionClosed is correctly processed
and relayed. In that, however, *the maps should still contain the data in the
slots for the Qt signal* so you can figure out which connection it was.

Other than that, can't think of much - but have you looked at lcov-check, if
any significant portions of the code are still red?

The StreamTube stuff is finally looking rather mature, good work!

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



More information about the telepathy-bugs mailing list