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

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

 



2010/11/29 Jiri Denemark <jdenemar@xxxxxxxxxx>:
> ---
> Â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.

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]