From: John Snow <jsnow@redhat.com>

This doesn't fix everything in here, but it does help clean up the

pylint report considerably.

This should be 100% style changes only; the intent is to make pylint

more useful by working on establishing a baseline for iotests that we

can gate against in the future.

Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200331000014.11581-2-jsnow@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/iotests.py | 83 ++++++++++++++++++-----------------

1 file changed, 43 insertions(+), 40 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@

# along with this program. If not, see <http://www.gnu.org/licenses/>.

#

-import errno

import os

import re

import subprocess

-import string

import unittest

import sys

import struct

@@ -XXX,XX +XXX,XX @@ import faulthandler

sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))

from qemu import qtest

-assert sys.version_info >= (3,6)

+assert sys.version_info >= (3, 6)

faulthandler.enable()

@@ -XXX,XX +XXX,XX @@ def qemu_img_log(*args):

return result

def img_info_log(filename, filter_path=None, imgopts=False, extra_args=[]):

- args = [ 'info' ]

+ args = ['info']

if imgopts:

args.append('--image-opts')

else:

- args += [ '-f', imgfmt ]

+ args += ['-f', imgfmt]

args += extra_args

args.append(filename)

@@ -XXX,XX +XXX,XX @@ class QemuIoInteractive:

# quit command is in close(), '\n' is added automatically

assert '\n' not in cmd

cmd = cmd.strip()

- assert cmd != 'q' and cmd != 'quit'

+ assert cmd not in ('q', 'quit')

self._p.stdin.write(cmd + '\n')

self._p.stdin.flush()

return self._read_output()

@@ -XXX,XX +XXX,XX @@ def qemu_nbd_early_pipe(*args):

sys.stderr.write('qemu-nbd received signal %i: %s\n' %

(-exitcode,

' '.join(qemu_nbd_args + ['--fork'] + list(args))))

- if exitcode == 0:

- return exitcode, ''

- else:

- return exitcode, subp.communicate()[0]

+

+ return exitcode, subp.communicate()[0] if exitcode else ''

def qemu_nbd_popen(*args):

'''Run qemu-nbd in daemon mode and return the parent's exit code'''

@@ -XXX,XX +XXX,XX @@ def filter_qmp(qmsg, filter_fn):

items = qmsg.items()

for k, v in items:

- if isinstance(v, list) or isinstance(v, dict):

+ if isinstance(v, (dict, list)):

qmsg[k] = filter_qmp(v, filter_fn)

else:

qmsg[k] = filter_fn(k, v)

@@ -XXX,XX +XXX,XX @@ def filter_testfiles(msg):

return msg.replace(prefix, 'TEST_DIR/PID-')

def filter_qmp_testfiles(qmsg):

- def _filter(key, value):

+ def _filter(_key, value):

if is_str(value):

return filter_testfiles(value)

return value

@@ -XXX,XX +XXX,XX @@ def filter_imgfmt(msg):

return msg.replace(imgfmt, 'IMGFMT')

def filter_qmp_imgfmt(qmsg):

- def _filter(key, value):

+ def _filter(_key, value):

if is_str(value):

return filter_imgfmt(value)

return value

@@ -XXX,XX +XXX,XX @@ def log(msg, filters=[], indent=None):

