[PATCH v2 0/2] pylint: fix new errors and warnings in qemu-iotests

[PATCH v2 0/2] pylint: fix new errors and warnings in qemu-iotests

[Emanuele Giuseppe Esposito]

There are some warnings and errors that we either miss or
are new in pylint. Anyways, test 297 of qemu-iotests fails
because of that, so we need to fix it.

All these fixes involve just indentation or additional spaces
added.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
v2:
* temporarly enable and then disable "bad whitespace" error in image-fleecing
* better indentation for a fix in test 129 in patch one

Emanuele Giuseppe Esposito (2):
  pylint: fix errors and warnings generated by tests/qemu-iotests/297
  qemu-iotests: fix image-fleecing pylint errors

 tests/qemu-iotests/129                  | 18 +++++++++---------
 tests/qemu-iotests/310                  | 16 ++++++++--------
 tests/qemu-iotests/check                | 11 ++++++-----
 tests/qemu-iotests/iotests.py           |  7 ++++---
 tests/qemu-iotests/tests/image-fleecing |  8 ++++++--
 5 files changed, 33 insertions(+), 27 deletions(-)

-- 
2.27.0

[PATCH v2 1/2] pylint: fix errors and warnings generated by tests/qemu-iotests/297

[Emanuele Giuseppe Esposito]

Test 297 in tests/qemu-iotests currently fails: pylint has
learned new things to check, or we simply missed them.

All fixes in this patch are related to additional spaces used
or wrong indentation. No functional change intended.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 tests/qemu-iotests/129                  | 18 +++++++++---------
 tests/qemu-iotests/310                  | 16 ++++++++--------
 tests/qemu-iotests/check                | 11 ++++++-----
 tests/qemu-iotests/iotests.py           |  7 ++++---
 tests/qemu-iotests/tests/image-fleecing |  4 ++--
 5 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129
index 5251e2669e..c75ec62ecf 100755
--- a/tests/qemu-iotests/129
+++ b/tests/qemu-iotests/129
@@ -77,8 +77,8 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
         self.do_test_stop("drive-backup", device="drive0",
                           target=self.target_img, format=iotests.imgfmt,
                           sync="full",
-                          x_perf={ 'max-chunk': 65536,
-                                   'max-workers': 8 })
+                          x_perf={'max-chunk': 65536,
+                                  'max-workers': 8})
 
     def test_block_commit(self):
         # Add overlay above the source node so that we actually use a
