qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] gtester questions/issues
@ 2011-06-09 18:47 Luiz Capitulino
  2011-06-09 19:04 ` Anthony Liguori
  0 siblings, 1 reply; 9+ messages in thread
From: Luiz Capitulino @ 2011-06-09 18:47 UTC (permalink / raw)
  To: aliguori; +Cc: qemu-devel, Stefan Hajnoczi, mdroth, Markus Armbruster


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.

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 :(

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

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] gtester questions/issues
  2011-06-09 18:47 [Qemu-devel] gtester questions/issues Luiz Capitulino
@ 2011-06-09 19:04 ` Anthony Liguori
  2011-06-09 20:02   ` Luiz Capitulino
  0 siblings, 1 reply; 9+ messages in thread
From: Anthony Liguori @ 2011-06-09 19:04 UTC (permalink / raw)
  To: Luiz Capitulino; +Cc: qemu-devel, Stefan Hajnoczi, mdroth, Markus Armbruster

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.

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

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

The check tests have bit rotted over time to the point that they're 
broken in the tree.  I attribute this to the fact that they aren't built 
by default.

Regards,

Anthony Liguori

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] gtester questions/issues
  2011-06-09 19:04 ` Anthony Liguori
@ 2011-06-09 20:02   ` Luiz Capitulino
  2011-06-09 23:04     ` Michael Roth
  0 siblings, 1 reply; 9+ messages in thread
From: Luiz Capitulino @ 2011-06-09 20:02 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel, Stefan Hajnoczi, mdroth, Markus Armbruster

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] gtester questions/issues
  2011-06-09 20:02   ` Luiz Capitulino
@ 2011-06-09 23:04     ` Michael Roth
  2011-06-09 23:07       ` Michael Roth
  2011-06-10 14:55       ` Luiz Capitulino
  0 siblings, 2 replies; 9+ messages in thread
From: Michael Roth @ 2011-06-09 23:04 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Stefan Hajnoczi, Anthony Liguori, Markus Armbruster, qemu-devel

On 06/09/2011 03:02 PM, Luiz Capitulino wrote:
> 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.
>

You kind of get the desired behavior if you run the test via something like:

gtester -k -o test.xml test-visiter

The gtester utility will log the return code after a test bombs, then 
restart and skip to the test following the one that bombed. And I'm sure 
gtester-report can process the resulting test.xml in manner similar to 
check... unfortunately it appears to be broken for me on Ubuntu 10.04 so 
here's the raw XML dump for reference:

<?xml version="1.0"?>
<gtester>
   <testbinary path="./test-visiter">
     <binary file="./test-visiter"/>
     <random-seed>R02S13c4d9e6d35c23e8dd988917863a66b1</random-seed>
     <testcase path="/0.15/visiter_core">
       <duration>0.000346</duration>
       <status exit-status="0" n-forks="0" result="success"/>
     </testcase>
     <testcase path="/0.15/epic_fail">
       <duration>0.000000</duration>
       <status exit-status="-256" n-forks="0" result="failed"/>
     </testcase>
     <duration>0.015056</duration>
   </testbinary>
   <testbinary path="./test-visiter">
     <binary file="./test-visiter"/>
     <random-seed>R02S7acda18e321c5a41ccaee4f524877343</random-seed>
     <testcase path="/0.15/visiter_core" skipped="1"/>
     <testcase path="/0.15/epic_fail" skipped="1"/>
     <testcase path="/0.15/epic_fail2">
 
<error>ERROR:/home/mdroth/w/qemu2.git/test-visiter.c:312:test_epic_fail2: assertion 
failed: (false)</error>
       <duration>0.000000</duration>
       <status exit-status="-256" n-forks="0" result="failed"/>
     </testcase>
     <duration>0.006355</duration>
   </testbinary>
   <testbinary path="./test-visiter">
     <binary file="./test-visiter"/>
     <random-seed>R02S73a208dd8f1b127c23b6a7883df9b78f</random-seed>
     <testcase path="/0.15/visiter_core" skipped="1"/>
     <testcase path="/0.15/epic_fail" skipped="1"/>
     <testcase path="/0.15/epic_fail2" skipped="1"/>
     <testcase path="/0.15/nested_structs">
       <duration>0.000318</duration>
       <status exit-status="0" n-forks="0" result="success"/>
     </testcase>
     <testcase path="/0.15/enums">
       <duration>0.000036</duration>
       <status exit-status="0" n-forks="0" result="success"/>
     </testcase>
     <testcase path="/0.15/nested_enums">
       <duration>0.000059</duration>
       <status exit-status="0" n-forks="0" result="success"/>
     </testcase>
     <duration>0.008079</duration>
   </testbinary>
