[Bug 32902] review Pidgin log store and dbus testsuite

[bugzilla-daemon at freedesktop.org]

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

--- Comment #5 from Cosimo Alfarano <cosimo.alfarano at collabora.co.uk> 2011-01-14 10:04:59 PST ---
> >+static GDate *
> >+log_store_pidgin_get_time (const gchar *filename)
> >+  dir = g_dir_open (directory, 0, NULL);
> >+  if (dir == NULL)
> >+    {
> >+      DEBUG ("Could not open directory:'%s'", directory);
> >+      return NULL;
> >+    }

I see this problem only in get_dates (fixed).

> You need to free 'directory' inside the if. Also same thing in
> log_store_pidgin_get_dates().
> 
> >+          /* FIXME do we need to set the entity type as well? */
> >+
> >+          /* FIXME a better way to obtain a log-id from purlple? */
> >+          if (TRUE)
> >+            {
> Could you elaborate more about those, also I don't like so much this runtime if
> (TRUE).
> 
> >+              TPL_ENTRY_DIRECTION_NONE);
> Can't we get the direction here ?

Not 100% accurate, direction is anyway useless and will be removed, but so far
I'll set it only if is_html==TRUE, which means I can deduct the direction from
the logs. Otherwise leave it to NONE.

The same is valid for tpl_entity_set_entity_type(), with the difference that,
when is_html==FALSE, it's not possible to understand if the log is actually
being send by SELF or not.
This for example will result in Empathy wrongly colouring the log window as it
won't understand the user's log. 

> >diff --git a/tests/dbus/test-tpl-log-store-pidgin.c b/tests/dbus/test-tpl-log-store-pidgin.c
> >+++ b/tests/dbus/test-tpl-log-store-pidgin.c
> >@@ -0,0 +1,576 @@
> >+/* FIXME: hugly kludge: we need to include all the declarations which are used
> >+ * by the GInterface and thus not in the -internal.h */
> >+#include "../../telepathy-logger/log-store-pidgin.c"
> 
> It's really hugly, anything we could do about that ?

The way to fix it would be export symbols in a .h just for testing uses.

tp-glib has some testes that use this terrible hack, it's from which I copied
it actually. It does not make it any better but I think that it can go with it.

The only side effect is that DEBUG() won't work properly with
TPL_DEBUG=testsuite as DEBUG_FLAG has been set in the included file.

-- 
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.