[libvirt] [PATCH 05/10] tests: use virtTestDifferenceFull in tests where we have output file

Peter Krempa pkrempa at redhat.com
Fri Jan 8 13:21:02 UTC 2016


On Thu, Jan 07, 2016 at 12:32:37 +0100, Pavel Hrdina wrote:
> This will enable regenerate functionality for those tests to make
> developer lives easier while updating tests.
> 
> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> ---
>  tests/domainsnapshotxml2xmltest.c |  2 +-
>  tests/interfacexml2xmltest.c      |  2 +-
>  tests/lxcconf2xmltest.c           |  2 +-
>  tests/nodedevxml2xmltest.c        |  2 +-
>  tests/qemuargv2xmltest.c          |  7 ++++---
>  tests/qemuhotplugtest.c           | 11 ++++++++---
>  tests/vboxsnapshotxmltest.c       |  2 +-
>  7 files changed, 17 insertions(+), 11 deletions(-)
> 

[...]

> diff --git a/tests/lxcconf2xmltest.c b/tests/lxcconf2xmltest.c
> index ed21e8a..fd5bc03 100644
> --- a/tests/lxcconf2xmltest.c
> +++ b/tests/lxcconf2xmltest.c
> @@ -51,7 +51,7 @@ testCompareXMLToConfigFiles(const char *xml,
>              goto fail;

In the context above blankProblemElements() is called, and thus the
output XML will not have the <uuid> element even if it had one before,
but only in case when something else faied. This might be confusing,
since if you use it you'll get a spurious hunk.

On the other hand, I don't think the filter would do anything since
there isn't any difference in the next patch that is regnerating it.
The filter then can be dropped possibly.

>  
>          if (STRNEQ(expectxml, actualxml)) {
> -            virtTestDifference(stderr, expectxml, actualxml);
> +            virtTestDifferenceFull(stderr, expectxml, xml, actualxml, NULL);
>              goto fail;
>          }
>      }

[...]

> diff --git a/tests/qemuargv2xmltest.c b/tests/qemuargv2xmltest.c
> index 7759a09..fc18b55 100644
> --- a/tests/qemuargv2xmltest.c
> +++ b/tests/qemuargv2xmltest.c
> @@ -90,12 +90,13 @@ static int testCompareXMLToArgvFiles(const char *xml,
>      if (!(actualxml = virDomainDefFormat(vmdef, 0)))
>          goto fail;
>  
> -    if (blankProblemElements(expectxml) < 0 ||
> -        blankProblemElements(actualxml) < 0)
> +    if (!virTestGetRegenerate() &&
> +        (blankProblemElements(expectxml) < 0 ||
> +         blankProblemElements(actualxml) < 0))
>          goto fail;

Here you are using a different approach. Also the qemu argv2xml blanking
function has much more fields.

>  
>      if (STRNEQ(expectxml, actualxml)) {
> -        virtTestDifference(stderr, expectxml, actualxml);
> +        virtTestDifferenceFull(stderr, expectxml, xml, actualxml, NULL);
>          goto fail;
>      }
>

I'm specially worried about this one since the argv parser is very old
and obsolete. I'd rather prefer if the regexes that postprocess the
output were tweaked or othe code mocked so that it gives deterministic
results at this point before this code is used.

I'd probably rather see the NoRegenerate version for this case though.

[...]

> diff --git a/tests/vboxsnapshotxmltest.c b/tests/vboxsnapshotxmltest.c
> index 4ee7537..87b8571 100644
> --- a/tests/vboxsnapshotxmltest.c
> +++ b/tests/vboxsnapshotxmltest.c
> @@ -85,7 +85,7 @@ testCompareXMLtoXMLFiles(const char *xml)
>          goto cleanup;
>  
>      if (STRNEQ(actual, xmlData)) {
> -        virtTestDifference(stderr, xmlData, actual);
> +        virtTestDifferenceFull(stderr, xmlData, xml, actual, NULL);

Again, this test calls testFilterXML which modifies the XML. Not sure
whether we want to touch this place either.

>          goto cleanup;
>      }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160108/c20eb53a/attachment-0001.sig>


More information about the libvir-list mailing list