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

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



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 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)) {

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


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