Re: [PATCH 4/4] iotests: make qemu_img raise on non-zero rc by default

[John Snow]

On Tue, Feb 15, 2022 at 6:09 PM Eric Blake <eblake@redhat.com> wrote:
>
> On Tue, Feb 15, 2022 at 05:08:53PM -0500, John Snow wrote:
> > re-configure qemu_img() into a function that will by default raise a
> > VerboseProcessException (extended from CalledProcessException) on
> > non-zero return codes. This will produce a stack trace that will show
> > the command line arguments and return code from the failed process run.
> >
> > Users that want something more flexible (There appears to be only one)
>
> s/There/there/
>
> > can use check=False and manage the return themselves.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  tests/qemu-iotests/257        |  8 ++++--
> >  tests/qemu-iotests/iotests.py | 53 +++++++++++++++++++++++++++++++----
> >  2 files changed, 53 insertions(+), 8 deletions(-)
> >
>
> > +def qemu_img(*args: str, check: bool = True, combine_stdio: bool = True
> > +             ) -> subprocess.CompletedProcess[str]:
> > +    """
> > +    Run qemu_img, returning a CompletedProcess instance.
> > +
> > +    The CompletedProcess object has args, returncode, and output 
> > properties.
> > +    If streams are not combined, It will also have stdout/stderr 
> > properties.
>
> s/It/it/
>
> > +
> > +    :param args: command-line arguments to qemu_img.
> > +    :param check: set to False to suppress VerboseProcessError.
> > +    :param combine_stdio: set to False to keep stdout/stderr separated.
> > +
> > +    :raise VerboseProcessError:
> > +        On non-zero exit code, when 'check=True' was provided. This
> > +        exception has 'stderr', 'stdout' and 'returncode' properties
> > +        that may be inspected to show greater detail. If this exception
> > +        is not handled, The command-line, return code, and all console
>
> s/The/the/
>
> > +        output will be included at the bottom of the stack trace.
> > +    """
>
> > @@ -469,8 +511,9 @@ def qemu_nbd_popen(*args):
> >
> >  def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
> >      '''Return True if two image files are identical'''
> > -    return qemu_img('compare', '-f', fmt1,
> > -                    '-F', fmt2, img1, img2) == 0
> > +    res = qemu_img('compare', '-f', fmt1,
> > +                   '-F', fmt2, img1, img2, check=False)
> > +    return res.returncode == 0
>
> Not sure why there was so much Mid-sentence capitalization ;)

Mostly the result of editing a few different drafts together and
failing to fix the casing. Oopsie.

>
> Seems useful, although it is at the limits of my python review skills,
> so this is a weak:
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>

Understood, thanks!

--js