* [PATCH 0/2] pylint: fix new errors and warnings @ 2021-10-06 13:00 Emanuele Giuseppe Esposito 2021-10-06 13:00 ` [PATCH 1/2] pylint: fix errors and warnings from qemu-tests test 297 Emanuele Giuseppe Esposito 2021-10-06 13:01 ` [PATCH 2/2] qemu-iotests: fix image-fleecing pylint errors Emanuele Giuseppe Esposito 0 siblings, 2 replies; 12+ messages in thread From: Emanuele Giuseppe Esposito @ 2021-10-06 13:00 UTC (permalink / raw) To: qemu-block Cc: Kevin Wolf, Emanuele Giuseppe Esposito, Hanna Reitz, qemu-devel 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. Emanuele Giuseppe Esposito (2): pylint: fix errors and warnings from qemu-tests test 297 qemu-iotests: fix image-fleecing pylint errors tests/qemu-iotests/129 | 9 ++--- tests/qemu-iotests/310 | 16 ++++----- tests/qemu-iotests/check | 11 +++--- tests/qemu-iotests/iotests.py | 7 ++-- tests/qemu-iotests/tests/image-fleecing | 47 +++++++++++++++---------- 5 files changed, 51 insertions(+), 39 deletions(-) -- 2.27.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/2] pylint: fix errors and warnings from qemu-tests test 297 2021-10-06 13:00 [PATCH 0/2] pylint: fix new errors and warnings Emanuele Giuseppe Esposito @ 2021-10-06 13:00 ` Emanuele Giuseppe Esposito 2021-10-06 16:46 ` Kevin Wolf 2021-10-06 13:01 ` [PATCH 2/2] qemu-iotests: fix image-fleecing pylint errors Emanuele Giuseppe Esposito 1 sibling, 1 reply; 12+ messages in thread From: Emanuele Giuseppe Esposito @ 2021-10-06 13:00 UTC (permalink / raw) To: qemu-block Cc: Kevin Wolf, Emanuele Giuseppe Esposito, Hanna Reitz, qemu-devel Test 297 in qemu-iotests folder 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 | 9 +++++---- 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, 25 insertions(+), 22 deletions(-) diff --git a/tests/qemu-iotests/129 b/tests/qemu-iotests/129 index 5251e2669e..21df239597 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 @@ -87,13 +87,14 @@ class TestStopWithBlockJob(iotests.QMPTestCase): iotests.qemu_img('create', '-f', iotests.imgfmt, self.overlay_img, '1G') - result = self.vm.qmp('blockdev-add', **{ + result = self.vm.qmp('blockdev-add', + **{ 'node-name': 'overlay', 'driver': iotests.imgfmt, 'file': { 'driver': 'file', 'filename': self.overlay_img - } + } }) self.assert_qmp(result, 'return', {}) 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pylint: fix errors and warnings from qemu-tests test 297 2021-10-06 13:00 ` [PATCH 1/2] pylint: fix errors and warnings from qemu-tests test 297 Emanuele Giuseppe Esposito @ 2021-10-06 16:46 ` Kevin Wolf 2021-10-07 7:51 ` Emanuele Giuseppe Esposito 0 siblings, 1 reply; 12+ messages in thread From: Kevin Wolf @ 2021-10-06 16:46 UTC (permalink / raw) To: Emanuele Giuseppe Esposito; +Cc: Hanna Reitz, qemu-devel, qemu-block Am 06.10.2021 um 15:00 hat Emanuele Giuseppe Esposito geschrieben: > Test 297 in qemu-iotests folder 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> > @@ -87,13 +87,14 @@ class TestStopWithBlockJob(iotests.QMPTestCase): > iotests.qemu_img('create', '-f', iotests.imgfmt, self.overlay_img, > '1G') > > - result = self.vm.qmp('blockdev-add', **{ > + result = self.vm.qmp('blockdev-add', > + **{ > 'node-name': 'overlay', > 'driver': iotests.imgfmt, > 'file': { > 'driver': 'file', > 'filename': self.overlay_img > - } > + } > }) > self.assert_qmp(result, 'return', {}) Am I the only one to think that the new indentation for the closing brace there is horrible? PEP-8 explictly allows things like: my_list = [ 1, 2, 3, 4, 5, 6, ] Some of the other changes in this patch should be made, but at least if these are behind different switches, I would consider just disabling the one that complains about nicely formatted dicts. Kevin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pylint: fix errors and warnings from qemu-tests test 297 2021-10-06 16:46 ` Kevin Wolf @ 2021-10-07 7:51 ` Emanuele Giuseppe Esposito 2021-10-07 8:45 ` Kevin Wolf 0 siblings, 1 reply; 12+ messages in thread From: Emanuele Giuseppe Esposito @ 2021-10-07 7:51 UTC (permalink / raw) To: Kevin Wolf; +Cc: Hanna Reitz, qemu-devel, qemu-block On 06/10/2021 18:46, Kevin Wolf wrote: > Am 06.10.2021 um 15:00 hat Emanuele Giuseppe Esposito geschrieben: >> Test 297 in qemu-iotests folder 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> > >> @@ -87,13 +87,14 @@ class TestStopWithBlockJob(iotests.QMPTestCase): >> iotests.qemu_img('create', '-f', iotests.imgfmt, self.overlay_img, >> '1G') >> >> - result = self.vm.qmp('blockdev-add', **{ >> + result = self.vm.qmp('blockdev-add', >> + **{ >> 'node-name': 'overlay', >> 'driver': iotests.imgfmt, >> 'file': { >> 'driver': 'file', >> 'filename': self.overlay_img >> - } >> + } >> }) >> self.assert_qmp(result, 'return', {}) > > Am I the only one to think that the new indentation for the closing > brace there is horrible? PEP-8 explictly allows things like: > > my_list = [ > 1, 2, 3, > 4, 5, 6, > ] > > Some of the other changes in this patch should be made, but at least if > these are behind different switches, I would consider just disabling the > one that complains about nicely formatted dicts. The error is "C0330: Wrong hanging indentation" so it is not about dicts. I guess we can disable the error, but the problem is that we will disable it for the whole file, which doesn't seem right. Alternatively, this also works fine: - result = self.vm.qmp('blockdev-add', - **{ - 'node-name': 'overlay', - 'driver': iotests.imgfmt, - 'file': { - 'driver': 'file', - 'filename': self.overlay_img - } - }) + result = self.vm.qmp('blockdev-add', **{ + 'node-name': 'overlay', + 'driver': iotests.imgfmt, + 'file': { + 'driver': 'file', + 'filename': self.overlay_img + }}) What do you think? Otherwise I am happy to disable the error altogether. Emanuele ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pylint: fix errors and warnings from qemu-tests test 297 2021-10-07 7:51 ` Emanuele Giuseppe Esposito @ 2021-10-07 8:45 ` Kevin Wolf 2021-10-07 10:34 ` Emanuele Giuseppe Esposito 0 siblings, 1 reply; 12+ messages in thread From: Kevin Wolf @ 2021-10-07 8:45 UTC (permalink / raw) To: Emanuele Giuseppe Esposito; +Cc: Hanna Reitz, qemu-devel, qemu-block Am 07.10.2021 um 09:51 hat Emanuele Giuseppe Esposito geschrieben: > > > On 06/10/2021 18:46, Kevin Wolf wrote: > > Am 06.10.2021 um 15:00 hat Emanuele Giuseppe Esposito geschrieben: > > > Test 297 in qemu-iotests folder 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> > > > > > @@ -87,13 +87,14 @@ class TestStopWithBlockJob(iotests.QMPTestCase): > > > iotests.qemu_img('create', '-f', iotests.imgfmt, self.overlay_img, > > > '1G') > > > - result = self.vm.qmp('blockdev-add', **{ > > > + result = self.vm.qmp('blockdev-add', > > > + **{ > > > 'node-name': 'overlay', > > > 'driver': iotests.imgfmt, > > > 'file': { > > > 'driver': 'file', > > > 'filename': self.overlay_img > > > - } > > > + } > > > }) > > > self.assert_qmp(result, 'return', {}) > > > > Am I the only one to think that the new indentation for the closing > > brace there is horrible? PEP-8 explictly allows things like: > > > > my_list = [ > > 1, 2, 3, > > 4, 5, 6, > > ] > > > > Some of the other changes in this patch should be made, but at least if > > these are behind different switches, I would consider just disabling the > > one that complains about nicely formatted dicts. > > The error is "C0330: Wrong hanging indentation" > so it is not about dicts. I guess we can disable the error, but the problem > is that we will disable it for the whole file, which doesn't seem right. Actually, I would disable it globally in pylintrc because building dictionaries for JSON is something that we do a lot. But then I'm surprised that this is the only instance that actually fails. I wonder what the difference is. For example, 129 doesn't seem to be skipped and has this code: result = self.vm.qmp('blockdev-add', **{ 'node-name': 'overlay', 'driver': iotests.imgfmt, 'file': { 'driver': 'file', 'filename': self.overlay_img } }) Yet you don't report a pylint error for this file. Oooh... I think I do see a difference: The final line is indented by one space more in the case that fails for you. It should be vertically aligned with the "'" in the first line, but it is actually aligned with the "b" of "blockdev-add". Does removing one space of indentation in the last line fix the report? Kevin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pylint: fix errors and warnings from qemu-tests test 297 2021-10-07 8:45 ` Kevin Wolf @ 2021-10-07 10:34 ` Emanuele Giuseppe Esposito 2021-10-07 16:25 ` Kevin Wolf 0 siblings, 1 reply; 12+ messages in thread From: Emanuele Giuseppe Esposito @ 2021-10-07 10:34 UTC (permalink / raw) To: Kevin Wolf; +Cc: Hanna Reitz, qemu-devel, qemu-block >> The error is "C0330: Wrong hanging indentation" >> so it is not about dicts. I guess we can disable the error, but the problem >> is that we will disable it for the whole file, which doesn't seem right. > > Actually, I would disable it globally in pylintrc because building > dictionaries for JSON is something that we do a lot. > > But then I'm surprised that this is the only instance that actually > fails. I wonder what the difference is. > > For example, 129 doesn't seem to be skipped and has this code: > > result = self.vm.qmp('blockdev-add', **{ > 'node-name': 'overlay', > 'driver': iotests.imgfmt, > 'file': { > 'driver': 'file', > 'filename': self.overlay_img > } > }) > > Yet you don't report a pylint error for this file. Well, unless I am misunderstanding something... 129 *is* the file I am reporting. And that is exactly the function where pylint complains. > > Oooh... I think I do see a difference: The final line is indented by one > space more in the case that fails for you. It should be vertically > aligned with the "'" in the first line, but it is actually aligned with > the "b" of "blockdev-add" > > Does removing one space of indentation in the last line fix the report? It's not only the final line, it's from "**{" till the ending ")". 'node-name' is under "ock" of 'blockdev-add'. It is clearly bad indented, regardless of the new style and pylint new rules. Pylint itself suggests to move it 4 spaces more than "result =", ie 21 spaces before. Still, applying your suggestion to all the lines and removing 1 space from all lines still does not make pylint happy, as it asks to remove 20 spaces. To simplify things, this is the error I get: === pylint === +************* Module 129 +129:91:0: C0330: Wrong hanging indentation (remove 21 spaces). + 'node-name': 'overlay', + | ^ (bad-continuation) +129:92:0: C0330: Wrong hanging indentation (remove 21 spaces). + 'driver': iotests.imgfmt, + | ^ (bad-continuation) +129:93:0: C0330: Wrong hanging indentation (remove 21 spaces). + 'file': { + | ^ (bad-continuation) +129:97:0: C0330: Wrong hanging indentation. + }) + | | ^ (bad-continuation) So unless you want to disable it overall, one way of fixing 129 is to follow what pylint suggests, and do like I wrote in the previous email: Either: result = self.vm.qmp('blockdev-add', **{ 'node-name': 'overlay', <-- 21 spaces less 'driver': iotests.imgfmt, <-- 21 spaces less 'file': { <-- 21 spaces less 'driver': 'file', <-- 21 spaces less 'filename': self.overlay_img <-- 21 spaces less } <-- 21 spaces less }) <-- 21 spaces less or (difference is in the last line only): result = self.vm.qmp('blockdev-add', **{ 'node-name': 'overlay', <-- 21 spaces less 'driver': iotests.imgfmt, <-- 21 spaces less 'file': { <-- 21 spaces less 'driver': 'file', <-- 21 spaces less 'filename': self.overlay_img <-- 21 spaces less }}) <-- 21 spaces less Emanuele ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/2] pylint: fix errors and warnings from qemu-tests test 297 2021-10-07 10:34 ` Emanuele Giuseppe Esposito @ 2021-10-07 16:25 ` Kevin Wolf 0 siblings, 0 replies; 12+ messages in thread From: Kevin Wolf @ 2021-10-07 16:25 UTC (permalink / raw) To: Emanuele Giuseppe Esposito; +Cc: Hanna Reitz, qemu-devel, qemu-block Am 07.10.2021 um 12:34 hat Emanuele Giuseppe Esposito geschrieben: > > > > The error is "C0330: Wrong hanging indentation" > > > so it is not about dicts. I guess we can disable the error, but the problem > > > is that we will disable it for the whole file, which doesn't seem right. > > > > Actually, I would disable it globally in pylintrc because building > > dictionaries for JSON is something that we do a lot. > > > > But then I'm surprised that this is the only instance that actually > > fails. I wonder what the difference is. > > > > For example, 129 doesn't seem to be skipped and has this code: > > > > > > result = self.vm.qmp('blockdev-add', **{ > > 'node-name': 'overlay', > > 'driver': iotests.imgfmt, > > 'file': { > > 'driver': 'file', > > 'filename': self.overlay_img > > } > > }) > > > > Yet you don't report a pylint error for this file. > > Well, unless I am misunderstanding something... 129 *is* the file I am > reporting. And that is exactly the function where pylint complains. Indeed, my bad. I got confused there. And the other files that do something similar are all in SKIP_FILES in 297. So it looks like we don't have another case to copy. > > > > Oooh... I think I do see a difference: The final line is indented by one > > space more in the case that fails for you. It should be vertically > > aligned with the "'" in the first line, but it is actually aligned with > > the "b" of "blockdev-add" > > > > Does removing one space of indentation in the last line fix the report? > > It's not only the final line, it's from "**{" till the ending ")". > 'node-name' is under "ock" of 'blockdev-add'. It is clearly bad indented, > regardless of the new style and pylint new rules. > > Pylint itself suggests to move it 4 spaces more than "result =", ie 21 > spaces before. > > Still, applying your suggestion to all the lines and removing 1 space from > all lines still does not make pylint happy, as it asks to remove 20 spaces. > > To simplify things, this is the error I get: > > === pylint === > +************* Module 129 > +129:91:0: C0330: Wrong hanging indentation (remove 21 spaces). > + 'node-name': 'overlay', > + | ^ (bad-continuation) > +129:92:0: C0330: Wrong hanging indentation (remove 21 spaces). > + 'driver': iotests.imgfmt, > + | ^ (bad-continuation) > +129:93:0: C0330: Wrong hanging indentation (remove 21 spaces). > + 'file': { > + | ^ (bad-continuation) > +129:97:0: C0330: Wrong hanging indentation. > + }) > + | | ^ (bad-continuation) > > So unless you want to disable it overall, one way of fixing 129 is to follow > what pylint suggests, and do like I wrote in the previous email: > > Either: > result = self.vm.qmp('blockdev-add', **{ > 'node-name': 'overlay', <-- 21 spaces less > 'driver': iotests.imgfmt, <-- 21 spaces less > 'file': { <-- 21 spaces less > 'driver': 'file', <-- 21 spaces less > 'filename': self.overlay_img <-- 21 spaces less > } <-- 21 spaces less > }) <-- 21 spaces less Yes, this looks reasonble enough. Kevin ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/2] qemu-iotests: fix image-fleecing pylint errors 2021-10-06 13:00 [PATCH 0/2] pylint: fix new errors and warnings Emanuele Giuseppe Esposito 2021-10-06 13:00 ` [PATCH 1/2] pylint: fix errors and warnings from qemu-tests test 297 Emanuele Giuseppe Esposito @ 2021-10-06 13:01 ` Emanuele Giuseppe Esposito 2021-10-06 16:51 ` Kevin Wolf 1 sibling, 1 reply; 12+ messages in thread From: Emanuele Giuseppe Esposito @ 2021-10-06 13:01 UTC (permalink / raw) To: qemu-block Cc: Kevin Wolf, Emanuele Giuseppe Esposito, Hanna Reitz, qemu-devel 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. A solution is to transform them as string with arbitrary spaces, and then convert it back into a tuple. Removing the spaces makes it a little bit ugly, and directly using the string forces us to change the test reference output accordingly, which will 1) contain ugly weird formatted strings, 2) is not portable if somebody changes the formatting in the test string. Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> --- tests/qemu-iotests/tests/image-fleecing | 43 +++++++++++++++---------- 1 file changed, 26 insertions(+), 17 deletions(-) diff --git a/tests/qemu-iotests/tests/image-fleecing b/tests/qemu-iotests/tests/image-fleecing index 8c5472f421..9709ecbba1 100755 --- a/tests/qemu-iotests/tests/image-fleecing +++ b/tests/qemu-iotests/tests/image-fleecing @@ -30,23 +30,27 @@ iotests.script_initialize( supported_platforms=['linux'], ) -patterns = [('0x5d', '0', '64k'), - ('0xd5', '1M', '64k'), - ('0xdc', '32M', '64k'), - ('0xcd', '0x3ff0000', '64k')] # 64M - 64K - -overwrite = [('0xab', '0', '64k'), # Full overwrite - ('0xad', '0x00f8000', '64k'), # Partial-left (1M-32K) - ('0x1d', '0x2008000', '64k'), # Partial-right (32M+32K) - ('0xea', '0x3fe0000', '64k')] # Adjacent-left (64M - 128K) - -zeroes = [('0', '0x00f8000', '32k'), # Left-end of partial-left (1M-32K) - ('0', '0x2010000', '32k'), # Right-end of partial-right (32M+64K) - ('0', '0x3fe0000', '64k')] # overwrite[3] - -remainder = [('0xd5', '0x108000', '32k'), # Right-end of partial-left [1] - ('0xdc', '32M', '32k'), # Left-end of partial-right [2] - ('0xcd', '0x3ff0000', '64k')] # patterns[3] +# Each string in patterns, overwrite and remainder is formatted in such way +# for readability. Using a 3-tuple will make pylint trigger +# "bad-whitespace" when we try to format it in the same way +# The testing code will take care of splitting it properly. +patterns = ['0x5d 0 64k', + '0xd5 1M 64k', + '0xdc 32M 64k', + '0xcd 0x3ff0000 64k'] # 64M - 64K + +overwrite = ['0xab 0 64k', # Full overwrite + '0xad 0x00f8000 64k', # Partial-left (1M-32K) + '0x1d 0x2008000 64k', # Partial-right (32M+32K) + '0xea 0x3fe0000 64k'] # Adjacent-left (64M - 128K) + +zeroes = ['0 0x00f8000 32k', # Left-end of partial-left (1M-32K) + '0 0x2010000 32k', # Right-end of partial-right (32M+64K) + '0 0x3fe0000 64k'] # overwrite[3] + +remainder = ['0xd5 0x108000 32k', # Right-end of partial-left [1] + '0xdc 32M 32k', # Left-end of partial-right [2] + '0xcd 0x3ff0000 64k'] # patterns[3] def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): log('--- Setting up images ---') @@ -56,6 +60,7 @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): assert qemu_img('create', '-f', 'qcow2', fleece_img_path, '64M') == 0 for p in patterns: + p = tuple(p.split()) qemu_io('-f', iotests.imgfmt, '-c', 'write -P%s %s %s' % p, base_img_path) @@ -124,6 +129,7 @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): log('') for p in patterns + zeroes: + p = tuple(p.split()) cmd = 'read -P%s %s %s' % p log(cmd) assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0 @@ -133,6 +139,7 @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): log('') for p in overwrite: + p = tuple(p.split()) cmd = 'write -P%s %s %s' % p log(cmd) log(vm.hmp_qemu_io(qom_path, cmd, qdev=True)) @@ -142,6 +149,7 @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): log('') for p in patterns + zeroes: + p = tuple(p.split()) cmd = 'read -P%s %s %s' % p log(cmd) assert qemu_io_silent('-r', '-f', 'raw', '-c', cmd, nbd_uri) == 0 @@ -168,6 +176,7 @@ def do_test(use_cbw, base_img_path, fleece_img_path, nbd_sock_path, vm): log('') for p in overwrite + remainder: + p = tuple(p.split()) cmd = 'read -P%s %s %s' % p log(cmd) assert qemu_io_silent(base_img_path, '-c', cmd) == 0 -- 2.27.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] qemu-iotests: fix image-fleecing pylint errors 2021-10-06 13:01 ` [PATCH 2/2] qemu-iotests: fix image-fleecing pylint errors Emanuele Giuseppe Esposito @ 2021-10-06 16:51 ` Kevin Wolf 2021-10-07 7:53 ` Emanuele Giuseppe Esposito 0 siblings, 1 reply; 12+ messages in thread From: Kevin Wolf @ 2021-10-06 16:51 UTC (permalink / raw) To: Emanuele Giuseppe Esposito; +Cc: Hanna Reitz, qemu-devel, qemu-block Am 06.10.2021 um 15:01 hat Emanuele Giuseppe Esposito geschrieben: > 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. > A solution is to transform them as string with arbitrary spaces, > and then convert it back into a tuple. > > Removing the spaces makes it a little bit ugly, and directly > using the string forces us to change the test reference output > accordingly, which will 1) contain ugly weird formatted strings, > 2) is not portable if somebody changes the formatting in the test > string. > > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> Changing our logic because of a style checker feels wrong. I'd rather stick in a line like this before the definitions: # pylint: disable=bad-whitespace (Not sure if the syntax of this is entirely correct, but from the comment in your patch and existing uses in iotests, I think this would be the line.) Kevin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] qemu-iotests: fix image-fleecing pylint errors 2021-10-06 16:51 ` Kevin Wolf @ 2021-10-07 7:53 ` Emanuele Giuseppe Esposito 2021-10-07 8:36 ` Kevin Wolf 0 siblings, 1 reply; 12+ messages in thread From: Emanuele Giuseppe Esposito @ 2021-10-07 7:53 UTC (permalink / raw) To: Kevin Wolf; +Cc: Hanna Reitz, qemu-devel, qemu-block On 06/10/2021 18:51, Kevin Wolf wrote: > Am 06.10.2021 um 15:01 hat Emanuele Giuseppe Esposito geschrieben: >> 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. >> A solution is to transform them as string with arbitrary spaces, >> and then convert it back into a tuple. >> >> Removing the spaces makes it a little bit ugly, and directly >> using the string forces us to change the test reference output >> accordingly, which will 1) contain ugly weird formatted strings, >> 2) is not portable if somebody changes the formatting in the test >> string. >> >> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> > > Changing our logic because of a style checker feels wrong. I'd rather > stick in a line like this before the definitions: > > # pylint: disable=bad-whitespace > > (Not sure if the syntax of this is entirely correct, but from the > comment in your patch and existing uses in iotests, I think this would > be the line.) Ok, I will add the line. Same remarks from the previous patch applies: unfortunately then we disable the warning for the whole file. Since here (like the previous patch) the error spans on multiple lines, adding a # pylint: disable= comment on each line is infeasible and ugly. Emanuele ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] qemu-iotests: fix image-fleecing pylint errors 2021-10-07 7:53 ` Emanuele Giuseppe Esposito @ 2021-10-07 8:36 ` Kevin Wolf 2021-10-07 10:35 ` Emanuele Giuseppe Esposito 0 siblings, 1 reply; 12+ messages in thread From: Kevin Wolf @ 2021-10-07 8:36 UTC (permalink / raw) To: Emanuele Giuseppe Esposito; +Cc: Hanna Reitz, qemu-devel, qemu-block Am 07.10.2021 um 09:53 hat Emanuele Giuseppe Esposito geschrieben: > > > On 06/10/2021 18:51, Kevin Wolf wrote: > > Am 06.10.2021 um 15:01 hat Emanuele Giuseppe Esposito geschrieben: > > > 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. > > > A solution is to transform them as string with arbitrary spaces, > > > and then convert it back into a tuple. > > > > > > Removing the spaces makes it a little bit ugly, and directly > > > using the string forces us to change the test reference output > > > accordingly, which will 1) contain ugly weird formatted strings, > > > 2) is not portable if somebody changes the formatting in the test > > > string. > > > > > > Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> > > > > Changing our logic because of a style checker feels wrong. I'd rather > > stick in a line like this before the definitions: > > > > # pylint: disable=bad-whitespace > > > > (Not sure if the syntax of this is entirely correct, but from the > > comment in your patch and existing uses in iotests, I think this would > > be the line.) > > Ok, I will add the line. Same remarks from the previous patch applies: > unfortunately then we disable the warning for the whole file. > > Since here (like the previous patch) the error spans on multiple lines, > adding a # pylint: disable= comment on each line is infeasible and ugly. It doesn't fail with my pylint version, so I can't try it out, but does the following work? # pylint: disable=bad-whitespace ... definitions ... # pylint: enable=bad-whitespace Kevin ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/2] qemu-iotests: fix image-fleecing pylint errors 2021-10-07 8:36 ` Kevin Wolf @ 2021-10-07 10:35 ` Emanuele Giuseppe Esposito 0 siblings, 0 replies; 12+ messages in thread From: Emanuele Giuseppe Esposito @ 2021-10-07 10:35 UTC (permalink / raw) To: Kevin Wolf; +Cc: Hanna Reitz, qemu-devel, qemu-block On 07/10/2021 10:36, Kevin Wolf wrote: > Am 07.10.2021 um 09:53 hat Emanuele Giuseppe Esposito geschrieben: >> >> >> On 06/10/2021 18:51, Kevin Wolf wrote: >>> Am 06.10.2021 um 15:01 hat Emanuele Giuseppe Esposito geschrieben: >>>> 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. >>>> A solution is to transform them as string with arbitrary spaces, >>>> and then convert it back into a tuple. >>>> >>>> Removing the spaces makes it a little bit ugly, and directly >>>> using the string forces us to change the test reference output >>>> accordingly, which will 1) contain ugly weird formatted strings, >>>> 2) is not portable if somebody changes the formatting in the test >>>> string. >>>> >>>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com> >>> >>> Changing our logic because of a style checker feels wrong. I'd rather >>> stick in a line like this before the definitions: >>> >>> # pylint: disable=bad-whitespace >>> >>> (Not sure if the syntax of this is entirely correct, but from the >>> comment in your patch and existing uses in iotests, I think this would >>> be the line.) >> >> Ok, I will add the line. Same remarks from the previous patch applies: >> unfortunately then we disable the warning for the whole file. >> >> Since here (like the previous patch) the error spans on multiple lines, >> adding a # pylint: disable= comment on each line is infeasible and ugly. > > It doesn't fail with my pylint version, so I can't try it out, but does > the following work? > > # pylint: disable=bad-whitespace > ... definitions ... > # pylint: enable=bad-whitespace You are right, this is valid and looks very good. In this way I can just temporarily disable the error for the variables. > > Kevin > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-10-07 17:06 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-06 13:00 [PATCH 0/2] pylint: fix new errors and warnings Emanuele Giuseppe Esposito 2021-10-06 13:00 ` [PATCH 1/2] pylint: fix errors and warnings from qemu-tests test 297 Emanuele Giuseppe Esposito 2021-10-06 16:46 ` Kevin Wolf 2021-10-07 7:51 ` Emanuele Giuseppe Esposito 2021-10-07 8:45 ` Kevin Wolf 2021-10-07 10:34 ` Emanuele Giuseppe Esposito 2021-10-07 16:25 ` Kevin Wolf 2021-10-06 13:01 ` [PATCH 2/2] qemu-iotests: fix image-fleecing pylint errors Emanuele Giuseppe Esposito 2021-10-06 16:51 ` Kevin Wolf 2021-10-07 7:53 ` Emanuele Giuseppe Esposito 2021-10-07 8:36 ` Kevin Wolf 2021-10-07 10:35 ` Emanuele Giuseppe Esposito
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.