[Bug 43598] Add high-level API for Proto.I.Addressing interface

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Sat Dec 17 11:59:22 CET 2011


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

--- Comment #10 from Olli Salli <ollisal at gmail.com> 2011-12-17 02:59:22 PST ---
(In reply to comment #9)
> Branch updated with the following tests:
> 1) all info in .manager file
> 2) no .manager file, but some immutable properties present, though not all
> 3) all immutable props present
> 4) no .manager file, no immutable properties present
> 

The tests look nice, except for one thing:

+    static int introspectionCalled;

Why are these statics? Apart from the reset thing (which should be a method on
the CM helper object, or perhaps just creating a new CMHelper object in the
beginning of each test case as needed, and deleting it in cleanup() IMO), you
seem to access them through an instance pointer. This looks a bit messy to me,
could you please clean it up slightly?

Having them as non-statics would make the test more flexible wrt. future
changes, especially ones where we have multiple adaptors of a single kind (for
multiple protocols) instantiated at a time, so we should do it unless there is
some really big benefit for the current usage in making them statics.

> I used adaptors here as it was the best way to properly test this without
> adding backdoors.
> 

Indeed, the backdoors might have been a bit complicated to check all the cases?
Anyway, great that you went that (bothersome) distance to implement the fake
services instead.

> I also fixed an issue with the introspection code.

Ah, it didn't properly extract the interfaces from received properties before
checking for them to decide what to introspect next? Very good fix - this is
why we unit-test our code! (cf.
http://blogs.msdn.com/cfs-filesystemfile.ashx/__key/communityserver-blogs-components-weblogfiles/00-00-01-32-02-metablogapi/7317.image_5F00_0F65063B.png)

I assume the test coverage is now very much in the green zone, with all these
important tests?

Overall, a very good job. After perhaps cleaning up the introspection counters
a bit, please merge, along with the conn.i.addressing branch you based on top
of this.

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



More information about the telepathy-bugs mailing list