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 rename. > > > @@ -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
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
