All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>,
	qemu-block@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: [PATCH v6 1/9] iotests: do a light delinting
Date: Wed, 26 Feb 2020 19:06:31 -0500	[thread overview]
Message-ID: <20200227000639.9644-2-jsnow@redhat.com> (raw)
In-Reply-To: <20200227000639.9644-1-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. This will be important if (when?) we
begin adding type hints to our code base.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 tests/qemu-iotests/iotests.py | 88 ++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 43 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 8815052eb5..e8a0ea14fc 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -16,11 +16,9 @@
 # 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
@@ -34,7 +32,7 @@
 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)
 
 # This will not work if arguments contain spaces but is necessary if we
 # want to support the override options that ./check supports.
@@ -138,11 +136,11 @@ 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)
 
@@ -221,7 +219,7 @@ def cmd(self, cmd):
         # 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()
@@ -245,8 +243,7 @@ def qemu_nbd_early_pipe(*args):
                           ' '.join(qemu_nbd_args + ['--fork'] + list(args))))
     if exitcode == 0:
         return exitcode, ''
-    else:
-        return exitcode, subp.communicate()[0]
+    return exitcode, subp.communicate()[0]
 
 def qemu_nbd_popen(*args):
     '''Run qemu-nbd in daemon mode and return the parent's exit code'''
@@ -310,7 +307,7 @@ 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)
@@ -321,7 +318,7 @@ 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
@@ -347,7 +344,7 @@ 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
@@ -358,7 +355,7 @@ 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
@@ -369,14 +366,14 @@ 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):
@@ -385,7 +382,7 @@ def timeout(self, signum, frame):
 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.
 
@@ -455,10 +452,9 @@ def file_path(*names, base_dir=test_dir):
 def remote_filename(path):
     if imgproto == 'file':
         return path
-    elif imgproto == 'ssh':
+    if imgproto == 'ssh':
         return "ssh://%s@127.0.0.1:22%s" % (os.environ.get('USER'), path)
-    else:
-        raise Exception("Protocol %s not supported" % (imgproto))
+    raise Exception("Protocol %s not supported" % (imgproto))
 
 class VM(qtest.QEMUQtestMachine):
     '''A QEMU VM'''
@@ -532,11 +528,11 @@ def pause_drive(self, drive, event=None):
             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'''
@@ -547,8 +543,8 @@ def flatten_qmp_object(self, obj, output=None, basestr=''):
         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, atom in enumerate(obj):
+                self.flatten_qmp_object(atom, output, basestr + str(i) + '.')
         elif isinstance(obj, dict):
             for key in obj:
                 self.flatten_qmp_object(obj[key], output, basestr + key + '.')
@@ -703,9 +699,7 @@ def get_bitmap(self, node_name, bitmap_name, recording=None, bitmaps=None):
 
         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
 
@@ -756,12 +750,13 @@ def assert_block_path(self, root, path, expected_node, graph=None):
             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
 
@@ -779,6 +774,12 @@ def assert_block_path(self, root, path, expected_node, graph=None):
 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('/'):
@@ -824,7 +825,7 @@ def assert_qmp(self, d, path, value):
         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')
@@ -834,15 +835,15 @@ def assert_has_block_node(self, node_name=None, file_name=None):
         """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
@@ -891,13 +892,14 @@ def wait_until_completed(self, drive='drive0', check_offset=True, wait=60.0,
                         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 } }
+        f = {'data': {'type': 'mirror', 'device': drive}}
         event = self.vm.event_wait(name='BLOCK_JOB_READY', match=f)
+        return event
 
     def wait_ready_and_cancel(self, drive='drive0'):
         self.wait_ready(drive=drive)
@@ -926,7 +928,7 @@ def pause_wait(self, job_id='job0'):
                 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
@@ -1023,8 +1025,8 @@ 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):
@@ -1056,8 +1058,8 @@ def func_wrapper(test_case: QMPTestCase, *args, **kwargs):
             if usf_list:
                 test_case.case_skip('{}: formats {} are not whitelisted'.format(
                     test_case, usf_list))
-            else:
-                return func(test_case, *args, **kwargs)
+                return None
+            return func(test_case, *args, **kwargs)
         return func_wrapper
     return skip_test_decorator
 
@@ -1067,8 +1069,8 @@ 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]))
-        else:
-            return func(*args, **kwargs)
+            return None
+        return func(*args, **kwargs)
     return func_wrapper
 
 def execute_unittest(output, verbosity, debug):
-- 
2.21.1



  reply	other threads:[~2020-02-27  0:09 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-27  0:06 [PATCH v6 0/9] iotests: use python logging John Snow
2020-02-27  0:06 ` John Snow [this message]
2020-02-27 12:59   ` [PATCH v6 1/9] iotests: do a light delinting Max Reitz
2020-03-03 21:25     ` John Snow
2020-03-04 11:12       ` Kevin Wolf
2020-03-04 18:35         ` John Snow
2020-02-27  0:06 ` [PATCH v6 2/9] iotests: add script_initialize John Snow
2020-02-27 13:47   ` Max Reitz
2020-03-03 21:12     ` John Snow
2020-02-27  0:06 ` [PATCH v6 3/9] iotests: replace mutable list default args John Snow
2020-02-27 13:50   ` Max Reitz
2020-02-27  0:06 ` [PATCH v6 4/9] iotest 258: use script_main John Snow
2020-02-27 13:55   ` Max Reitz
2020-02-27 14:10   ` Philippe Mathieu-Daudé
2020-02-27  0:06 ` [PATCH v6 5/9] iotests: Mark verify functions as private John Snow
2020-02-27 13:59   ` Max Reitz
2020-02-27  0:06 ` [PATCH v6 6/9] iotests: use python logging for iotests.log() John Snow
2020-02-27 14:21   ` Max Reitz
2020-03-03 20:00     ` John Snow
2020-02-27  0:06 ` [PATCH v6 7/9] iotests: ignore import warnings from pylint John Snow
2020-02-27 14:14   ` Philippe Mathieu-Daudé
2020-03-03 19:57     ` John Snow
2020-03-04  0:02       ` Philippe Mathieu-Daudé
2020-03-04 18:59         ` John Snow
2020-02-27  0:06 ` [PATCH v6 8/9] iotests: don't use 'format' for drive_add John Snow
2020-02-27 14:12   ` Philippe Mathieu-Daudé
2020-02-27 14:26   ` Max Reitz
2020-02-27  0:06 ` [PATCH v6 9/9] iotests: add pylintrc file John Snow
2020-02-27  1:57   ` Eric Blake
2020-02-27 14:11   ` Philippe Mathieu-Daudé
2020-03-03 19:52     ` John Snow
2020-03-04  7:22   ` Markus Armbruster
2020-03-04 19:17     ` John Snow
2020-03-05  5:49       ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200227000639.9644-2-jsnow@redhat.com \
    --to=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.