On Mon, 9 Oct 2006, Iago Toral Quiroga wrote:
Hi, as part of the build-brigade (http://live.gnome.org/BuildBrigade) I have been working a bit on developing a set of unit tests for Gtk+ using Check (http://check.sourceforge.net/).
Gtk+ definietely lacks a unit test frame work, so it's good to try to get the ball rolling on this some. i've recently worked on improving unit test integration in another project, a brief summary of that can be found in my last blog entry: http://blogs.gnome.org/view/timj/2006/10/23/0 # Beast and unit testing
I have set up a temporary modified version of Gtk+ here: cvs -d :pserver:anonymous cvs igalia com:/var/publiccvs co gtk+ Currently, this modified version includes a small set of unit tests for several widgets that would grow in ammount if the project seems interesting to the Gtk+ community.
first i have to say that copying a random Gtk+ CVS version and adding tests to it is not the most conveninent review basis. getting a reviewable diff of your changes took figuring of an exact CVS version (-D "2006/09/17 11:59:00") and significant manual cleanups. i've attached the resulting diff for future reference. second, your code is missing a license, releasing/publishing code should always be accompanied by *some* kind of license information. in our current society, there's little point in publishing something without telling what can legally be done and not done with the published material.
I would like to know your opinion about this project (the tests), specially if you think it is interesting enough to be (now or in the future) inside the official GTK+ repository. In case you think it is interesting, I would like to know if you have any recomendations or suggestions about whatever you think might improve it. Any feedback would be very appreciated.
ok, running your tests and reading through some of your code, i collected a few comments: i'm not very fond of the introduction of a new library/package dependency for Gtk+ (or GLib) just for writing unit tests. Check in particular has a few quirks that i find particularly undesirable, such as the prefix/postfix magic macros around each test function which also tend to break C syntax parsers, see: http://bugzilla.gnome.org/show_bug.cgi?id=364672 for more details. other oddities involve e.g. its automake/configure.in integration. i'll get into more details about this in another email. the way you're using fail_if(assert_expr,error_string) tends to state too much the obvious, e.g.: + button = GTK_BUTTON (gtk_button_new_with_label (set_label));+ + fail_if (!GTK_IS_BUTTON (button),
+ "gtk_button_new_with_label fails"); this can really be shortened to: button = gtk_button_new_with_label (label); ASSERT (GTK_IS_BUTTON (button)); other things are arguably not worth checking: + get_label = gtk_button_get_label (button);+ + fail_if (strcmp (set_label, get_label) != 0,
+ "Retrieved label is not the same as the one used at construction: (%s,%s)",+ set_label, get_label); i.e. i'd not be surprised if a future or tailored version of Gtk+
returned a translated or maybe space-stripped string here. and then there's a number of tests which are plain wrong (edited for brevity): + GtkWidget *widget = NULL; + widget = gtk_hbox_new (0, FALSE); + /* Test 3 */ + get_text_buffer = gtk_text_view_get_buffer (GTK_TEXT_VIEW (widget)); + fail_if (get_text_buffer != NULL, + "gtk_text_view_get_buffer does not return NULL when " + "applied to an object that is not a GtkTextView"); the g_return_val_if_fail() statement in gtk_text_view_get_buffer() is NOT part of the API contract, it may even be optimized away in some configurations. the point in having unit tests is to check that the use cases meant to be covered by the code base are working correctly. not that things which weren'taccounted for in the design and implementation do not work. (if that was the case, the number of tests we had to write would be endless.)
that's simply because the libraries get used for what they CAN do, not what they CANNOT do ;) you should call gtk_init (&argc, &argv); with real argc/argv from main(). using: + int argc = 0; + gtk_init (&argc, NULL); all over the place is not the best idea (maybe this is due to using Check in fork mode, but it's still a bad idea), because this elimintaes the ability to (manually) try out test cases in different modes (--sync, --display, --test-slow, etc...) about using fork mode, there really is no need to fork at all for a test, except when checking tests that are supposed to fail (which is rarely the case), e.g. when you need to assert proper abort()-like behavior of e.g. g_error().
Iago.
in general, i think it's great that you started on writing unit tests for Gtk+, however, i'd say that what Gtk+ needs differs significantly fromyour approach. i'll provide more detailed thoughts on this in a follow up.
--- ciaoTJ
Attachment:
xutpatch.diff.bz2
Description: xutpatch.diff.bz2