qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH 03/14] iotests: add qemu_img_info()


From: John Snow
Subject: Re: [PATCH 03/14] iotests: add qemu_img_info()
Date: Thu, 17 Mar 2022 10:51:31 -0400



On Thu, Mar 17, 2022, 7:09 AM Hanna Reitz <hreitz@redhat.com> wrote:
On 09.03.22 04:53, John Snow wrote:
> Add qemu_img_info() by analogy with qemu_img_measure() and
> qemu_img_check(). Modify image_size() to use this function instead to
> take advantage of the better diagnostic information on failure provided
> (ultimately) by qemu_img().
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>   tests/qemu-iotests/065        |  5 ++---
>   tests/qemu-iotests/242        |  5 ++---
>   tests/qemu-iotests/iotests.py | 15 +++++++++++----
>   3 files changed, 15 insertions(+), 10 deletions(-)

[...]

> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 7b37938d45..62f82281a9 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py

[...]

> @@ -570,10 +573,14 @@ def create_image(name, size):
>               file.write(sector)
>               i = i + 512
>   
> -def image_size(img):
> -    '''Return image's virtual size'''
> -    r = qemu_img_pipe('info', '--output=json', '-f', imgfmt, img)
> -    return json.loads(r)['virtual-size']
> +def image_size(img: str) -> int:
> +    """Return image's virtual size"""
> +    value = qemu_img_info('-f', imgfmt, img)['virtual-size']
> +    if not isinstance(value, int):
> +        type_name = type(value).__name__
> +        raise TypeError("Expected 'int' for 'virtual-size', "
> +                        f"got '{value}' of type '{type_name}'")
> +    return value

Bit overkill for my taste for the scope of iotests.py, but if you want
to go the extra mile, I’m not stopping you.

Reviewed-by: Hanna Reitz <hreitz@redhat.com>

As long as it isn't *too* overkill. Tests should be simple and stupid, but the helper utilities should be *solid*.

I added a type hint but needed to make it true. I could have used an assert, but asserts should not be used in library code for sanitizing input data.

(I've got "Trying to make a public PyPI package" brain and it's hard to switch off. Maybe an assert would've been fine, ...)

--js

reply via email to

[Prev in Thread] Current Thread [Next in Thread]