</gtester>

XML or HTML...it's not pretty, but we can make use of it for automated 
tests. And for interactive use I don't think it's as much a problem 
since that'll for the most part be developers making sure they didn't 
break any tests before committing, or working on failures picked up by 
automated runs: not a big deal in those cases if the unit tests stop at 
the first abort.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] gtester questions/issues
  2011-06-09 23:04     ` Michael Roth
@ 2011-06-09 23:07       ` Michael Roth
  2011-06-10 14:55       ` Luiz Capitulino
  1 sibling, 0 replies; 9+ messages in thread
From: Michael Roth @ 2011-06-09 23:07 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Stefan Hajnoczi, Anthony Liguori, Markus Armbruster, qemu-devel

On 06/09/2011 06:04 PM, Michael Roth wrote:
> On 06/09/2011 03:02 PM, Luiz Capitulino wrote:
>> 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.
>>
>
> You kind of get the desired behavior if you run the test via something
> like:
>
> gtester -k -o test.xml test-visiter
>
> The gtester utility will log the return code after a test bombs, then
> restart and skip to the test following the one that bombed. And I'm sure
> gtester-report can process the resulting test.xml in manner similar to
> check... unfortunately it appears to be broken for me on Ubuntu 10.04 so
> here's the raw XML dump for reference:

(the epic_fail test segfaults, the epic_fail2 test fails an assert)

>
> <?xml version="1.0"?>
> <gtester>
> <testbinary path="./test-visiter">
> <binary file="./test-visiter"/>
> <random-seed>R02S13c4d9e6d35c23e8dd988917863a66b1</random-seed>
> <testcase path="/0.15/visiter_core">
> <duration>0.000346</duration>
> <status exit-status="0" n-forks="0" result="success"/>
> </testcase>
> <testcase path="/0.15/epic_fail">
> <duration>0.000000</duration>
> <status exit-status="-256" n-forks="0" result="failed"/>
> </testcase>
> <duration>0.015056</duration>
> </testbinary>
> <testbinary path="./test-visiter">
> <binary file="./test-visiter"/>
> <random-seed>R02S7acda18e321c5a41ccaee4f524877343</random-seed>
> <testcase path="/0.15/visiter_core" skipped="1"/>
> <testcase path="/0.15/epic_fail" skipped="1"/>
> <testcase path="/0.15/epic_fail2">
>
> <error>ERROR:/home/mdroth/w/qemu2.git/test-visiter.c:312:test_epic_fail2: assertion
> failed: (false)</error>
> <duration>0.000000</duration>
> <status exit-status="-256" n-forks="0" result="failed"/>
> </testcase>
> <duration>0.006355</duration>
> </testbinary>
> <testbinary path="./test-visiter">
> <binary file="./test-visiter"/>
> <random-seed>R02S73a208dd8f1b127c23b6a7883df9b78f</random-seed>
> <testcase path="/0.15/visiter_core" skipped="1"/>
> <testcase path="/0.15/epic_fail" skipped="1"/>
> <testcase path="/0.15/epic_fail2" skipped="1"/>
> <testcase path="/0.15/nested_structs">
> <duration>0.000318</duration>
> <status exit-status="0" n-forks="0" result="success"/>
> </testcase>
> <testcase path="/0.15/enums">
> <duration>0.000036</duration>
> <status exit-status="0" n-forks="0" result="success"/>
> </testcase>
> <testcase path="/0.15/nested_enums">
> <duration>0.000059</duration>
> <status exit-status="0" n-forks="0" result="success"/>
> </testcase>
> <duration>0.008079</duration>
> </testbinary>
> </gtester>
>
> XML or HTML...it's not pretty, but we can make use of it for automated
> tests. And for interactive use I don't think it's as much a problem
> since that'll for the most part be developers making sure they didn't
> break any tests before committing, or working on failures picked up by
> automated runs: not a big deal in those cases if the unit tests stop at
> the first abort.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] gtester questions/issues
  2011-06-09 23:04     ` Michael Roth
  2011-06-09 23:07       ` Michael Roth
@ 2011-06-10 14:55       ` Luiz Capitulino
  2011-06-10 15:05         ` Anthony Liguori
  2011-06-10 15:38         ` Michael Roth
  1 sibling, 2 replies; 9+ messages in thread
