Re: [libvirt] [PATCH 1/3] virjsontest: introduce DO_TEST_PARSE_FILE
On Wed, 2019-02-13 at 15:07 +0100, Ján Tomko wrote:
On Wed, Feb 13, 2019 at 02:18:32PM +0100, Andrea Bolognani wrote:
> 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
Oh, I see, they're there to keep the input JSON as a single line
as it was originally. Makes sense.
> 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.
Alright, it can be done in separate commits then.
[...]
All the error messages match the original test. Guess it would make
more
sense to alter them before copying them.
Yeah, that way they get improved for FromString tests as well.
> 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;
}
That's exactly what I had in mind.
--
Andrea Bolognani / Red Hat / Virtualization