If indent is provided, JSON serializable messages are pretty-printed.'''

for flt in filters:

msg = flt(msg)

- if isinstance(msg, dict) or isinstance(msg, list):

+ if isinstance(msg, (dict, list)):

# Python < 3.4 needs to know not to add whitespace when pretty-printing:

separators = (', ', ': ') if indent is None else (',', ': ')

# Don't sort if it's already sorted

@@ -XXX,XX +XXX,XX @@ def log(msg, filters=[], indent=None):

print(msg)

class Timeout:

- def __init__(self, seconds, errmsg = "Timeout"):

+ def __init__(self, seconds, errmsg="Timeout"):

self.seconds = seconds

self.errmsg = errmsg

def __enter__(self):

signal.signal(signal.SIGALRM, self.timeout)

signal.setitimer(signal.ITIMER_REAL, self.seconds)

return self

- def __exit__(self, type, value, traceback):

+ def __exit__(self, exc_type, value, traceback):

signal.setitimer(signal.ITIMER_REAL, 0)

return False

def timeout(self, signum, frame):

@@ -XXX,XX +XXX,XX @@ class Timeout:

def file_pattern(name):

return "{0}-{1}".format(os.getpid(), name)

-class FilePaths(object):

+class FilePaths:

"""

FilePaths is an auto-generated filename that cleans itself up.

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

self.pause_drive(drive, "write_aio")

return

self.qmp('human-monitor-command',

- command_line='qemu-io %s "break %s bp_%s"' % (drive, event, drive))

+ command_line='qemu-io %s "break %s bp_%s"' % (drive, event, drive))

def resume_drive(self, drive):

self.qmp('human-monitor-command',

- command_line='qemu-io %s "remove_break bp_%s"' % (drive, drive))

+ command_line='qemu-io %s "remove_break bp_%s"' % (drive, drive))

def hmp_qemu_io(self, drive, cmd):

'''Write to a given drive using an HMP command'''

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

if output is None:

output = dict()

if isinstance(obj, list):

- for i in range(len(obj)):

- self.flatten_qmp_object(obj[i], output, basestr + str(i) + '.')

+ for i, item in enumerate(obj):

+ self.flatten_qmp_object(item, output, basestr + str(i) + '.')

elif isinstance(obj, dict):

for key in obj:

self.flatten_qmp_object(obj[key], output, basestr + key + '.')

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

for bitmap in bitmaps[node_name]:

if bitmap.get('name', '') == bitmap_name:

- if recording is None:

- return bitmap

- elif bitmap.get('recording') == recording:

+ if recording is None or bitmap.get('recording') == recording:

return bitmap

return None

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

assert node is not None, 'Cannot follow path %s%s' % (root, path)

try:

- node_id = next(edge['child'] for edge in graph['edges'] \

- if edge['parent'] == node['id'] and

- edge['name'] == child_name)

+ node_id = next(edge['child'] for edge in graph['edges']

+ if (edge['parent'] == node['id'] and

+ edge['name'] == child_name))

+

+ node = next(node for node in graph['nodes']

+ if node['id'] == node_id)

- node = next(node for node in graph['nodes'] \

- if node['id'] == node_id)

except StopIteration:

node = None

@@ -XXX,XX +XXX,XX @@ index_re = re.compile(r'([^\[]+)\[([^\]]+)\]')

class QMPTestCase(unittest.TestCase):

'''Abstract base class for QMP test cases'''

+ def __init__(self, *args, **kwargs):

+ super().__init__(*args, **kwargs)

+ # Many users of this class set a VM property we rely on heavily

+ # in the methods below.

+ self.vm = None

+

def dictpath(self, d, path):

'''Traverse a path in a nested dict'''

for component in path.split('/'):

@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):

else:

self.assertEqual(result, value,

'"%s" is "%s", expected "%s"'

- % (path, str(result), str(value)))

+ % (path, str(result), str(value)))

def assert_no_active_block_jobs(self):

result = self.vm.qmp('query-block-jobs')

@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):

"""Issue a query-named-block-nodes and assert node_name and/or

file_name is present in the result"""

def check_equal_or_none(a, b):

- return a == None or b == None or a == b

+ return a is None or b is None or a == b

assert node_name or file_name

result = self.vm.qmp('query-named-block-nodes')

for x in result["return"]:

if check_equal_or_none(x.get("node-name"), node_name) and \

check_equal_or_none(x.get("file"), file_name):

return

- self.assertTrue(False, "Cannot find %s %s in result:\n%s" % \

- (node_name, file_name, result))

+ self.fail("Cannot find %s %s in result:\n%s" %

+ (node_name, file_name, result))

def assert_json_filename_equal(self, json_filename, reference):

'''Asserts that the given filename is a json: filename and that its

@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):

self.assert_qmp(event, 'data/error', error)

self.assert_no_active_block_jobs()

return event