From: Luiz Capitulino @ 2011-06-10 14:55 UTC (permalink / raw)
  To: Michael Roth
  Cc: Stefan Hajnoczi, Anthony Liguori, Markus Armbruster, qemu-devel

On Thu, 09 Jun 2011 18:04:44 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> On 06/09/2011 03:02 PM, Luiz Capitulino wrote:
> > 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.
> >
> 
> You kind of get the desired behavior if you run the test via something like:
> 
> gtester -k -o test.xml test-visiter
> 
> The gtester utility will log the return code after a test bombs, then 
> restart and skip to the test following the one that bombed. And I'm sure 
> gtester-report can process the resulting test.xml in manner similar to 
> check...

Ok, that makes the problem less worse and I agree it's possible to cook
a workaround for it. But IMO, glib's test framework is flawed. You just
can't require developers to run two additional utilities and dump xml so
that they can know a particular test exploded.

The argument that qemu will be linked against glib is a valid one. But I
really think we're changing for the worse here, and this can compromise
all the plans on focusing on more unit-tests. What's the point in investing
time in writing and maintaining unit-tests if they can get as difficult
as the VM itself to be debugged?

> unfortunately it appears to be broken for me on Ubuntu 10.04 so 
> here's the raw XML dump for reference:

Yes, there's this one too and the memory leak.

> 
> <?xml version="1.0"?>
> <gtester>
>    <testbinary path="./test-visiter">
>      <binary file="./test-visiter"/>
>      <random-seed>R02S13c4d9e6d35c23e8dd988917863a66b1</random-seed>
>      <testcase path="/0.15/visiter_core">
>        <duration>0.000346</duration>
>        <status exit-status="0" n-forks="0" result="success"/>
>      </testcase>
>      <testcase path="/0.15/epic_fail">
>        <duration>0.000000</duration>
>        <status exit-status="-256" n-forks="0" result="failed"/>
>      </testcase>
>      <duration>0.015056</duration>
>    </testbinary>
>    <testbinary path="./test-visiter">
>      <binary file="./test-visiter"/>
>      <random-seed>R02S7acda18e321c5a41ccaee4f524877343</random-seed>
>      <testcase path="/0.15/visiter_core" skipped="1"/>
>      <testcase path="/0.15/epic_fail" skipped="1"/>
>      <testcase path="/0.15/epic_fail2">
>  
> <error>ERROR:/home/mdroth/w/qemu2.git/test-visiter.c:312:test_epic_fail2: assertion 
> failed: (false)</error>
>        <duration>0.000000</duration>
>        <status exit-status="-256" n-forks="0" result="failed"/>
>      </testcase>
>      <duration>0.006355</duration>
>    </testbinary>
>    <testbinary path="./test-visiter">
>      <binary file="./test-visiter"/>
>      <random-seed>R02S73a208dd8f1b127c23b6a7883df9b78f</random-seed>
>      <testcase path="/0.15/visiter_core" skipped="1"/>
>      <testcase path="/0.15/epic_fail" skipped="1"/>
>      <testcase path="/0.15/epic_fail2" skipped="1"/>
>      <testcase path="/0.15/nested_structs">
>        <duration>0.000318</duration>
>        <status exit-status="0" n-forks="0" result="success"/>
>      </testcase>
>      <testcase path="/0.15/enums">
>        <duration>0.000036</duration>
>        <status exit-status="0" n-forks="0" result="success"/>
>      </testcase>
>      <testcase path="/0.15/nested_enums">
>        <duration>0.000059</duration>
>        <status exit-status="0" n-forks="0" result="success"/>
>      </testcase>
>      <duration>0.008079</duration>
>    </testbinary>
> </gtester>
> 
> XML or HTML...it's not pretty, but we can make use of it for automated 
> tests. And for interactive use I don't think it's as much a problem 
> since that'll for the most part be developers making sure they didn't 
> break any tests before committing, or working on failures picked up by 
> automated runs: not a big deal in those cases if the unit tests stop at 
> the first abort.

I hope you're not saying we're going to live with an XML output. I don't even
consider having to read XML as test output. I'm under the assumption that
we'll get this fixed in glib.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] gtester questions/issues
  2011-06-10 14:55       ` Luiz Capitulino
@ 2011-06-10 15:05         ` Anthony Liguori
  2011-06-10 15:13           ` Luiz Capitulino
  2011-06-10 15:38         ` Michael Roth
  1 sibling, 1 reply; 9+ messages in thread
