[libvirt] [PATCH 03/12] tests: cleanup qemuxml2argvtest

[Pavel Hrdina]

On Tue, Mar 22, 2016 at 10:28:35AM +0100, Jiri Denemark wrote:
> On Tue, Mar 15, 2016 at 14:15:59 +0100, Pavel Hrdina wrote:
> > This removes the testFailed magic and makes the code more readable.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> >  tests/qemuxml2argvtest.c | 62 ++++++++++++++++++++++--------------------------
> >  1 file changed, 28 insertions(+), 34 deletions(-)
> > 
> > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> > index e15da37..150a50d 100644
> > --- a/tests/qemuxml2argvtest.c
> > +++ b/tests/qemuxml2argvtest.c
> ...
> > @@ -391,15 +372,28 @@ static int testCompareXMLToArgvFiles(const char *xml,
> >      if (virtTestCompareToFile(actualargv, cmdline) < 0)
> >          goto out;
> >  
> > +    ret = 0;
> > +
> >   ok:
> > -    if (!virtTestOOMActive() &&
> > -        (flags & FLAG_EXPECT_ERROR)) {
> > -        /* need to suppress the errors */
> > +    if (ret == 0 &&
> > +        ((flags & FLAG_EXPECT_ERROR) |
> > +         (flags & FLAG_EXPECT_FAILURE))) {
> 
> Are you trying to optimize the code here or is it just a typo (| vs ||)?
> :-) In any case
> 
>        if (ret == 0 &&
>            (flags & FLAG_EXPECT_ERROR ||
>             flags & FLAG_EXPECT_FAILURE)) {

Yes :) it's a typo and I'll use this version.

> 
> or (if you prefer bit operations, which is uglier IMHO)
> 
>        if (ret == 0 &&
>            flags & (FLAG_EXPECT_ERROR | FLAG_EXPECT_FAILURE)) {
> 
> would look a bit better.
> 
> > +        ret = -1;
> > +        VIR_TEST_DEBUG("Error expected but there wasn't any.\n");
> > +        goto out;
> > +    }
> > +    if (!virtTestOOMActive()) {
> > +        if (flags & FLAG_EXPECT_ERROR) {
> > +            if ((log = virtTestLogContentAndReset()))
> > +                VIR_TEST_DEBUG("Got expected error: \n%s", log);
> > +        } else if (flags & FLAG_EXPECT_FAILURE) {
> > +            VIR_TEST_DEBUG("Got expected failure: %s\n",
> > +                    virGetLastErrorMessage());
> 
> Indentation is off a bit.
> 
> > +        }
> >          virResetLastError();
> > +        ret = 0;
> >      }
> >  
> > -    ret = 0;
> > -
> >   out:
> >      VIR_FREE(log);
> >      VIR_FREE(actualargv);
> 
> ACK once the nits are fixed.
> 
> Jirka

Fixed. Thanks,

Pavel