@@ -88,13 +88,13 @@ class TestStopWithBlockJob(iotests.QMPTestCase):
                          '1G')
 
         result = self.vm.qmp('blockdev-add', **{
-                                 'node-name': 'overlay',
-                                 'driver': iotests.imgfmt,
-                                 'file': {
-                                     'driver': 'file',
-                                     'filename': self.overlay_img
-                                 }
-                             })
+            'node-name': 'overlay',
+            'driver': iotests.imgfmt,
+            'file': {
+                'driver': 'file',
+                'filename': self.overlay_img
+            }
+        })
         self.assert_qmp(result, 'return', {})
 
         result = self.vm.qmp('blockdev-snapshot',
diff --git a/tests/qemu-iotests/310 b/tests/qemu-iotests/310
index 9d9c928c4b..33c3411869 100755
--- a/tests/qemu-iotests/310
+++ b/tests/qemu-iotests/310
@@ -48,11 +48,11 @@ with iotests.FilePath('base.img') as base_img_path, \
     assert qemu_io_silent(base_img_path, '-c', 'write -P 1 3M 1M') == 0
     assert qemu_img('create', '-f', iotests.imgfmt, '-b', base_img_path,
                     '-F', iotests.imgfmt, mid_img_path) == 0
-    assert qemu_io_silent(mid_img_path,  '-c', 'write -P 3 2M 1M') == 0
-    assert qemu_io_silent(mid_img_path,  '-c', 'write -P 3 4M 1M') == 0
+    assert qemu_io_silent(mid_img_path, '-c', 'write -P 3 2M 1M') == 0
+    assert qemu_io_silent(mid_img_path, '-c', 'write -P 3 4M 1M') == 0
     assert qemu_img('create', '-f', iotests.imgfmt, '-b', mid_img_path,
                     '-F', iotests.imgfmt, top_img_path) == 0
-    assert qemu_io_silent(top_img_path,  '-c', 'write -P 2 1M 1M') == 0
+    assert qemu_io_silent(top_img_path, '-c', 'write -P 2 1M 1M') == 0
 
 #      0 1 2 3 4
 # top    2
@@ -108,10 +108,10 @@ with iotests.FilePath('base.img') as base_img_path, \
     assert qemu_img('rebase', '-u', '-b', '', '-f', iotests.imgfmt,
                     top_img_path) == 0
 
-    assert qemu_io_silent(top_img_path,  '-c', 'read -P 0 0 1M') == 0
-    assert qemu_io_silent(top_img_path,  '-c', 'read -P 2 1M 1M') == 0
-    assert qemu_io_silent(top_img_path,  '-c', 'read -P 3 2M 1M') == 0
-    assert qemu_io_silent(top_img_path,  '-c', 'read -P 0 3M 1M') == 0
-    assert qemu_io_silent(top_img_path,  '-c', 'read -P 3 4M 1M') == 0
+    assert qemu_io_silent(top_img_path, '-c', 'read -P 0 0 1M') == 0
+    assert qemu_io_silent(top_img_path, '-c', 'read -P 2 1M 1M') == 0
+    assert qemu_io_silent(top_img_path, '-c', 'read -P 3 2M 1M') == 0
+    assert qemu_io_silent(top_img_path, '-c', 'read -P 0 3M 1M') == 0
+    assert qemu_io_silent(top_img_path, '-c', 'read -P 3 4M 1M') == 0
 
     log('Done')
diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index da1bfb839e..43a4b694cc 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -37,13 +37,14 @@ def make_argparser() -> argparse.ArgumentParser:
 
     p.add_argument('-d', dest='debug', action='store_true', help='debug')
     p.add_argument('-p', dest='print', action='store_true',
-                help='redirects qemu\'s stdout and stderr to the test output')
+                   help='redirects qemu\'s stdout and stderr to '
+                        'the test output')
     p.add_argument('-gdb', action='store_true',
-                   help="start gdbserver with $GDB_OPTIONS options \
-                        ('localhost:12345' if $GDB_OPTIONS is empty)")
+                   help="start gdbserver with $GDB_OPTIONS options "
+                        "('localhost:12345' if $GDB_OPTIONS is empty)")
     p.add_argument('-valgrind', action='store_true',
-                    help='use valgrind, sets VALGRIND_QEMU environment '
-                    'variable')
+                   help='use valgrind, sets VALGRIND_QEMU environment '
+                        'variable')
 
     p.add_argument('-misalign', action='store_true',
                    help='misalign memory allocations')
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index ce06cf5630..168cc5736a 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -608,7 +608,7 @@ def _post_shutdown(self) -> None:
         super()._post_shutdown()
         if not qemu_valgrind or not self._popen:
             return
-        valgrind_filename =  f"{test_dir}/{self._popen.pid}.valgrind"
+        valgrind_filename = f"{test_dir}/{self._popen.pid}.valgrind"
         if self.exitcode() == 99:
             with open(valgrind_filename, encoding='utf-8') as f:
                 print(f.read())
@@ -1350,8 +1350,9 @@ def write(self, arg=None):
 
 class ReproducibleTestRunner(unittest.TextTestRunner):
     def __init__(self, stream: Optional[TextIO] = None,
-             resultclass: Type[unittest.TestResult] = ReproducibleTestResult,
-             **kwargs: Any) -> None:
+                 resultclass: Type[unittest.TestResult] =
+                 ReproducibleTestResult,
+                 **kwargs: Any) -> None:
         rstream = ReproducibleStreamWrapper(stream or sys.stdout)
         super().__init__(stream=rstream,           # type: ignore
                          descriptions=True,
diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index f6318492c6..8c5472f421 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -114,8 +114,8 @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm):
 
     nbd_uri = 'nbd+unix:///%s?socket=%s' % (tmp_node, nbd_sock_path)
     log(vm.qmp('nbd-server-start',
-               {'addr': { 'type': 'unix',
-                          'data': { 'path': nbd_sock_path } } }))
+               {'addr': {'type': 'unix',
+                         'data': {'path': nbd_sock_path}}}))
 
     log(vm.qmp('nbd-server-add', device=tmp_node))
 
-- 
2.27.0

[PATCH v2 2/2] qemu-iotests: fix image-fleecing pylint errors

[Emanuele Giuseppe Esposito]

The problem here is that some variables are formatted with
unnecessary spaces to make it prettier and easier to read.

However, pylint complains about those additional spaces.
Temporarly remove the check, but just on those variables.

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
 tests/qemu-iotests/tests/image-fleecing | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing
