Re: [Qemu-devel] gtester questions/issues

[Michael Roth <mdroth@linux.vnet.ibm.com>]

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.