From: Anthony Liguori @ 2011-06-10 15:05 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: qemu-devel, Stefan Hajnoczi, Michael Roth, Markus Armbruster

On 06/10/2011 09:55 AM, Luiz Capitulino wrote:
> On Thu, 09 Jun 2011 18:04:44 -0500
>> You kind of get the desired behavior if you run the test via something like:
>>
>> gtester -k -o test.xml test-visiter
>>
>> The gtester utility will log the return code after a test bombs, then
>> restart and skip to the test following the one that bombed. And I'm sure
>> gtester-report can process the resulting test.xml in manner similar to
>> check...
>
> Ok, that makes the problem less worse and I agree it's possible to cook
> a workaround for it. But IMO, glib's test framework is flawed. You just
> can't require developers to run two additional utilities and dump xml so
> that they can know a particular test exploded.

It all happens automagically during make check.  I don't understand what 
the problem here is.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] gtester questions/issues
  2011-06-10 15:05         ` Anthony Liguori
@ 2011-06-10 15:13           ` Luiz Capitulino
  0 siblings, 0 replies; 9+ messages in thread
From: Luiz Capitulino @ 2011-06-10 15:13 UTC (permalink / raw)
  To: Anthony Liguori
  Cc: qemu-devel, Stefan Hajnoczi, Michael Roth, Markus Armbruster

On Fri, 10 Jun 2011 10:05:17 -0500
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 06/10/2011 09:55 AM, Luiz Capitulino wrote:
> > On Thu, 09 Jun 2011 18:04:44 -0500
> >> You kind of get the desired behavior if you run the test via something like:
> >>
> >> gtester -k -o test.xml test-visiter
> >>
> >> The gtester utility will log the return code after a test bombs, then
> >> restart and skip to the test following the one that bombed. And I'm sure
> >> gtester-report can process the resulting test.xml in manner similar to
> >> check...
> >
> > Ok, that makes the problem less worse and I agree it's possible to cook
> > a workaround for it. But IMO, glib's test framework is flawed. You just
> > can't require developers to run two additional utilities and dump xml so
> > that they can know a particular test exploded.
> 
> It all happens automagically during make check.  I don't understand what 
> the problem here is.

That's the "I agree it's possible to cook a workaround for it" part. But of
course that we have to fix gtester-report first. It doesn't work today and it
only knows how to dump HTML.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Qemu-devel] gtester questions/issues
  2011-06-10 14:55       ` Luiz Capitulino
  2011-06-10 15:05         ` Anthony Liguori
