[libvirt] [PATCH] tests: Fix detection of expected errors

[Matthias Bolte]

2010/11/29 Jiri Denemark <jdenemar at redhat.com>:
> ---
>  tests/nwfilterxml2xmltest.c |   15 +++++----------
>  tests/qemuxml2argvtest.c    |    8 +++-----
>  2 files changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/tests/nwfilterxml2xmltest.c b/tests/nwfilterxml2xmltest.c
> index 070aa98..9cad913 100644
> --- a/tests/nwfilterxml2xmltest.c
> +++ b/tests/nwfilterxml2xmltest.c
> @@ -25,7 +25,7 @@ static char *abs_srcdir;
>
>  static int testCompareXMLToXMLFiles(const char *inxml,
>                                     const char *outxml,
> -                                    bool expect_warning) {
> +                                    bool expect_error) {
>     char inXmlData[MAX_FILE];
>     char *inXmlPtr = &(inXmlData[0]);
>     char outXmlData[MAX_FILE];
> @@ -33,26 +33,21 @@ static int testCompareXMLToXMLFiles(const char *inxml,
>     char *actual = NULL;
>     int ret = -1;
>     virNWFilterDefPtr dev = NULL;
> -    char *log;
>
>     if (virtTestLoadFile(inxml, &inXmlPtr, MAX_FILE) < 0)
>         goto fail;
>     if (virtTestLoadFile(outxml, &outXmlPtr, MAX_FILE) < 0)
>         goto fail;
>
> -    if (!(dev = virNWFilterDefParseString(NULL, inXmlData)))
> -        goto fail;
> +    virResetLastError();
>
> -    if ((log = virtTestLogContentAndReset()) == NULL)
> +    if (!(dev = virNWFilterDefParseString(NULL, inXmlData)))
>         goto fail;
>
> -    if ((*log != '\0') != expect_warning) {
> -        free(log);
> +    if (!!virGetLastError() != expect_error)
>         goto fail;
> -    }
> -    free(log);
>
> -    if (expect_warning) {
> +    if (expect_error) {
>         /* need to suppress the errors */
>         virResetLastError();
>     }
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index b149ef4..c5e0448 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -84,6 +84,7 @@ static int testCompareXMLToArgvFiles(const char *xml,
>
>
>     free(virtTestLogContentAndReset());
> +    virResetLastError();
>
>     if (qemudBuildCommandLine(conn, &driver,
>                               vmdef, &monitor_chr, 0, flags,
> @@ -91,11 +92,8 @@ static int testCompareXMLToArgvFiles(const char *xml,
>                               NULL, NULL, migrateFrom, NULL) < 0)
>         goto fail;
>
> -    if ((log = virtTestLogContentAndReset()) == NULL)
> -        goto fail;
> -
> -    if (!!strstr(log, ": error :") != expectError) {
> -        if (virTestGetDebug())
> +    if (!!virGetLastError() != expectError) {
> +        if (virTestGetDebug() && (log = virtTestLogContentAndReset()))
>             fprintf(stderr, "\n%s", log);
>         goto fail;
>     }
> --
> 1.7.3.2
>

ACK to the patch as is. It fixes the problem, but I wonder why
qemudBuildCommandLine returns 0 and reports and error. Actually it
doesn't report an error, because then it should return non-zero. What
it does in the 3 cases where the test expects and "error" it reports
actually a warning. For example this one:

qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                _("the QEMU binary %s does not support kvm"),
                emulator);

This is currently reported at error level (VIR_ERR_ERROR) but it
should actually be reported at warning level (VIR_ERR_WARNING),
because it's treated as non-fatal.

The nwfilter test expects errors from this line in virNWFilterRuleDetailsParse:

virNWFilterReportError(VIR_ERR_INTERNAL_ERROR,
                       _("%s has illegal value %s"),
                       att[idx].name, prop);

The only caller of virNWFilterRuleDetailsParse is
virNWFilterRuleParse, but it decided to ignore malformed rules:

if (virNWFilterRuleDetailsParse(cur,
                                ret,
                                virAttr[i].att) < 0) {
    /* we ignore malformed rules
       goto err_exit;
    */
}

I wonder if in this case virNWFilterRuleParse should call
virRestLastError, because it decided to ignore malformed rules. In
that case the nwfilter test would not expect an error, because there
is actually no error to expect.

Matthias.