index 8c5472f421..99101fe902 100755
--- a/tests/qemu-iotests/tests/image-fleecing
+++ b/tests/qemu-iotests/tests/image-fleecing
@@ -30,6 +30,9 @@ iotests.script_initialize(
     supported_platforms=['linux'],
 )
 
+# Temporarly disable bad-whitespace pylint check to keep
+# this formatting.
+# pylint: disable=bad-whitespace
 patterns = [('0x5d', '0',         '64k'),
             ('0xd5', '1M',        '64k'),
             ('0xdc', '32M',       '64k'),
@@ -47,6 +50,7 @@ zeroes = [('0', '0x00f8000', '32k'), # Left-end of partial-left (1M-32K)
 remainder = [('0xd5', '0x108000',  '32k'), # Right-end of partial-left [1]
              ('0xdc', '32M',       '32k'), # Left-end of partial-right [2]
              ('0xcd', '0x3ff0000', '64k')] # patterns[3]
+# pylint: enable=bad-whitespace
 
 def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm):
     log('--- Setting up images ---')
-- 
2.27.0

Re: [PATCH v2 0/2] pylint: fix new errors and warnings in qemu-iotests

[Hanna Reitz]

On 08.10.21 08:28, Emanuele Giuseppe Esposito wrote:
> There are some warnings and errors that we either miss or
> are new in pylint. Anyways, test 297 of qemu-iotests fails
> because of that, so we need to fix it.
>
> All these fixes involve just indentation or additional spaces
> added.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
> v2:
> * temporarly enable and then disable "bad whitespace" error in image-fleecing
> * better indentation for a fix in test 129 in patch one

So the changes look good to me, but I can’t get my pylint to generate a 
bad-whitespace warning no matter how hard I try.  (When you asked on IRC 
whether others see pylint warnings, I thought you meant the 
consider-using-f-string warnings that John disabled in 
3765315d4c84f9c0799744f43a314169baaccc05.)

I have pylint 2.11.1, which should be the newest version.  So I tried to 
look around what might be the cause and found this: 
https://pylint.pycqa.org/en/latest/whatsnew/2.6.html – it seems that as 
of pylint 2.6, bad-whitespace warnings are no longer emitted.  If that’s 
the reason why I can’t see the warning, then I think we should take only 
patch 1 (because it just makes sense), but skip patch 2.

Hanna

Re: [PATCH v2 0/2] pylint: fix new errors and warnings in qemu-iotests

[Emanuele Giuseppe Esposito]

On 11/10/2021 11:29, Hanna Reitz wrote:
> On 08.10.21 08:28, Emanuele Giuseppe Esposito wrote:
>> There are some warnings and errors that we either miss or
>> are new in pylint. Anyways, test 297 of qemu-iotests fails
>> because of that, so we need to fix it.
>>
>> All these fixes involve just indentation or additional spaces
>> added.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> ---
>> v2:
>> * temporarly enable and then disable "bad whitespace" error in 
>> image-fleecing
>> * better indentation for a fix in test 129 in patch one
> 
> So the changes look good to me, but I can’t get my pylint to generate a 
> bad-whitespace warning no matter how hard I try.  (When you asked on IRC 
> whether others see pylint warnings, I thought you meant the 
> consider-using-f-string warnings that John disabled in 
> 3765315d4c84f9c0799744f43a314169baaccc05.)
> 
> I have pylint 2.11.1, which should be the newest version.  So I tried to 
> look around what might be the cause and found this: 
> https://pylint.pycqa.org/en/latest/whatsnew/2.6.html – it seems that as 
> of pylint 2.6, bad-whitespace warnings are no longer emitted.  If that’s 
> the reason why I can’t see the warning, then I think we should take only 
> patch 1 (because it just makes sense), but skip patch 2.
> 

Yes you are right. I had 2.4.4, and now that I upgraded to 2.11.1 I 
don't see bad-whitespace errors anymore (actually pylint does not seem 
to complain at all). So I agree we can just take patch 1, as formatting 
is wrong anyways.

Thank you,
Emanuele

Re: [PATCH v2 0/2] pylint: fix new errors and warnings in qemu-iotests

[Hanna Reitz]

