Re: Comments on glib testing framework

[Tim Janik <timj imendio com>]

On Wed, 9 Jan 2008, Tommi Komulainen wrote:

> Hi,

> make -k test probably shouldn't abort gtester on first failing assertion

hm, currently, we have these test framework makefile rules:
test:		run all tests recursively, abort on first error
test-report:	run tests in subdirs, generate report, continue on error
perf-report:	run tests like test-report, include performance tests via -m perf
full-report:	like test-report, include all tests with -m perf and -m slow

so if you want to run tests and not abort, you're probably looking for
test-report:. one rule needs to be there to abort immediately for developers
to trace test failures, and currently that is test:.
i wouldn't know of a portable way to detect "make -k" invocations to make
the test: rule behave differently. and, is there really a need for that,
given that you could run test-report: instead?


> glib/gtestutils.h references g_object_unref() which requires gobject.

this is "hidden" in a macro ;) i.e.:
  #define g_test_queue_unref(gobject) g_test_queue_destroy (g_object_unref, gobject)

so if you use g_test_queue_unref() on a GObject, you usually need to have
glib-object.h included already, and g_test_queue_unref() will work without
problems.
(i guess one could argue for g_test_queue_unref() to be moved to a gobject/
header for "cleanliness", but then we'd have to find a proper place for it,
and probably have documentation splits as well...)

> It would be nice to have g_assert_cmpenum and g_assert_cmpflags to
> pretty print the name of enum/flags rather than plain numerical value
> (See example code for cmpenum attached, needs to be in gobject.) For
> example:
>
>        assertion failed (mode == GTK_SIZE_GROUP_HORIZONTAL): (3 (GTK_SIZE_GROUP_BOTH) == 1 (GTK_SIZE_GROUP_HORIZONTAL))

hm, it looks a bit cumbersome to use, since you always need to provide
to correct enum type id as well:
  g_assert_cmpenum (mode, ==, GTK_SIZE_GROUP_BOTH, GTK_TYPE_SIZE_GROUP_MODE);

judging from other gtk code errors, those type ids are easily gotten wrong
if unchecked, and in your implementation, the type id is used very seldomly,
i.e. only if the actual assertion fails...

also, g_assert_cmpint() will already provide a lot of information (in
particular the stringified assertion macro arguments):
  gint mode = GTK_SIZE_GROUP_HORIZONTAL;
  g_assert_cmpint (mode, ==, GTK_SIZE_GROUP_BOTH);
generates
  ** ERROR:(testing.c:195):main: assertion failed (mode == GTK_SIZE_GROUP_BOTH): (1 == 3)

i guess you're mostly aiming for cases where both macro arguments are
variables?

> While we are on this topic, does it make sense to add something like
> g_enum_get_name (GType, gint)? The g_enum_* functions can be used to do
> this, but are a bit cumbersome to use.

this has been raised a lot of times. there's one thing to keep in mind
here, like all other types, enum classes may be implemented by a plugin
that needs to be dlopen()ed. so strings provided by the enum class will
not neccessarily persist in memory, i.e. g_enum_get_name() can not return
a const char* but would have to g_strdup() the returned string.

and, adding such API will hide the fact that classes my need to be loaded,
which can lead to code like:
  gchar *name = g_enum_get_name (GTK_TYPE_SIZE_GROUP_MODE, GTK_SIZE_GROUP_BOTH);
  gchar *nick = g_enum_get_nick (GTK_TYPE_SIZE_GROUP_MODE, GTK_SIZE_GROUP_BOTH);
  gchar *blurb = g_enum_get_blurb (GTK_TYPE_SIZE_GROUP_MODE, GTK_SIZE_GROUP_BOTH);
  g_print ("enum value: name=%s nick=%s blurb=%s\n", name, nick, blurb);
  g_free (name);
  g_free (nick);
  g_free (blurb);
which looks harmless, but behind the scenes does
  dlclose (dlopen("EnumPLugin.so"));
3 times.

> gtk_test_create_widget is nice, but unfortunately won't be usable in the
> cases where a widget does more than a g_object_new call in its _new
> function (GtkSpinButton, text variants of comboboxes, GtkAspectFrame,
> any other third party widget, ...). Maybe not too important, but worth
> noting.

widgets that do more than g_object_new() in their _new() functions are
broken and need to be fixed to provide the functionality of all
_new(arguments) through the GObject property interface.
(they are also problematic for gui builders, docu generation and
language bindings).

> gtk_test_teardown. See bug #507256
>
> There are two technologies that could help in the creation of testcases:
>      * A way to record a series of events and replay them at will.
>        (Already exists?)

yeah, Gerd (Gtk+ Event Recorder) does this:
  http://testbit.eu/~timj/historic/gerd/
the code is faily old though (2000), and with Gtk+/Gdk's infrastructure
will also work on X11 only.

>      * A way to record either the static aspect (screenshot) or an
>        extended period of time (video, or series of images matched with
>        a checkpoint) and compare it against a pre-defined image/video.

this has been implemented in a spun off of Gerd by some solaris people
a couple years ago. doing pixel-by-pixel comparisons has some fundamental
problems for toolkit testing though, they simply turn out to be too fragile.
so you can end up breaking large numbers of test cases by comparatively
simple changes, such as padding fixes, font hinting fixes (e.g. due to a new
font or freetype version), gamma corrections, dithering changes, etc.
the list is long. pixel correctness tests turn out to be most useful for
lower level libraries like cairo.

> --
> Tommi Komulainen                            <tommi komulainen nokia com>

---
ciaoTJ