- elif event['event'] == 'JOB_STATUS_CHANGE':

+ if event['event'] == 'JOB_STATUS_CHANGE':

self.assert_qmp(event, 'data/id', drive)

def wait_ready(self, drive='drive0'):

- '''Wait until a block job BLOCK_JOB_READY event'''

- f = {'data': {'type': 'mirror', 'device': drive } }

- event = self.vm.event_wait(name='BLOCK_JOB_READY', match=f)

+ """Wait until a BLOCK_JOB_READY event, and return the event."""

+ f = {'data': {'type': 'mirror', 'device': drive}}

+ return self.vm.event_wait(name='BLOCK_JOB_READY', match=f)

def wait_ready_and_cancel(self, drive='drive0'):

self.wait_ready(drive=drive)

@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):

for job in result['return']:

if job['device'] == job_id:

found = True

- if job['paused'] == True and job['busy'] == False:

+ if job['paused'] and not job['busy']:

return job

break

assert found

@@ -XXX,XX +XXX,XX @@ def qemu_pipe(*args):

universal_newlines=True)

exitcode = subp.wait()

if exitcode < 0:

- sys.stderr.write('qemu received signal %i: %s\n' % (-exitcode,

- ' '.join(args)))

+ sys.stderr.write('qemu received signal %i: %s\n' %

+ (-exitcode, ' '.join(args)))

return subp.communicate()[0]

def supported_formats(read_only=False):

@@ -XXX,XX +XXX,XX @@ def skip_if_unsupported(required_formats=[], read_only=False):

if usf_list:

test_case.case_skip('{}: formats {} are not whitelisted'.format(

test_case, usf_list))

+ return None

else:

return func(test_case, *args, **kwargs)

return func_wrapper

@@ -XXX,XX +XXX,XX @@ def skip_if_user_is_root(func):

def func_wrapper(*args, **kwargs):

if os.getuid() == 0:

case_notrun('{}: cannot be run as root'.format(args[0]))

+ return None

else:

return func(*args, **kwargs)

return func_wrapper

--

2.26.2

From: John Snow <jsnow@redhat.com>

It shadows (with a different type) the built-in format.

Use something else.

Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200331000014.11581-3-jsnow@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/055 | 3 ++-

tests/qemu-iotests/iotests.py | 6 +++---

2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055

index XXXXXXX..XXXXXXX 100755

--- a/tests/qemu-iotests/055

+++ b/tests/qemu-iotests/055

@@ -XXX,XX +XXX,XX @@ class TestDriveCompression(iotests.QMPTestCase):

qemu_img('create', '-f', fmt, blockdev_target_img,

str(TestDriveCompression.image_len), *args)

if attach_target:

- self.vm.add_drive(blockdev_target_img, format=fmt, interface="none")

+ self.vm.add_drive(blockdev_target_img,

+ img_format=fmt, interface="none")

self.vm.launch()

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

self._args.append(opts)

return self

- def add_drive(self, path, opts='', interface='virtio', format=imgfmt):

+ def add_drive(self, path, opts='', interface='virtio', img_format=imgfmt):

'''Add a virtio-blk drive to the VM'''

options = ['if=%s' % interface,

'id=drive%d' % self._num_drives]

if path is not None:

options.append('file=%s' % path)

- options.append('format=%s' % format)

+ options.append('format=%s' % img_format)

options.append('cache=%s' % cachemode)

options.append('aio=%s' % aiomode)

if opts:

options.append(opts)

- if format == 'luks' and 'key-secret' not in opts:

+ if img_format == 'luks' and 'key-secret' not in opts:

# default luks support

if luks_default_secret_object not in self._args:

self.add_object(luks_default_secret_object)

--

2.26.2

From: John Snow <jsnow@redhat.com>

The right way to solve this is to come up with a virtual environment

infrastructure that sets all the paths correctly, and/or to create

installable python modules that can be imported normally.

That's hard, so just silence this error for now.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Signed-off-by: John Snow <jsnow@redhat.com>

Message-Id: <20200331000014.11581-4-jsnow@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/iotests.py | 1 +

