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

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

 



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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux