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

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

 



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@xxxxxxxxxx>
> ---
>  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;
>      }

Attachment: signature.asc
Description: Digital signature

--
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]