1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@ import io

from collections import OrderedDict

import faulthandler

+# pylint: disable=import-error, wrong-import-position

sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))

from qemu import qtest

--

2.26.2

From: John Snow <jsnow@redhat.com>

It's bad hygiene: if we modify this list, it will be modified across all

invocations.

(Remaining bad usages are fixed in a subsequent patch which changes the

function signature anyway.)

Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200331000014.11581-5-jsnow@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/iotests.py | 24 ++++++++++++------------

1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@ def qemu_img_log(*args):

log(result, filters=[filter_testfiles])

return result

-def img_info_log(filename, filter_path=None, imgopts=False, extra_args=[]):

+def img_info_log(filename, filter_path=None, imgopts=False, extra_args=()):

args = ['info']

if imgopts:

args.append('--image-opts')

@@ -XXX,XX +XXX,XX @@ def filter_qmp_imgfmt(qmsg):

return value

return filter_qmp(qmsg, _filter)

-def log(msg, filters=[], indent=None):

+def log(msg, filters=(), indent=None):

'''Logs either a string message or a JSON serializable message (like QMP).

If indent is provided, JSON serializable messages are pretty-printed.'''

for flt in filters:

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

result.append(filter_qmp_event(ev))

return result

- def qmp_log(self, cmd, filters=[], indent=None, **kwargs):

+ def qmp_log(self, cmd, filters=(), indent=None, **kwargs):

full_cmd = OrderedDict((

("execute", cmd),

("arguments", ordered_qmp(kwargs))

@@ -XXX,XX +XXX,XX @@ def case_notrun(reason):

open('%s/%s.casenotrun' % (output_dir, seq), 'a').write(

' [case not run] ' + reason + '\n')

-def verify_image_format(supported_fmts=[], unsupported_fmts=[]):

+def verify_image_format(supported_fmts=(), unsupported_fmts=()):

assert not (supported_fmts and unsupported_fmts)

if 'generic' in supported_fmts and \

@@ -XXX,XX +XXX,XX @@ def verify_image_format(supported_fmts=[], unsupported_fmts=[]):

if not_sup or (imgfmt in unsupported_fmts):

notrun('not suitable for this image format: %s' % imgfmt)

-def verify_protocol(supported=[], unsupported=[]):

+def verify_protocol(supported=(), unsupported=()):

assert not (supported and unsupported)

if 'generic' in supported:

@@ -XXX,XX +XXX,XX @@ def verify_platform(supported=None, unsupported=None):

if not any((sys.platform.startswith(x) for x in supported)):

notrun('not suitable for this OS: %s' % sys.platform)

-def verify_cache_mode(supported_cache_modes=[]):

+def verify_cache_mode(supported_cache_modes=()):

if supported_cache_modes and (cachemode not in supported_cache_modes):

notrun('not suitable for this cache mode: %s' % cachemode)

-def verify_aio_mode(supported_aio_modes=[]):

+def verify_aio_mode(supported_aio_modes=()):

if supported_aio_modes and (aiomode not in supported_aio_modes):

notrun('not suitable for this aio mode: %s' % aiomode)

@@ -XXX,XX +XXX,XX @@ def supported_formats(read_only=False):

return supported_formats.formats[read_only]

-def skip_if_unsupported(required_formats=[], read_only=False):

+def skip_if_unsupported(required_formats=(), read_only=False):

'''Skip Test Decorator

Runs the test if all the required formats are whitelisted'''

def skip_test_decorator(func):

@@ -XXX,XX +XXX,XX @@ def execute_unittest(output, verbosity, debug):

sys.stderr.write(out)

def execute_test(test_function=None,

- supported_fmts=[],

+ supported_fmts=(),

supported_platforms=None,

- supported_cache_modes=[], supported_aio_modes={},

- unsupported_fmts=[], supported_protocols=[],

- unsupported_protocols=[]):

+ supported_cache_modes=(), supported_aio_modes=(),

+ unsupported_fmts=(), supported_protocols=(),

+ unsupported_protocols=()):

"""Run either unittest or script-style tests."""

# We are using TEST_DIR and QEMU_DEFAULT_MACHINE as proxies to

--

2.26.2

From: John Snow <jsnow@redhat.com>

This allows others to get repeatable results with pylint. If you run

`pylint iotests.py`, you should see a 100% pass.

Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200331000014.11581-6-jsnow@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/pylintrc | 22 ++++++++++++++++++++++

1 file changed, 22 insertions(+)

create mode 100644 tests/qemu-iotests/pylintrc

diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc

new file mode 100644

index XXXXXXX..XXXXXXX

--- /dev/null

+++ b/tests/qemu-iotests/pylintrc

@@ -XXX,XX +XXX,XX @@

+[MESSAGES CONTROL]

+

+# Disable the message, report, category or checker with the given id(s). You

+# can either give multiple identifiers separated by comma (,) or put this

+# option multiple times (only on the command line, not in the configuration

+# file where it should appear only once). You can also use "--disable=all" to

+# disable everything first and then reenable specific checks. For example, if

+# you want to run only the similarities checker, you can use "--disable=all

+# --enable=similarities". If you want to run only the classes checker, but have

+# no Warning level messages displayed, use "--disable=all --enable=classes

+# --disable=W".

+disable=invalid-name,

+ no-else-return,

+ too-few-public-methods,

+ too-many-arguments,

+ too-many-branches,

+ too-many-lines,

+ too-many-locals,

+ too-many-public-methods,

+ # These are temporary, and should be removed:

+ line-too-long,

+ missing-docstring,

--

2.26.2

From: John Snow <jsnow@redhat.com>

I had to fix a merge conflict, so do this tiny harmless thing while I'm

here.

Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200331000014.11581-7-jsnow@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/iotests.py | 18 +++++++++---------

1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@

# along with this program. If not, see <http://www.gnu.org/licenses/>.

#

+import atexit

+from collections import OrderedDict

+import faulthandler

+import io

+import json

+import logging

import os

import re

+import signal

+import struct

import subprocess

-import unittest

import sys

-import struct

-import json

-import signal

-import logging

-import atexit

-import io

-from collections import OrderedDict

-import faulthandler

+import unittest

# pylint: disable=import-error, wrong-import-position

sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))

