Re: [Qemu-devel] gtester questions/issues

[Luiz Capitulino <lcapitulino@redhat.com>]

On Thu, 09 Jun 2011 14:04:37 -0500
Anthony Liguori <aliguori@us.ibm.com> wrote:

> On 06/09/2011 01:47 PM, Luiz Capitulino wrote:
> >
> > I've started writing some tests with the glib test framework (used by the qapi
> > patches) but am facing some issues that doesn't seem to exist with check (our
> > current framework).
> >
> > Of course that it's possible that I'm missing something, in this case pointers
> > are welcome, but I must admit that my first impression wasn't positive.
> >
> > 1. Catching test abortion
> >
> > By default check runs each test on a separate process, this way it's able to
> > catch any kind of abortion (such as an invalid pointer deference) and it
> > prints a very developer friendly message:
> >
> >   Running suite(s): Memory module test suite
> >   0%: Checks: 1, Failures: 0, Errors: 1
> >   check-memory.c:20:E:Memory API:test_read_write_byte_simple:33: (after this point) Received signal 11 (Segmentation fault)
> >
> > The glib suite doesn't seem to do that, at least not by default, so this is
> > what you get on an invalid pointer:
> >
> >   ~/src/qmp-unstable/build (qapi-review)/ ./test-visiter2
> >   /qapi/visitor/input/int: Segmentation fault (core dumped)
> >   ~/src/qmp-unstable/build (qapi-review)/
> >
> > Is it possible to have check's functionality someway? I read about the
> > g_test_trap_fork() function, but one would have to use it manually in
> > each test case, this is a no-no.
> 
> I think this is a personal preference thing.  I think having fork() be 
> optional is great because it makes it easier to use common state for 
> multiple test cases.

Coupling test-cases like this is almost always a bad thing. Test-cases have
to be independent from each other so that they can be run and debugged
individually, also a failing test won't bring the whole suite down, as this
makes a failing report useless.

That said, you can still do this sharing without sacrificing essential features.
Like disabling the fork mode altogether or subdividing test cases.

Anyway, If there's a non-ultra cumbersome way to use g_test_trap_fork() (or any
other workaround) to catch segfaults and abortions, then fine. Otherwise I
consider this a blocker, as any code we're going to test in qemu can possibly
crash. This is really a very basic feature that a C unit-test framework can
offer.

> 
> >
> > 2. Memory leaks
> >
> > If you write something as simple as:
> >
> > int main(int argc, char **argv)
> > {
> >      g_test_init(&argc,&argv, NULL);
> >      return g_test_run();
> > }
> >
> > And run it under valgrind, you'll see this leaks memory. If you add
> > tests cases to it you'll see that it floods memory. This makes it almost
> > impossible to debug memory leaks.
> >
> > Is there a cleanup function I'm missing? I googled for it, but I found only
> > other people complaining about this too :(
> 
> My version of glib/valgrind doesn't have this problem.  Maybe there's a 
> valgrind filter for gtester on ubuntu and not fedora?

What's the version you're using?

> 
> >
> > Now, let me say that this will also happen with check if you it in fork mode
> > (which is the default). However, the leak goes away when you run it in
> > non-fork mode which is what you want to do if you want to do any kind of debug
> > with check (having the bug is still not acceptable though, but the fact is that
> > it won't bite you in practice).
> >
> > 3. Test output
> >
> > The default output I get when I run a gtester test is:
> >
> >   ~/src/qmp-unstable/build (qapi-review)/ ./test-visiter2
> >   /qapi/visitor/input/int: OK
> >   /qapi/visitor/input/str: OK
> >   ~/src/qmp-unstable/build (qapi-review)/
> >
> > Which is probably ok for a small amount of tests. However, you don't want to
> > look for a list of 10 or more lines to see if a test failed, you want something
> > more obvious, like what check does:
> >
> >   ~/src/qmp-unstable/build (qapi-review)/ ./check-qint
> >   Running suite(s): QInt test-suite
> >   100%: Checks: 5, Failures: 0, Errors: 0
> >   ~/src/qmp-unstable/build (qapi-review)/
> >
> > Now, I read about the gtester program and the gtester-report and I can understand
> > the wonders of a xml + html report (like having on the web page, etc) but running
> > two programs and transforming xml is not what developers want to do when they're
> > running unit-tests every few minutes (not to mention that I'm getting all kinds of
> > crashes when I run gtester-report in fedora).
> 
> I actually like the way gtester does it and the html output is quite 
> nice IMHO.
> 
> But the main motivator between gtester is that it's there.  It can be a 
> non-optional build dependency.  libcheck cannot because it's not widely 
> available/used.  It's also much harder to use libcheck since you have to 
> create a test hierarchy programmatically.

Agreed.

> The check tests have bit rotted over time to the point that they're 
> broken in the tree.

No, that's not true.

Only check-qjson has a failing test and I did that on purpose (ie. my fault).
I fixed a few issues wrt the handling of backslashes last year and realized
that some tests where missing. I added them but that one didn't pass. I was
sure it was a problem in the code (and I think I talked to you) but I didn't
know how to fix it, so I decided to let the test failing as a way to remind
me it had a problem.

check-qdict is not broken, it just requires its test file to exist in the
same directory. I never bothered to fix this because I used to build qemu
in the top directory.

Both problems are long standing because I was avoiding touching any QMP
code since the QAPI discussions started.

>  I attribute this to the fact that they aren't built 
> by default.

This is true. I doubt both problems would exist if the tests were run
in every (developer?) build.

> 
> Regards,
> 
> Anthony Liguori
> 
> > Ah, I just found out that check also has xml support but I've never
> > used it...
>