Re: [PATCH 04/19] iotests.py: Add qemu_img_pipe_and_status()

[Maxim Levitsky <mlevitsk@redhat.com>]

On Thu, 2020-06-25 at 14:55 +0200, Max Reitz wrote:
> This function will be used by the next patch, which intends to check
> both the exit code and qemu-img's output.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/iotests.py | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 5ea4c4df8b..eee94e18cc 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -29,7 +29,7 @@ import struct
>  import subprocess
>  import sys
>  from typing import (Any, Callable, Dict, Iterable,
> -                    List, Optional, Sequence, TypeVar)
> +                    List, Optional, Sequence, Tuple, TypeVar)
>  import unittest
>  
>  # pylint: disable=import-error, wrong-import-position
> @@ -90,15 +90,23 @@ luks_default_secret_object = 'secret,id=keysec0,data=' + \
>  luks_default_key_secret_opt = 'key-secret=keysec0'
>  
>  
> -def qemu_img(*args):
> -    '''Run qemu-img and return the exit code'''
> -    devnull = open('/dev/null', 'r+')
> -    exitcode = subprocess.call(qemu_img_args + list(args),
> -                               stdin=devnull, stdout=devnull)
> +def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
> +    """
> +    Run qemu-img and return both its output and its exit code
> +    """
> +    subp = subprocess.Popen(qemu_img_args + list(args),
> +                            stdout=subprocess.PIPE,
> +                            stderr=subprocess.STDOUT,
> +                            universal_newlines=True)
> +    exitcode = subp.wait()
>      if exitcode < 0:
>          sys.stderr.write('qemu-img received signal %i: %s\n'
>                           % (-exitcode, ' '.join(qemu_img_args + list(args))))
> -    return exitcode
> +    return (subp.communicate()[0], exitcode)
> +
> +def qemu_img(*args: str) -> int:
> +    '''Run qemu-img and return the exit code'''
> +    return qemu_img_pipe_and_status(*args)[1]
>  
>  def ordered_qmp(qmsg, conv_keys=True):
>      # Dictionaries are not ordered prior to 3.6, therefore:
> @@ -140,17 +148,9 @@ def qemu_img_verbose(*args):
>                           % (-exitcode, ' '.join(qemu_img_args + list(args))))
>      return exitcode
>  
> -def qemu_img_pipe(*args):
> +def qemu_img_pipe(*args: str) -> str:
>      '''Run qemu-img and return its output'''
> -    subp = subprocess.Popen(qemu_img_args + list(args),
> -                            stdout=subprocess.PIPE,
> -                            stderr=subprocess.STDOUT,
> -                            universal_newlines=True)
> -    exitcode = subp.wait()
> -    if exitcode < 0:
> -        sys.stderr.write('qemu-img received signal %i: %s\n'
> -                         % (-exitcode, ' '.join(qemu_img_args + list(args))))
> -    return subp.communicate()[0]
> +    return qemu_img_pipe_and_status(*args)[0]
>  
>  def qemu_img_log(*args):
>      result = qemu_img_pipe(*args)

You made me learn a bit about python type hints, and I don't regret it :-)
Looks OK.

Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Best regards,
	Maxim Levitsky