--

2.26.2

From: John Snow <jsnow@redhat.com>

We no longer need to accommodate <3.4, drop this code.

(The lines were > 79 chars and it stood out.)

Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200331000014.11581-8-jsnow@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/iotests.py | 5 +----

1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@ def log(msg, filters=(), indent=None):

for flt in filters:

msg = flt(msg)

if isinstance(msg, (dict, list)):

- # Python < 3.4 needs to know not to add whitespace when pretty-printing:

- separators = (', ', ': ') if indent is None else (',', ': ')

# Don't sort if it's already sorted

do_sort = not isinstance(msg, OrderedDict)

- print(json.dumps(msg, sort_keys=do_sort,

- indent=indent, separators=separators))

+ print(json.dumps(msg, sort_keys=do_sort, indent=indent))

else:

print(msg)

--

2.26.2

From: John Snow <jsnow@redhat.com>

Representing nested, recursive data structures in mypy is notoriously

difficult; the best we can reliably do right now is denote the leaf

types as "Any" while describing the general shape of the data.

Regardless, this fully annotates the log() function.

Typing notes:

TypeVar is a Type variable that can optionally be constrained by a

sequence of possible types. This variable is bound to a specific type

per-invocation, like a Generic.

log() behaves as log<Msg>() now, where the incoming type informs the

signature it expects for any filter arguments passed in. If Msg is a

str, then filter should take and return a str.

Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200331000014.11581-9-jsnow@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/iotests.py | 14 +++++++++++---

1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@ import signal

import struct

import subprocess

import sys

+from typing import (Any, Callable, Dict, Iterable, List, Optional, TypeVar)

import unittest

# pylint: disable=import-error, wrong-import-position

@@ -XXX,XX +XXX,XX @@ def filter_qmp_imgfmt(qmsg):

return value

return filter_qmp(qmsg, _filter)

