[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 4/4] tests: Add VIR_TEST_REGENERATE_OUTPUT



On 04/23/2015 03:46 PM, Laine Stump wrote:
> On 04/23/2015 02:20 PM, Cole Robinson wrote:
>> If this enviroment variable is set, the virTestCompareToFile helper
>> will overwrite the file content we are comparing against, if the
>> file doesn't exist or it doesn't match the expected input.
>>
>> This is useful when adding new test cases, or making changes that
>> generate a lot of output churn.
>> ---
>>  HACKING              |  7 +++++++
>>  docs/hacking.html.in | 12 ++++++++++++
>>  tests/testutils.c    | 45 +++++++++++++++++++++++++++------------------
>>  3 files changed, 46 insertions(+), 18 deletions(-)
>>
>> diff --git a/HACKING b/HACKING
>> index 94d9d2c..cfc507a 100644
>> --- a/HACKING
>> +++ b/HACKING
>> @@ -136,6 +136,13 @@ Also, individual tests can be run from inside the "tests/" directory, like:
>>  
>>    ./qemuxml2xmltest
>>  
>> +If you are adding new test cases, or making changes that alter existing test
>> +output, you can use the environment variable VIR_TEST_REGENERATE_OUTPUT to
>> +quickly update the saved test data. Of course you still need to review the
>> +changes to ensure they are correct.
> 
> 
> Maybe say "review the changes *VERY CAREFULLY*" or something like that.
> This is incredibly convenient, but could make it easier for someone to
> gloss over a regression that happens to be introduced with the same
> patch where they are adding some new functionality.
> 
> 
>> [...]
>>  
>> -    if (STRNEQ(fixedcontent ? fixedcontent : strcontent, filecontent)) {
>> +    if (STRNEQ_NULLABLE(fixedcontent ? fixedcontent : strcontent,
>> +                        filecontent)) {
>> +        if (regenerate) {
>> +            if (virFileWriteStr(filename, strcontent, 0666) < 0)
>> +                goto failure;
>> +            goto out;
>> +        }
>>          virtTestDifference(stderr, strcontent, filecontent);
>>          goto failure;
> 
> Okay, so you still report a failure for this test, but in the process
> you update the test file. Makes sense - that way you can see which tests
> are getting updated.
>

Actually no failure is reported, see goto out; vs goto failure;

However git diff/git status still makes it very obvious what files have been
added or changed, so I think that's okay.


> This is *really* cool! ACK++
> 

Thanks for the review. I pushed the the docs suggestion, and the cppi fix for
patch #1

- Cole


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]