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

John Snow posted 14 patches 3 years, 11 months ago
[PATCH 03/14] iotests: add qemu_img_info()
Posted by John Snow 3 years, 11 months ago
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/065 b/tests/qemu-iotests/065
index f7c1b68dad..9466ce7df4 100755
--- a/tests/qemu-iotests/065
+++ b/tests/qemu-iotests/065
@@ -24,7 +24,7 @@ import os
 import re
 import json
 import iotests
-from iotests import qemu_img, qemu_img_pipe
+from iotests import qemu_img, qemu_img_info, qemu_img_pipe
 import unittest
 
 test_img = os.path.join(iotests.test_dir, 'test.img')
@@ -49,8 +49,7 @@ class TestQemuImgInfo(TestImageInfoSpecific):
     human_compare = None
 
     def test_json(self):
-        data = json.loads(qemu_img_pipe('info', '--output=json', test_img))
-        data = data['format-specific']
+        data = qemu_img_info(test_img)['format-specific']
         self.assertEqual(data['type'], iotests.imgfmt)
         self.assertEqual(data['data'], self.json_compare)
 
diff --git a/tests/qemu-iotests/242 b/tests/qemu-iotests/242
index 96a30152b0..547bf382e3 100755
--- a/tests/qemu-iotests/242
+++ b/tests/qemu-iotests/242
@@ -22,7 +22,7 @@
 import iotests
 import json
 import struct
-from iotests import qemu_img_create, qemu_io, qemu_img_pipe, \
+from iotests import qemu_img_create, qemu_io, qemu_img_info, \
     file_path, img_info_log, log, filter_qemu_io
 
 iotests.script_initialize(supported_fmts=['qcow2'],
@@ -39,8 +39,7 @@ flag_offset = 0x5000f
 def print_bitmap(extra_args):
     log('qemu-img info dump:\n')
     img_info_log(disk, extra_args=extra_args)
-    result = json.loads(qemu_img_pipe('info', '--force-share',
-                                      '--output=json', disk))
+    result = qemu_img_info('--force-share', disk)
     if 'bitmaps' in result['format-specific']['data']:
         bitmaps = result['format-specific']['data']['bitmaps']
         log('The same bitmaps in JSON format:')
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
@@ -320,6 +320,9 @@ def qemu_img_measure(*args: str) -> Any:
 def qemu_img_check(*args: str) -> Any:
     return qemu_img_json("check", "--output", "json", *args)
 
+def qemu_img_info(*args: str) -> Any:
+    return qemu_img_json('info', "--output", "json", *args)
+
 def qemu_img_pipe(*args: str) -> str:
     '''Run qemu-img and return its output'''
     return qemu_img_pipe_and_status(*args)[0]
@@ -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
 
 def is_str(val):
     return isinstance(val, str)
-- 
2.34.1
Re: [PATCH 03/14] iotests: add qemu_img_info()
Posted by Hanna Reitz 3 years, 10 months ago
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>


Re: [PATCH 03/14] iotests: add qemu_img_info()
Posted by John Snow 3 years, 10 months ago
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

>