-def log(msg, filters=(), indent=None):

- '''Logs either a string message or a JSON serializable message (like QMP).

- If indent is provided, JSON serializable messages are pretty-printed.'''

+

+Msg = TypeVar('Msg', Dict[str, Any], List[Any], str)

+

+def log(msg: Msg,

+ filters: Iterable[Callable[[Msg], Msg]] = (),

+ indent: Optional[int] = None) -> None:

+ """

+ Logs either a string message or a JSON serializable message (like QMP).

+ If indent is provided, JSON serializable messages are pretty-printed.

+ """

for flt in filters:

msg = flt(msg)

if isinstance(msg, (dict, list)):

--

2.26.2

From: John Snow <jsnow@redhat.com>

79 is the PEP8 recommendation. This recommendation works well for

reading patch diffs in TUI email clients.

Signed-off-by: John Snow <jsnow@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Message-Id: <20200331000014.11581-10-jsnow@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/iotests.py | 64 +++++++++++++++++++++++------------

tests/qemu-iotests/pylintrc | 6 +++-

2 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@ luks_default_key_secret_opt = 'key-secret=keysec0'

def qemu_img(*args):

'''Run qemu-img and return the exit code'''

devnull = open('/dev/null', 'r+')

- exitcode = subprocess.call(qemu_img_args + list(args), stdin=devnull, stdout=devnull)

+ exitcode = subprocess.call(qemu_img_args + list(args),

+ stdin=devnull, stdout=devnull)

if exitcode < 0:

- sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args))))

+ sys.stderr.write('qemu-img received signal %i: %s\n'

+ % (-exitcode, ' '.join(qemu_img_args + list(args))))

return exitcode

def ordered_qmp(qmsg, conv_keys=True):

@@ -XXX,XX +XXX,XX @@ def qemu_img_verbose(*args):

'''Run qemu-img without suppressing its output and return the exit code'''

exitcode = subprocess.call(qemu_img_args + list(args))

if exitcode < 0:

- sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args))))

+ sys.stderr.write('qemu-img received signal %i: %s\n'

+ % (-exitcode, ' '.join(qemu_img_args + list(args))))

return exitcode

def qemu_img_pipe(*args):

@@ -XXX,XX +XXX,XX @@ def qemu_img_pipe(*args):

universal_newlines=True)

exitcode = subp.wait()

if exitcode < 0:

- sys.stderr.write('qemu-img received signal %i: %s\n' % (-exitcode, ' '.join(qemu_img_args + list(args))))

+ sys.stderr.write('qemu-img received signal %i: %s\n'

+ % (-exitcode, ' '.join(qemu_img_args + list(args))))

return subp.communicate()[0]

def qemu_img_log(*args):

@@ -XXX,XX +XXX,XX @@ def qemu_io(*args):

universal_newlines=True)

exitcode = subp.wait()

if exitcode < 0:

- sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))

+ sys.stderr.write('qemu-io received signal %i: %s\n'

+ % (-exitcode, ' '.join(args)))

return subp.communicate()[0]

def qemu_io_log(*args):

@@ -XXX,XX +XXX,XX @@ win32_re = re.compile(r"\r")

def filter_win32(msg):

return win32_re.sub("", msg)

-qemu_io_re = re.compile(r"[0-9]* ops; [0-9\/:. sec]* \([0-9\/.inf]* [EPTGMKiBbytes]*\/sec and [0-9\/.inf]* ops\/sec\)")

+qemu_io_re = re.compile(r"[0-9]* ops; [0-9\/:. sec]* "

+ r"\([0-9\/.inf]* [EPTGMKiBbytes]*\/sec "

+ r"and [0-9\/.inf]* ops\/sec\)")

def filter_qemu_io(msg):

msg = filter_win32(msg)

- return qemu_io_re.sub("X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)", msg)

+ return qemu_io_re.sub("X ops; XX:XX:XX.X "

+ "(XXX YYY/sec and XXX ops/sec)", msg)

chown_re = re.compile(r"chown [0-9]+:[0-9]+")

def filter_chown(msg):