On 11.10.21 11:58, Emanuele Giuseppe Esposito wrote:
>
>
> On 11/10/2021 11:29, Hanna Reitz wrote:
>> On 08.10.21 08:28, Emanuele Giuseppe Esposito wrote:
>>> There are some warnings and errors that we either miss or
>>> are new in pylint. Anyways, test 297 of qemu-iotests fails
>>> because of that, so we need to fix it.
>>>
>>> All these fixes involve just indentation or additional spaces
>>> added.
>>>
>>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>>> ---
>>> v2:
>>> * temporarly enable and then disable "bad whitespace" error in 
>>> image-fleecing
>>> * better indentation for a fix in test 129 in patch one
>>
>> So the changes look good to me, but I can’t get my pylint to generate 
>> a bad-whitespace warning no matter how hard I try. (When you asked on 
>> IRC whether others see pylint warnings, I thought you meant the 
>> consider-using-f-string warnings that John disabled in 
>> 3765315d4c84f9c0799744f43a314169baaccc05.)
>>
>> I have pylint 2.11.1, which should be the newest version.  So I tried 
>> to look around what might be the cause and found this: 
>> https://pylint.pycqa.org/en/latest/whatsnew/2.6.html – it seems that 
>> as of pylint 2.6, bad-whitespace warnings are no longer emitted.  If 
>> that’s the reason why I can’t see the warning, then I think we should 
>> take only patch 1 (because it just makes sense), but skip patch 2.
>>
>
> Yes you are right. I had 2.4.4, and now that I upgraded to 2.11.1 I 
> don't see bad-whitespace errors anymore (actually pylint does not seem 
> to complain at all). So I agree we can just take patch 1, as 
> formatting is wrong anyways.

OK, thanks!

I’ve applied patch 1 to my block branch:

https://gitlab.com/hreitz/qemu/-/commits/block/

Hanna

Re: [PATCH v2 0/2] pylint: fix new errors and warnings in qemu-iotests

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 2844 bytes --]

On Mon, Oct 11, 2021 at 5:58 AM Emanuele Giuseppe Esposito <
eesposit@redhat.com> wrote:

>
>
> On 11/10/2021 11:29, Hanna Reitz wrote:
> > On 08.10.21 08:28, Emanuele Giuseppe Esposito wrote:
> >> There are some warnings and errors that we either miss or
> >> are new in pylint. Anyways, test 297 of qemu-iotests fails
> >> because of that, so we need to fix it.
> >>
> >> All these fixes involve just indentation or additional spaces
> >> added.
> >>
> >> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> >> ---
> >> v2:
> >> * temporarly enable and then disable "bad whitespace" error in
> >> image-fleecing
> >> * better indentation for a fix in test 129 in patch one
> >
> > So the changes look good to me, but I can’t get my pylint to generate a
> > bad-whitespace warning no matter how hard I try.  (When you asked on IRC
> > whether others see pylint warnings, I thought you meant the
> > consider-using-f-string warnings that John disabled in
> > 3765315d4c84f9c0799744f43a314169baaccc05.)
> >
> > I have pylint 2.11.1, which should be the newest version.  So I tried to
> > look around what might be the cause and found this:
> > https://pylint.pycqa.org/en/latest/whatsnew/2.6.html – it seems that as
> > of pylint 2.6, bad-whitespace warnings are no longer emitted.  If that’s
> > the reason why I can’t see the warning, then I think we should take only
> > patch 1 (because it just makes sense), but skip patch 2.
> >
>
> Yes you are right. I had 2.4.4, and now that I upgraded to 2.11.1 I
> don't see bad-whitespace errors anymore (actually pylint does not seem
> to complain at all). So I agree we can just take patch 1, as formatting
> is wrong anyways.
>
>
FWIW, the minimum version of pylint supported by the python/ tests is
2.8.0, for other reasons -- see commit b4d37d81. I no longer test or check
for compatibility with older versions of pylint on any of our codebase.
I've also authored a series to add iotest 297 to the Python CI directly,
see https://lists.gnu.org/archive/html/qemu-devel/2021-10/msg04329.html --
this gives a sense of "canonically supported linter versions" to that test.

Currently, that support matrix is:

Python 3.6 to Python 3.10
mypy >= 0.770 (Known to work with current latest, 0.910)
pylint >= 2.8.0 (Known to work with current latest, 2.11.1)

The "check-python-pipenv" test is the one Python CI test that will actually
gate a build -- that test runs Python 3.6 with the oldest possible linter
versions we support to ensure we don't accidentally start using new
features.
"check-python-tox" tests all python versions, 3.6 through 3.10, with
bleeding edge packages. That CI test is "allowed to fail with a warning"
and serves to alert me to new incompatible changes in updated Python
dependencies.

--js

[-- Attachment #2: Type: text/html, Size: 3822 bytes --]