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 <ehabk...@redhat.com>
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> 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