@ 2011-06-10 15:38         ` Michael Roth
  1 sibling, 0 replies; 9+ messages in thread
From: Michael Roth @ 2011-06-10 15:38 UTC (permalink / raw)
  To: Luiz Capitulino
  Cc: Stefan Hajnoczi, Anthony Liguori, Markus Armbruster, qemu-devel

On 06/10/2011 09:55 AM, Luiz Capitulino wrote:
> On Thu, 09 Jun 2011 18:04:44 -0500
> Michael Roth<mdroth@linux.vnet.ibm.com>  wrote:
>
>> On 06/09/2011 03:02 PM, Luiz Capitulino wrote:
>>> 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.
>>>
>>
>> You kind of get the desired behavior if you run the test via something like:
>>
>> gtester -k -o test.xml test-visiter
>>
>> The gtester utility will log the return code after a test bombs, then
>> restart and skip to the test following the one that bombed. And I'm sure
>> gtester-report can process the resulting test.xml in manner similar to
>> check...
>
> Ok, that makes the problem less worse and I agree it's possible to cook
> a workaround for it. But IMO, glib's test framework is flawed. You just
> can't require developers to run two additional utilities and dump xml so
> that they can know a particular test exploded.
>
> The argument that qemu will be linked against glib is a valid one. But I
> really think we're changing for the worse here, and this can compromise
> all the plans on focusing on more unit-tests. What's the point in investing
> time in writing and maintaining unit-tests if they can get as difficult
> as the VM itself to be debugged?
>

The unit tests would only be slightly more difficult to debug if used 
interactively. You'd still be able to tell what test failed, you just 
wouldn't see failure beyond that without some extra work. So long as we 
don't carry unit test failures for extended periods this shouldn't hurt 
anyone's workflow too much. I agree there appears to be a little bit of 
a trade-off, but being able to do a make check by default on every build 
is a big gain.

And for those automated tests the gtester utilities should make it easy 
to work around. The XML seems sane enough.

>> unfortunately it appears to be broken for me on Ubuntu 10.04 so
>> here's the raw XML dump for reference:
>
> Yes, there's this one too and the memory leak.
>
>>
>> <?xml version="1.0"?>
>> <gtester>
>>     <testbinary path="./test-visiter">
>>       <binary file="./test-visiter"/>
>>       <random-seed>R02S13c4d9e6d35c23e8dd988917863a66b1</random-seed>
>>       <testcase path="/0.15/visiter_core">
>>         <duration>0.000346</duration>
>>         <status exit-status="0" n-forks="0" result="success"/>
>>       </testcase>
>>       <testcase path="/0.15/epic_fail">
>>         <duration>0.000000</duration>
>>         <status exit-status="-256" n-forks="0" result="failed"/>
>>       </testcase>
>>       <duration>0.015056</duration>
>>     </testbinary>
>>     <testbinary path="./test-visiter">
>>       <binary file="./test-visiter"/>
>>       <random-seed>R02S7acda18e321c5a41ccaee4f524877343</random-seed>
>>       <testcase path="/0.15/visiter_core" skipped="1"/>
>>       <testcase path="/0.15/epic_fail" skipped="1"/>
>>       <testcase path="/0.15/epic_fail2">
>>
>> <error>ERROR:/home/mdroth/w/qemu2.git/test-visiter.c:312:test_epic_fail2: assertion
>> failed: (false)</error>
>>         <duration>0.000000</duration>
>>         <status exit-status="-256" n-forks="0" result="failed"/>
>>       </testcase>
>>       <duration>0.006355</duration>
>>     </testbinary>
>>     <testbinary path="./test-visiter">
>>       <binary file="./test-visiter"/>
>>       <random-seed>R02S73a208dd8f1b127c23b6a7883df9b78f</random-seed>
>>       <testcase path="/0.15/visiter_core" skipped="1"/>
>>       <testcase path="/0.15/epic_fail" skipped="1"/>
>>       <testcase path="/0.15/epic_fail2" skipped="1"/>
>>       <testcase path="/0.15/nested_structs">
>>         <duration>0.000318</duration>
>>         <status exit-status="0" n-forks="0" result="success"/>
>>       </testcase>
>>       <testcase path="/0.15/enums">
>>         <duration>0.000036</duration>
>>         <status exit-status="0" n-forks="0" result="success"/>
>>       </testcase>
>>       <testcase path="/0.15/nested_enums">
>>         <duration>0.000059</duration>
>>         <status exit-status="0" n-forks="0" result="success"/>
>>       </testcase>
>>       <duration>0.008079</duration>
>>     </testbinary>
>> </gtester>
>>
>> XML or HTML...it's not pretty, but we can make use of it for automated
>> tests. And for interactive use I don't think it's as much a problem
>> since that'll for the most part be developers making sure they didn't
>> break any tests before committing, or working on failures picked up by
>> automated runs: not a big deal in those cases if the unit tests stop at
>> the first abort.
>
> I hope you're not saying we're going to live with an XML output. I don't even
> consider having to read XML as test output. I'm under the assumption that
> we'll get this fixed in glib.

I just mean for automation. Worse case we ignore gtester-report and pipe 
the xml through a python script or something for reporting.

I'm assuming if you're running individual unit tests directly you're 
operating in "debugging" mode where you don't care too much about 
results beyond a segfault. But even then you could still fall back to 
the automated tests for a more holistic view of the failures.

gtester-report does need to be fixed though (maybe it already is?)...but 
unfortunately widespread distro breakage means we might not be able to 
rely on it for a while. I think a python reporting script to process the 
XML might be a better solution.

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-06-10 15:38 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-09 18:47 [Qemu-devel] gtester questions/issues Luiz Capitulino
2011-06-09 19:04 ` Anthony Liguori
2011-06-09 20:02   ` Luiz Capitulino
2011-06-09 23:04     ` Michael Roth
2011-06-09 23:07       ` Michael Roth
2011-06-10 14:55       ` Luiz Capitulino
2011-06-10 15:05         ` Anthony Liguori
2011-06-10 15:13           ` Luiz Capitulino
2011-06-10 15:38         ` Michael Roth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).