[libvirt] [PATCH 1/3] virjsontest: introduce DO_TEST_PARSE_FILE

[Ján Tomko]

On Wed, Feb 13, 2019 at 02:18:32PM +0100, Andrea Bolognani wrote:
>On Tue, 2019-02-12 at 16:57 +0100, Ján Tomko wrote:
>[...]
>> +{"return": [{"filename": \
>> +"unix:/home/berrange/.libvirt/qemu/lib/tck.monitor,server",\
>> +"label": "charmonitor"}, {"filename": "pty:/dev/pts/158",\
>> +"label": "charserial0"}], "id": "libvirt-3"}
>
>Are the backslashes at the end of lines necessary?

In this patch? Yes. The aim is to preserve the test coverage done before
and after.

>I've tried
>removing a bunch of them and the test didn't break. Are the files
>even valid JSON with the backslashes included?

No, they get stripped by virTestLoadFile

>
>Additional question: can't we pretty-print at least the input
>files now? Unless of course the point of these specific test cases
>is to prove we can successfully parse certain unusual constructs.
>

The whole point of separating these was to allow easier changes
and make it more-readable, so introducing more whitespace by removing
the backslashes and prettifying it is possible.

>[...]
>> +    if (!injson) {
>> +        if (info->pass) {
>> +            VIR_TEST_VERBOSE("Fail to parse %s\n", info->name);
>> +            return -1;
>> +        } else {
>> +            VIR_TEST_DEBUG("Fail to parse %s\n", info->name);
>> +            return 0;
>> +        }
>> +    }
>
>The second message should read something like
>
>  Expected failure while parsing %s
>
>or
>
>  Failed to parse %s (expected)
>
>> +
>> +    if (!info->pass) {
>> +        VIR_TEST_VERBOSE("Should not have parsed %s\n", info->name);
>> +        return -1;
>> +    }
>
>Maybe
>
>  Expected failure while parsing %s, got success instead
>
>or something along those lines.
>

All the error messages match the original test. Guess it would make more
sense to alter them before copying them.

>I think it would also look more legible if this entire if block was
>inside the else branch of the previous if block, but if you feel
>strongly about this version then just leave it as is.
>

Like this?

if (!injson) {
  if (info->pass) {
     return 0;
  } else {
     return -1;
  }
} else {
  if (!info->pass)
     return -1;
}

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190213/025b7bcf/attachment-0001.sig>