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

Re: [libvirt] [PATCH] tests: Add DO_TEST_PARSE_ERROR() to qemuxml2xml

On Fri, Jul 07, 2017 at 13:00:10 +0200, Andrea Bolognani wrote:
> On Fri, 2017-07-07 at 12:48 +0200, Peter Krempa wrote:
> > > qemuxml2argv already supports the ability to include test
> > > cases that are known not to make it past
> XML parsing, and
> > > since we want to keep qemuxml2xml in sync with it as much
> > > as possible, we need to implement this missing feature.
> > 
> > I must say that I
> don't really see the point to have them fail twice ...
> > 
> > I'd understand if this would be added to the genericxml2xmltest (if it's
> > not there)
> We have a lot of test cases with xml2argv coverage but no
> xml2xml coverage, and vice versa. The obvious solution to

Well. If the file fails to parse, I don't see why it would make sense to
make it fail to parse twice.

The both tests in their success paths have value. Failing to parse the
same document twice does not.

> that is to, in due time, make it so you only need to list
> input file and capabilities once to have them tested both
> for xml2argv and xml2xml, and this is a step towards that
> still pretty far away goal.

This is certainly a good worthwhile goal. I just don't really see how
splitting the testing of unparsable documents into two places or
duplicating it will help your effort to simplify it.

The combined test will have to be renamed anyways and the xml2argv test
has the capability flags, so that seems as a better point to start the

> > > @@ -217,6 +231,17 @@ testCompareStatusXMLToXMLFiles(const void *opaque)
> > >  
> > >      ret = 0;
> > >  
> > > + ok:
> > 
> > This is not an okay name for a label.
> It's clearly an "ok" name though :P
> I borrowed it from testCompareXMLToArgv() in xml2argv,
> but I don't have a problem changing it to something you
> think is more suitable.

I'd prefer 'done'. 'ok' seems too short. Also the test still may fail at
that point, so 'done' should not imply success (hopefully).

Attachment: signature.asc
Description: PGP signature

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