[libvirt PATCH 6/7] virshtest: use virCommand instead of custom impl

Daniel P. Berrangé berrange at redhat.com
Tue Feb 11 13:05:19 UTC 2020


On Sun, Feb 09, 2020 at 02:32:36AM +0100, Ján Tomko wrote:
> Our virCommand helper API already has the ability to capture
> program output, there's no need to open-code it.
> 
> Apart from simplifying the code, the test is marginally faster
> due to recent improvements in virCommandMassClose.
> 
> Signed-off-by: Ján Tomko <jtomko at redhat.com>
> ---
>  tests/virshtest.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/virshtest.c b/tests/virshtest.c
> index 83675710ea..add33215b7 100644
> --- a/tests/virshtest.c
> +++ b/tests/virshtest.c
> @@ -5,6 +5,7 @@
>  #include "internal.h"
>  #include "virxml.h"
>  #include "testutils.h"
> +#include "vircommand.h"
>  #include "virstring.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_NONE
> @@ -61,10 +62,26 @@ testCompareOutputLit(const char *expectData,
>                       const char *filter, const char *const argv[])
>  {
>      g_autofree char *actualData = NULL;
> +    const char *empty = "";
> +    g_autoptr(virCommand) cmd = NULL;
> +    g_autofree char *errbuf = NULL;
>  
> -    if (virTestCaptureProgramOutput(argv, &actualData, 4096) < 0)
> +    if (!(cmd = virCommandNewArgs(argv)))
>          return -1;
>  
> +    virCommandAddEnvString(cmd, "LANG=C");
> +    virCommandSetInputBuffer(cmd, empty);
> +    virCommandSetOutputBuffer(cmd, &actualData);
> +    virCommandSetErrorBuffer(cmd, &errbuf);
> +
> +    if (virCommandRun(cmd, NULL) < 0)
> +        return -1;
> +
> +    if (STRNEQ(errbuf, "")) {
> +        fprintf(stderr, "Command reported error: %s", errbuf);
> +        return -1;
> +    }

The current method merges stdout and stderr into the same 'actualData'
buffer, so this impl is not functionally the same.

That said, assuming any commands we run don't write to stderr, I think
this impl is better.

Can you just mention that this is an intended change in the commit
message to remind us, in case we have a surprise later.

Reviewed-by: Daniel P. Berrangé <berrange at redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




More information about the libvir-list mailing list