| Message ID | 20211006130100.389521-3-eesposit@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pylint: fix new errors and warnings | expand |
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
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
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
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 >
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
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(-)