[libvirt] VIR_TEST_REGENERATE_OUTPUT is broken (Was: [PATCH] qemuxml2argvtest: Adapt to ethernet automatic tap creation)

[Cole Robinson]

On 04/12/2016 05:48 PM, Laine Stump wrote:
> On 03/23/2016 11:35 AM, Michal Privoznik wrote:
>> After 9c17d665fdc5 the tap device for ethernet network type is
>> automatically precreated before spawning qemu. Problem is, the
>> qemuxml2argvtest wasn't updated and thus is failing. Because of
>> all the APIs that new code is calling, I had to mock a lot. Also,
>> since the tap FDs are labeled separately from the rest of the
>> devices/files I had to enable NOP security driver for the test
>> too.
>>
>> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>> ---
> 
>> --- a/tests/qemuxml2argvmock.c
>> +++ b/tests/qemuxml2argvmock.c
> 
>> +}
>> +
>> +int
>> +virCommandRun(virCommandPtr cmd ATTRIBUTE_UNUSED,
>> +              int *exitstatus)
>> +{
>> +    if (exitstatus)
>> +        *exitstatus = 0;
>> +
>> +    return 0;
>> +}
> 
> 
> The problem with mocking virCommandRun is that it is called by the test
> infrastructure (virTestRewrapFile(), which is used when regenerating the test
> results (VIR_TEST_REGENERATE_OUTPUT=1)).
> 
> After this commit that function silently fails, which results in
> virFileWriteStr() calling strlen(NULL) and a crash of the test. (Nobody
> noticed this before because it's only called if you set
> VIR_TEST_REGENERATE_OUTPUT *and* the results of one of the qemuxml2argv tests
> has changed).
> 
> So what's the most reasonable way to deal with this? I suppose we could rename
> virCommandRun to e.g. virCommandRunInternal() which would be called by a new
> virCommandRun(), then have virTestRewrapFile() call virCommandRunInternal() so
> that it wouldn't get the mocked version. That seems ugly, inefficient, and
> hackish, but I can't think of any way that isn't ugly, inefficient, and
> hackish...
> 

I can't really decide if mocking virCommand is good or not. On one hand it'll
catch any virCommand calls in driver code that may leak into the test suite
which mean host environment dependencies, on the other hand losing the ability
to use virCommand in the test suite might be a problem some day.

Either way we can work around this issue easily enough by just using system()
to call test-wrap-argv.pl, I've sent a patch

- Cole