Re: reftests

[Kristian Rietveld <kris gtk org>]

On May 5, 2011, at 6:24 PM, Benjamin Otte wrote:
> On Thu, May 5, 2011 at 10:18 AM, Kristian Rietveld <kris gtk org> wrote:
>> As we've already discussed on IRC some time ago, I would really like to see all
>> GTK+ unit tests in one single place, instead of in several different places in the
>> source code.  We really need people to run the unit tests more often and thus
>> this needs to be made easy (like you also mention in your enumeration above),
>> I don't think putting different unit tests at different places makes this easier.
>> 
> I do agree with putting all things in one location. However, I do not
> agree with "people need to run tests more often". I think running
> tests is a job for machines.

Yes, ideally, we would have a buildbot that runs all tests after compiling a new revision of GTK+.  Of course in addition to people that run the tests after refactoring a part of the source code before committing.


>> We can always distribute the unit tests as a separate tarball if that will help, can't we?
>> 
> Well yeah, but then it still requires compilation of something. The
> approach with "Here, open this in glade" and "Does it look like this
> screenshot?" is a lot better, because it's very easy.

For testing layout and rendering it will work very well, but I do not have the impression that all tests can be made as easy as this.  For example, the tree view scrolling tests can definitely not be done in terms of creating a glade file and comparing screenshots.  However, it would definitely be interesting to include reference screenshots for a few of the tests (not all, that would be too much).

Which raises another question, would it be a good idea or make sense to merge the image differ into the GTK+ test utils so that other tests (e.g. the tree view scrolling test suite) can make use of it?


>> Another question: why was gtk-reftest put in gtk+/tests/reftests/gtk-reftest instead of in gtk+/gtk/tests/gtk-reftest, with a subdirectory reftests containing the glade files?  Then on make check for the GTK+ unit tests, the reftests would automatically be executed as well.  Currently, you also need to compile a GTK+ checkout to use reftests, right?
>> 
> I never liked the idea of putting tests in the same directory tree as
> the actual source code, because in the good case it creates spam to
> stdout (about entering directories I don't care about) and in the
> worst case it actually compiles tests, so it not only spams stdout but
> it also takes lots of time relinking. If I am actively working on

The relinking annoys me as well which is why I started to compile in the gtk/ subdirectory with make all-am to avoid that :)

> So I do think that tests, just like documentation should live outside
> of the normal source tree. Which is why I put it there.

Makes sense and I have to add that I do not have a very strong opinion with regard to where exactly the tests are located.  However, it looks like there are two main reasons the tests were put at this location:

1) There seems to be some logic behind where frequently run and less frequently tests are located.  Quoting from [1]:

``
1) Figure a place for the test case. For this it's useful to keep in mind
   that make check will traverse CWD recursively. So tests that should be
   run often when glib, gdk or gtk changed should go into glib/glib/tests/,
   gtk+/gtk/tests/ or gtk+/gdk/tests/. Tests more thorough or planned to
   be run less frequently can go into glib/tests/ or gtk+/tests/. This is
   e.g. the case for the generic object property tester in
   gtk+/tests/objecttests.c. To sum up:
     glib/tests/		# less frequently run GLib tests
     glib/glib/tests/		# frequent GLib testing
     glib/gobject/tests/	# frequent GObject testing
     gtk+/tests/		# less frequently run Gdk & Gtk+ tests
     gtk+/gdk/tests/		# frequent Gdk testing
     gtk+/gtk/tests/		# frequent Gtk+ testing
''

I think this only makes sense if we want to continue to maintain this division between frequently and less frequently run tests.

2) Tests that are maintained outside the normal source tree could be subject to becoming unmaintained quite quickly [2], which in turn leads to tests being run less often.  However, if the tests will still live under gtk+/unittest/ and are part of the default full build, then this might not be that big of a problem.

>> These sounds like numbers I would expect.  What in GTest would need improvement to realize this?
>> 
> GTest mainly needs usability improvements such as those you pointed
> out by those error messages. (I'm sorry if I offended you by taking a
> test written by you as the bad example, I just took a random file from
> gtk/tests as an example of why our current approach is bad.) The
> problem is that currently running tests (or a single test) manually is
> complicated and oftentimes ends up with unparsable error messages that
> often are no help in actually figuring out what got broken.

I fully agree the error messages should be parsable by humans and I will try to find time to improve the tests I wrote in this respect.  I will try out what Behdad pointed out and otherwise come up with some ideas for improving the test API.


>> About organization, I think for one all GTK+ unit tests should be in one place (and the GDK tests in another place).
>> 
> Yes, I would very much suggest tests/ for this. I don't care a whole
> lot if we have tests/gdk and tests/gtk, because I think it's hard to
> test GDK without also testing GTK as GTK has all the niceties that you
> want to have when writing tests (like ui files) and it's very
> cumbersome to write to the GDK API.

The only issue I have with tests/ is that non-automated and automated tests might be intermixed, which is IMHO confusing.  Perhaps a subdirectory unittest/ under tests/ would work?


>> Secondly, we also need to develop a consistent naming scheme for tests.  Unit tests currently have different ways of naming tests:
>> 
>> /FilterModel/self/verify-test-suite:
>> /expander/click-expander:
>> /recent-manager/get-default:
>> /tests/column-new:                       (these are for icon view)
>> /Builder/Window:
>> 
> I would attribute that to GTester. The path naming idea is kind of
> useless, because what you actually need to remember is the name of the
> test binary (so that you can run it), not the name of the actual test.

The test paths are there so that you can run a specific test from a given binary.  So a specific test of the unit tests for GtkLabel.  This is to implement the possibility of other test frameworks to point at a test to run by pointing out the class and test method (see [1]).  If a test suite contains 200+ tests and you are focussing on fixing one of these, this feature is quite useful.

> There is no way to run "all GtkLabel tests" by saying gtester -p
> /tests/gtk/widgets/label and that way making it grab all tests in the
> testsuite and make it run those that match this path. Instead you run
> tests/gtk/widgets/label and that's it.

I usually run the binary in this case, but to me this sounds like an interesting feature for gtester.  What would be nice in conjunction would maybe be regexp matching for paths.

> You can also see that gtk-reftest basically ignores the test paths -
> or better: abuses them to store filenames in them so that you actually
> get useful output when running a test, because it tells you the actual
> file that failed.

This makes sense.  For the filter model and tree view scrolling unit tests I have also tried to indicate the exact test in the path, so you have an idea what has failed (provided you are familiar with the scrolling API call in this case).


regards,

-kris.


[1] http://mail.gnome.org/archives/gtk-devel-list/2007-November/msg00000.html
[2] http://mail.gnome.org/archives/gtk-devel-list/2007-December/msg00181.html