@@ -XXX,XX +XXX,XX @@ def filter_img_info(output, filename):

line = filter_testfiles(line)

line = line.replace(imgfmt, 'IMGFMT')

line = re.sub('iters: [0-9]+', 'iters: XXX', line)

- line = re.sub('uuid: [-a-f0-9]+', 'uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX', line)

+ line = re.sub('uuid: [-a-f0-9]+',

+ 'uuid: XXXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXXXXX',

+ line)

line = re.sub('cid: [0-9]+', 'cid: XXXXXXXXXX', line)

lines.append(line)

return '\n'.join(lines)

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

self.pause_drive(drive, "write_aio")

return

self.qmp('human-monitor-command',

- command_line='qemu-io %s "break %s bp_%s"' % (drive, event, drive))

+ command_line='qemu-io %s "break %s bp_%s"'

+ % (drive, event, drive))

def resume_drive(self, drive):

self.qmp('human-monitor-command',

- command_line='qemu-io %s "remove_break bp_%s"' % (drive, drive))

+ command_line='qemu-io %s "remove_break bp_%s"'

+ % (drive, drive))

def hmp_qemu_io(self, drive, cmd):

'''Write to a given drive using an HMP command'''

@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):

idx = int(idx)

if not isinstance(d, dict) or component not in d:

- self.fail('failed path traversal for "%s" in "%s"' % (path, str(d)))

+ self.fail(f'failed path traversal for "{path}" in "{d}"')

d = d[component]

if m:

if not isinstance(d, list):

- self.fail('path component "%s" in "%s" is not a list in "%s"' % (component, path, str(d)))

+ self.fail(f'path component "{component}" in "{path}" '

+ f'is not a list in "{d}"')

try:

d = d[idx]

except IndexError:

- self.fail('invalid index "%s" in path "%s" in "%s"' % (idx, path, str(d)))

+ self.fail(f'invalid index "{idx}" in path "{path}" '

+ f'in "{d}"')

return d

def assert_qmp_absent(self, d, path):

@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):

'''Asserts that the given filename is a json: filename and that its

content is equal to the given reference object'''

self.assertEqual(json_filename[:5], 'json:')

- self.assertEqual(self.vm.flatten_qmp_object(json.loads(json_filename[5:])),

- self.vm.flatten_qmp_object(reference))

+ self.assertEqual(

+ self.vm.flatten_qmp_object(json.loads(json_filename[5:])),

+ self.vm.flatten_qmp_object(reference)

+ )

- def cancel_and_wait(self, drive='drive0', force=False, resume=False, wait=60.0):

+ def cancel_and_wait(self, drive='drive0', force=False,

+ resume=False, wait=60.0):

'''Cancel a block job and wait for it to finish, returning the event'''

result = self.vm.qmp('block-job-cancel', device=drive, force=force)

self.assert_qmp(result, 'return', {})

@@ -XXX,XX +XXX,XX @@ class QMPTestCase(unittest.TestCase):

self.assert_no_active_block_jobs()

return result

- def wait_until_completed(self, drive='drive0', check_offset=True, wait=60.0,

- error=None):

+ def wait_until_completed(self, drive='drive0', check_offset=True,

+ wait=60.0, error=None):

'''Wait for a block job to finish, returning the event'''

while True:

for event in self.vm.get_qmp_events(wait=wait):

@@ -XXX,XX +XXX,XX @@ def verify_quorum():

notrun('quorum support missing')

def qemu_pipe(*args):

- '''Run qemu with an option to print something and exit (e.g. a help option),

- and return its output'''

+ """

+ Run qemu with an option to print something and exit (e.g. a help option).

+

+ :return: QEMU's stdout output.

+ """

args = [qemu_prog] + qemu_opts + list(args)

