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

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

 



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


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





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

  Powered by Linux