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

Re: [libvirt] [PATCH] tests: add further XML namespace test



On Tue, 2017-07-11 at 14:00 +0100, Daniel P. Berrange wrote:
> > You can get rid of <disk> and <controller> too, then add
> > <controller type='usb' model='none'/> and
> > <memballoon model='none'/> to avoid unnecessary devices
> > popping up in the output file.
> 
> FYI, this XML file is identical to the pre-existing qemuxml2argv-qemu-ns.xml
> file, so I'm inclined to leave it as is

I'm aware of that, that's why I said that my R-b would stand
whether or not you decide to address these points.

That said, I'd really prefer it if we started being more
to the point with our test data and really only include the
relevant information rather than copy the same pointless
bits over and over again.

Here's a counter-offer: if you agree to follow my advice
above and use a minimal input file, I'll post a patch that
strips the other qemu-ns*.xml from unnecessary cruft. Does
that sound fair? :)

> > > +++ b/tests/qemuxml2argvtest.c
> > > @@ -1505,6 +1505,7 @@ mymain(void)
> > >   
> > >       DO_TEST("qemu-ns", NONE);
> > >       DO_TEST("qemu-ns-no-env", NONE);
> > > +    DO_TEST("qemu-ns-alt", NONE);
> > 
> > You'll want to add this to qemuxml2xmltest too.
> 
> This wont round trip - it'll end up outputing the canonical xmlns syntax
> instead, for which we already have output tests.

Hence proving that the alternative syntax and the canonical
one result in the same output file. Why wouldn't we want to
test that?

-- 
Andrea Bolognani / Red Hat / Virtualization


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