subp = subprocess.Popen(args, stdout=subprocess.PIPE,

stderr=subprocess.STDOUT,

@@ -XXX,XX +XXX,XX @@ def skip_if_unsupported(required_formats=(), read_only=False):

usf_list = list(set(fmts) - set(supported_formats(read_only)))

if usf_list:

- test_case.case_skip('{}: formats {} are not whitelisted'.format(

- test_case, usf_list))

+ msg = f'{test_case}: formats {usf_list} are not whitelisted'

+ test_case.case_skip(msg)

return None

else:

return func(test_case, *args, **kwargs)

diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/pylintrc

+++ b/tests/qemu-iotests/pylintrc

@@ -XXX,XX +XXX,XX @@ disable=invalid-name,

too-many-locals,

too-many-public-methods,

# These are temporary, and should be removed:

- line-too-long,

missing-docstring,

+

+[FORMAT]

+

+# Maximum number of characters on a single line.

+max-line-length=79

--

2.26.2

From: John Snow <jsnow@redhat.com>

Minor cleanup for HMP functions; helps with line length and consolidates

HMP helpers through one implementation function.

Although we are adding a universal toggle to turn QMP logging on or off,

many existing callers to hmp functions don't expect that output to be

logged, which causes quite a few changes in the test output.

For now, offer a use_log parameter.

Typing notes:

QMPResponse is just an alias for Dict[str, Any]. It holds no special

meanings and it is not a formal subtype of Dict[str, Any]. It is best

thought of as a lexical synonym.

We may well wish to add stricter subtypes in the future for certain

shapes of data that are not formalized as Python objects, at which point

we can simply retire the alias and allow mypy to more strictly check

usages of the name.

Signed-off-by: John Snow <jsnow@redhat.com>

Message-Id: <20200331000014.11581-11-jsnow@redhat.com>

Reviewed-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Max Reitz <mreitz@redhat.com>

---

tests/qemu-iotests/iotests.py | 39 +++++++++++++++++++++--------------

1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py

index XXXXXXX..XXXXXXX 100644

--- a/tests/qemu-iotests/iotests.py

+++ b/tests/qemu-iotests/iotests.py

@@ -XXX,XX +XXX,XX @@ from qemu import qtest

assert sys.version_info >= (3, 6)

+# Type Aliases

+QMPResponse = Dict[str, Any]

+

+

faulthandler.enable()

# This will not work if arguments contain spaces but is necessary if we

@@ -XXX,XX +XXX,XX @@ class VM(qtest.QEMUQtestMachine):

self._args.append(addr)

return self

- def pause_drive(self, drive, event=None):

- '''Pause drive r/w operations'''

+ def hmp(self, command_line: str, use_log: bool = False) -> QMPResponse:

+ cmd = 'human-monitor-command'

+ kwargs = {'command-line': command_line}

+ if use_log:

+ return self.qmp_log(cmd, **kwargs)

+ else:

+ return self.qmp(cmd, **kwargs)

+

+ def pause_drive(self, drive: str, event: Optional[str] = None) -> None:

+ """Pause drive r/w operations"""

if not event:

self.pause_drive(drive, "read_aio")

self.pause_drive(drive, "write_aio")

return

- self.qmp('human-monitor-command',

- command_line='qemu-io %s "break %s bp_%s"'

- % (drive, event, drive))

-

- def resume_drive(self, drive):

- self.qmp('human-monitor-command',

- command_line='qemu-io %s "remove_break bp_%s"'

- % (drive, drive))

-

- def hmp_qemu_io(self, drive, cmd):

- '''Write to a given drive using an HMP command'''

- return self.qmp('human-monitor-command',

- command_line='qemu-io %s "%s"' % (drive, cmd))

+ self.hmp(f'qemu-io {drive} "break {event} bp_{drive}"')

+

+ def resume_drive(self, drive: str) -> None:

+ """Resume drive r/w operations"""

+ self.hmp(f'qemu-io {drive} "remove_break bp_{drive}"')

+

+ def hmp_qemu_io(self, drive: str, cmd: str,

+ use_log: bool = False) -> QMPResponse:

+ """Write to a given drive using an HMP command"""

+ return self.hmp(f'qemu-io {drive} "{cmd}"', use_log=use_log)

def flatten_qmp_object(self, obj, output=None, basestr=''):

if output is None:

--

2.26.2