Re: [Qemu-block] [PATCH v2 5/9] iotests: Different iterator behavior in Python 3

[Eduardo Habkost]

On Fri, Oct 19, 2018 at 09:15:19PM +0200, Max Reitz wrote:
> In Python 3, several functions now return iterators instead of lists.
> This includes range(), items(), map(), and filter().  This means that if
> we really want a list, we have to wrap those instances with list().  But
> then again, the two instances where this is the case for map() and
> filter(), there are shorter expressions which work without either
> function.
> 
> On the other hand, sometimes we do just want an iterator, in which case
> we have sometimes used xrange() and iteritems() which no longer exist in
> Python 3.  Just change these calls to be range() and items(), works in
> both Python 2 and 3, and is really what we want in 3 (which is what
> matters).  But because it is so simple to do (and to find and remove
> once we completely switch to Python 3), make range() be an alias for
> xrange() in the two affected tests (044 and 163).
> 
> In one instance, we only wanted the first instance of the result of a
> filter() call.  Instead of using next(filter()) which would work only in
> Python 3, or list(filter())[0] which would work everywhere but is a bit
> weird, this instance is changed to use list comprehension with a next()
> wrapped around, which works both in 2.7 and 3.

Nit: the expression you put inside next(...) is not a list
comprehension; it's a generator expression.  A list comprehension
expression would generate the full list in advance before you get
the first element.

It would be OK to rewrite the expression using an actual list
comprehension:

    drive = [drive for drive in result if drive['device'] == 'drive0'][0]

But the solution you chose is OK, too.

Reviewed-by: Eduardo Habkost <address@hidden>

> 
> Signed-off-by: Max Reitz <address@hidden>
> ---
>  tests/qemu-iotests/044 | 16 ++++++++++------
>  tests/qemu-iotests/056 |  2 +-
>  tests/qemu-iotests/065 |  4 ++--
>  tests/qemu-iotests/124 |  4 ++--
>  tests/qemu-iotests/139 |  2 +-
>  tests/qemu-iotests/163 | 11 +++++++----
>  6 files changed, 23 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/qemu-iotests/044 b/tests/qemu-iotests/044
> index 7ef5e46fe9..9ec3dba734 100755
> --- a/tests/qemu-iotests/044
> +++ b/tests/qemu-iotests/044
> @@ -26,6 +26,10 @@ import iotests
>  from iotests import qemu_img, qemu_img_verbose, qemu_io
>  import struct
>  import subprocess
> +import sys
> +
> +if sys.version_info.major == 2:
> +    range = xrange
>  
>  test_img = os.path.join(iotests.test_dir, 'test.img')
>  
> @@ -52,23 +56,23 @@ class TestRefcountTableGrowth(iotests.QMPTestCase):
>              # Write a refcount table
>              fd.seek(off_reftable)
>  
> -            for i in xrange(0, h.refcount_table_clusters):
> +            for i in range(0, h.refcount_table_clusters):
>                  sector = b''.join(struct.pack('>Q',
>                      off_refblock + i * 64 * 512 + j * 512)
> -                    for j in xrange(0, 64))
> +                    for j in range(0, 64))
>                  fd.write(sector)
>  
>              # Write the refcount blocks
>              assert(fd.tell() == off_refblock)
>              sector = b''.join(struct.pack('>H', 1) for j in range(0, 64 * 
> 256))
> -            for block in xrange(0, h.refcount_table_clusters):
> +            for block in range(0, h.refcount_table_clusters):
>                  fd.write(sector)
>  
>              # Write the L1 table
>              assert(fd.tell() == off_l1)
>              assert(off_l2 + 512 * h.l1_size == off_data)
>              table = b''.join(struct.pack('>Q', (1 << 63) | off_l2 + 512 * j)
> -                for j in xrange(0, h.l1_size))
> +                for j in range(0, h.l1_size))
>              fd.write(table)
>  
>              # Write the L2 tables
> @@ -79,14 +83,14 @@ class TestRefcountTableGrowth(iotests.QMPTestCase):
>              off = off_data
>              while remaining > 1024 * 512:
>                  pytable = list((1 << 63) | off + 512 * j
> -                    for j in xrange(0, 1024))
> +                    for j in range(0, 1024))
>                  table = struct.pack('>1024Q', *pytable)
>                  fd.write(table)
>                  remaining = remaining - 1024 * 512
>                  off = off + 1024 * 512
>  
>              table = b''.join(struct.pack('>Q', (1 << 63) | off + 512 * j)
> -                for j in xrange(0, remaining // 512))
> +                for j in range(0, remaining // 512))
>              fd.write(table)
>  
>  
> diff --git a/tests/qemu-iotests/056 b/tests/qemu-iotests/056
> index 223292175a..3df323984d 100755
> --- a/tests/qemu-iotests/056
> +++ b/tests/qemu-iotests/056
> @@ -32,7 +32,7 @@ target_img = os.path.join(iotests.test_dir, 'target.img')
>  def img_create(img, fmt=iotests.imgfmt, size='64M', **kwargs):
>      fullname = os.path.join(iotests.test_dir, '%s.%s' % (img, fmt))
>      optargs = []
> -    for k,v in kwargs.iteritems():
> +    for k,v in kwargs.items():
>          optargs = optargs + ['-o', '%s=%s' % (k,v)]
>      args = ['create', '-f', fmt] + optargs + [fullname, size]
>      iotests.qemu_img(*args)
> diff --git a/tests/qemu-iotests/065 b/tests/qemu-iotests/065
> index 72aa9707c7..8bac383ea7 100755
> --- a/tests/qemu-iotests/065
> +++ b/tests/qemu-iotests/065
> @@ -59,7 +59,7 @@ class TestQemuImgInfo(TestImageInfoSpecific):
>                      :data.index('')]
>          for field in data:
>              self.assertTrue(re.match('^ {4}[^ ]', field) is not None)
> -        data = map(lambda line: line.strip(), data)
> +        data = [line.strip() for line in data]
>          self.assertEqual(data, self.human_compare)
>  
>  class TestQMP(TestImageInfoSpecific):
> @@ -80,7 +80,7 @@ class TestQMP(TestImageInfoSpecific):
>  
>      def test_qmp(self):
>          result = self.vm.qmp('query-block')['return']
> -        drive = filter(lambda drive: drive['device'] == 'drive0', result)[0]
> +        drive = next(drive for drive in result if drive['device'] == 
> 'drive0')
>          data = drive['inserted']['image']['format-specific']
>          self.assertEqual(data['type'], iotests.imgfmt)
>          self.assertEqual(data['data'], self.compare)
> diff --git a/tests/qemu-iotests/124 b/tests/qemu-iotests/124
> index 3ea4ac53f5..9f189e3b54 100755
> --- a/tests/qemu-iotests/124
> +++ b/tests/qemu-iotests/124
> @@ -39,7 +39,7 @@ def try_remove(img):
>  def transaction_action(action, **kwargs):
>      return {
>          'type': action,
> -        'data': dict((k.replace('_', '-'), v) for k, v in kwargs.iteritems())
> +        'data': dict((k.replace('_', '-'), v) for k, v in kwargs.items())
>      }
>  
>  
> @@ -134,7 +134,7 @@ class TestIncrementalBackupBase(iotests.QMPTestCase):
>      def img_create(self, img, fmt=iotests.imgfmt, size='64M',
>                     parent=None, parentFormat=None, **kwargs):
>          optargs = []
> -        for k,v in kwargs.iteritems():
> +        for k,v in kwargs.items():
>              optargs = optargs + ['-o', '%s=%s' % (k,v)]
>          args = ['create', '-f', fmt] + optargs + [img, size]
>          if parent:
> diff --git a/tests/qemu-iotests/139 b/tests/qemu-iotests/139
> index cc7fe337f3..62402c1c35 100755
> --- a/tests/qemu-iotests/139
> +++ b/tests/qemu-iotests/139
> @@ -51,7 +51,7 @@ class TestBlockdevDel(iotests.QMPTestCase):
>      # Check whether a BlockDriverState exists
>      def checkBlockDriverState(self, node, must_exist = True):
>          result = self.vm.qmp('query-named-block-nodes')
> -        nodes = filter(lambda x: x['node-name'] == node, result['return'])
> +        nodes = [x for x in result['return'] if x['node-name'] == node]
>          self.assertLessEqual(len(nodes), 1)
>          self.assertEqual(must_exist, len(nodes) == 1)
>  
> diff --git a/tests/qemu-iotests/163 b/tests/qemu-iotests/163
> index 5fd424761b..158ba5d092 100755
> --- a/tests/qemu-iotests/163
> +++ b/tests/qemu-iotests/163
> @@ -18,9 +18,12 @@
>  # along with this program.  If not, see <http://www.gnu.org/licenses/>.
>  #
>  
> -import os, random, iotests, struct, qcow2
> +import os, random, iotests, struct, qcow2, sys
>  from iotests import qemu_img, qemu_io, image_size
>  
> +if sys.version_info.major == 2:
> +    range = xrange
> +
>  test_img = os.path.join(iotests.test_dir, 'test.img')
>  check_img = os.path.join(iotests.test_dir, 'check.img')
>  
> @@ -41,7 +44,7 @@ class ShrinkBaseClass(iotests.QMPTestCase):
>          div_roundup = lambda n, d: (n + d - 1) // d
>  
>          def split_by_n(data, n):
> -            for x in xrange(0, len(data), n):
> +            for x in range(0, len(data), n):
>                  yield struct.unpack('>Q', data[x:x + n])[0] & l1_mask
>  
>          def check_l1_table(h, l1_data):
> @@ -135,8 +138,8 @@ class ShrinkBaseClass(iotests.QMPTestCase):
>          self.image_verify()
>  
>      def test_random_write(self):
> -        offs_list = range(0, size_to_int(self.image_len),
> -                          size_to_int(self.chunk_size))
> +        offs_list = list(range(0, size_to_int(self.image_len),
> +                               size_to_int(self.chunk_size)))
>          random.shuffle(offs_list)
>          for offs in offs_list:
>              qemu_io('-c', 'write -P 0xff %d %s' % (offs, self.chunk_size),
> -- 
> 2.17.1
> 

-- 
Eduardo