* [PATCH] qemu-iotests: fix pylint 2.8 consider-using-with error
@ 2021-05-06 8:48 Emanuele Giuseppe Esposito
2021-05-07 15:39 ` Max Reitz
0 siblings, 1 reply; 3+ messages in thread
From: Emanuele Giuseppe Esposito @ 2021-05-06 8:48 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Paolo Bonzini, Emanuele Giuseppe Esposito,
qemu-devel, Max Reitz
pylint 2.8 introduces consider-using-with error, suggesting
to use the 'with' block statement when possible.
http://pylint.pycqa.org/en/latest/whatsnew/2.8.html
Modify all subprocess.Popen calls to use the 'with' statement,
except one in __init__ of QemuIoInteractive class, since
the return value is assigned to a class field and used in other methods.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
tests/qemu-iotests/iotests.py | 63 ++++++++++++++++----------------
tests/qemu-iotests/testrunner.py | 22 +++++------
2 files changed, 42 insertions(+), 43 deletions(-)
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 5af0182895..e1117e8ae8 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -113,15 +113,14 @@ def qemu_tool_pipe_and_status(tool: str, args: Sequence[str],
Run a tool and return both its output and its exit code
"""
stderr = subprocess.STDOUT if connect_stderr else None
- subp = subprocess.Popen(args,
- stdout=subprocess.PIPE,
- stderr=stderr,
- universal_newlines=True)
- output = subp.communicate()[0]
- if subp.returncode < 0:
- cmd = ' '.join(args)
- sys.stderr.write(f'{tool} received signal {-subp.returncode}: {cmd}\n')
- return (output, subp.returncode)
+ with subprocess.Popen(args, stdout=subprocess.PIPE,
+ stderr=stderr, universal_newlines=True) as subp:
+ output = subp.communicate()[0]
+ if subp.returncode < 0:
+ cmd = ' '.join(args)
+ sys.stderr.write(f'{tool} received signal \
+ {-subp.returncode}: {cmd}\n')
+ return (output, subp.returncode)
def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
"""
@@ -237,6 +236,7 @@ def qemu_io_silent_check(*args):
class QemuIoInteractive:
def __init__(self, *args):
self.args = qemu_io_args_no_fmt + list(args)
+ # pylint: disable=consider-using-with
self._p = subprocess.Popen(self.args, stdin=subprocess.PIPE,
stdout=subprocess.PIPE,
stderr=subprocess.STDOUT,
@@ -310,22 +310,22 @@ def qemu_nbd_popen(*args):
cmd.extend(args)
log('Start NBD server')
- p = subprocess.Popen(cmd)
- try:
- while not os.path.exists(pid_file):
- if p.poll() is not None:
- raise RuntimeError(
- "qemu-nbd terminated with exit code {}: {}"
- .format(p.returncode, ' '.join(cmd)))
-
- time.sleep(0.01)
- yield
- finally:
- if os.path.exists(pid_file):
- os.remove(pid_file)
- log('Kill NBD server')
- p.kill()
- p.wait()
+ with subprocess.Popen(cmd) as p:
+ try:
+ while not os.path.exists(pid_file):
+ if p.poll() is not None:
+ raise RuntimeError(
+ "qemu-nbd terminated with exit code {}: {}"
+ .format(p.returncode, ' '.join(cmd)))
+
+ time.sleep(0.01)
+ yield
+ finally:
+ if os.path.exists(pid_file):
+ os.remove(pid_file)
+ log('Kill NBD server')
+ p.kill()
+ p.wait()
def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
'''Return True if two image files are identical'''
@@ -334,13 +334,12 @@ def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
def create_image(name, size):
'''Create a fully-allocated raw image with sector markers'''
- file = open(name, 'wb')
- i = 0
- while i < size:
- sector = struct.pack('>l504xl', i // 512, i // 512)
- file.write(sector)
- i = i + 512
- file.close()
+ with open(name, 'wb') as file:
+ i = 0
+ while i < size:
+ sector = struct.pack('>l504xl', i // 512, i // 512)
+ file.write(sector)
+ i = i + 512
def image_size(img):
'''Return image's virtual size'''
diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
index 1fc61fcaa3..729fe9ee3b 100644
--- a/tests/qemu-iotests/testrunner.py
+++ b/tests/qemu-iotests/testrunner.py
@@ -258,17 +258,17 @@ def do_run_test(self, test: str) -> TestResult:
t0 = time.time()
with f_bad.open('w', encoding="utf-8") as f:
- proc = subprocess.Popen(args, cwd=str(f_test.parent), env=env,
- stdout=f, stderr=subprocess.STDOUT)
- try:
- proc.wait()
- except KeyboardInterrupt:
- proc.terminate()
- proc.wait()
- return TestResult(status='not run',
- description='Interrupted by user',
- interrupted=True)
- ret = proc.returncode
+ with subprocess.Popen(args, cwd=str(f_test.parent), env=env,
+ stdout=f, stderr=subprocess.STDOUT) as proc:
+ try:
+ proc.wait()
+ except KeyboardInterrupt:
+ proc.terminate()
+ proc.wait()
+ return TestResult(status='not run',
+ description='Interrupted by user',
+ interrupted=True)
+ ret = proc.returncode
elapsed = round(time.time() - t0, 1)
--
2.30.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] qemu-iotests: fix pylint 2.8 consider-using-with error
2021-05-06 8:48 [PATCH] qemu-iotests: fix pylint 2.8 consider-using-with error Emanuele Giuseppe Esposito
@ 2021-05-07 15:39 ` Max Reitz
2021-05-10 8:36 ` Emanuele Giuseppe Esposito
0 siblings, 1 reply; 3+ messages in thread
From: Max Reitz @ 2021-05-07 15:39 UTC (permalink / raw)
To: Emanuele Giuseppe Esposito, qemu-block
Cc: Kevin Wolf, Paolo Bonzini, qemu-devel
On 06.05.21 10:48, Emanuele Giuseppe Esposito wrote:
> pylint 2.8 introduces consider-using-with error, suggesting
> to use the 'with' block statement when possible.
> http://pylint.pycqa.org/en/latest/whatsnew/2.8.html
>
> Modify all subprocess.Popen calls to use the 'with' statement,
> except one in __init__ of QemuIoInteractive class, since
> the return value is assigned to a class field and used in other methods.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
> tests/qemu-iotests/iotests.py | 63 ++++++++++++++++----------------
> tests/qemu-iotests/testrunner.py | 22 +++++------
> 2 files changed, 42 insertions(+), 43 deletions(-)
Thanks, looks good, functionally. But I just can’t keep myself from
nagging about indentation (I wouldn’t have, but flake8 says I may be
justified):
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 5af0182895..e1117e8ae8 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -113,15 +113,14 @@ def qemu_tool_pipe_and_status(tool: str, args: Sequence[str],
> Run a tool and return both its output and its exit code
> """
> stderr = subprocess.STDOUT if connect_stderr else None
> - subp = subprocess.Popen(args,
> - stdout=subprocess.PIPE,
> - stderr=stderr,
> - universal_newlines=True)
> - output = subp.communicate()[0]
> - if subp.returncode < 0:
> - cmd = ' '.join(args)
> - sys.stderr.write(f'{tool} received signal {-subp.returncode}: {cmd}\n')
> - return (output, subp.returncode)
> + with subprocess.Popen(args, stdout=subprocess.PIPE,
> + stderr=stderr, universal_newlines=True) as subp:
The second line’s indentation is not aligned to the opening parenthesis
of the Popen call. I’d like it better if it were.
> + output = subp.communicate()[0]
> + if subp.returncode < 0:
> + cmd = ' '.join(args)
> + sys.stderr.write(f'{tool} received signal \
> + {-subp.returncode}: {cmd}\n')
> + return (output, subp.returncode)
>
> def qemu_img_pipe_and_status(*args: str) -> Tuple[str, int]:
> """
> @@ -237,6 +236,7 @@ def qemu_io_silent_check(*args):
> class QemuIoInteractive:
> def __init__(self, *args):
> self.args = qemu_io_args_no_fmt + list(args)
> + # pylint: disable=consider-using-with
I think I would have added an additional comment why we need this
statement (because we need to keep the Popen object around), but then
again, I suppose it really is obvious.
(Wouldn’t have commented on this bit if I hadn’t replied because of the
indentation. So if you think it’s obvious, no need to add a comment.)
> self._p = subprocess.Popen(self.args, stdin=subprocess.PIPE,
> stdout=subprocess.PIPE,
> stderr=subprocess.STDOUT,
[...]
> diff --git a/tests/qemu-iotests/testrunner.py b/tests/qemu-iotests/testrunner.py
> index 1fc61fcaa3..729fe9ee3b 100644
> --- a/tests/qemu-iotests/testrunner.py
> +++ b/tests/qemu-iotests/testrunner.py
> @@ -258,17 +258,17 @@ def do_run_test(self, test: str) -> TestResult:
>
> t0 = time.time()
> with f_bad.open('w', encoding="utf-8") as f:
> - proc = subprocess.Popen(args, cwd=str(f_test.parent), env=env,
> - stdout=f, stderr=subprocess.STDOUT)
> - try:
> - proc.wait()
> - except KeyboardInterrupt:
> - proc.terminate()
> - proc.wait()
> - return TestResult(status='not run',
> - description='Interrupted by user',
> - interrupted=True)
> - ret = proc.returncode
> + with subprocess.Popen(args, cwd=str(f_test.parent), env=env,
> + stdout=f, stderr=subprocess.STDOUT) as proc:
As above, the second line is (no longer) aligned to the opening parenthesis.
> + try:
> + proc.wait()
> + except KeyboardInterrupt:
> + proc.terminate()
> + proc.wait()
> + return TestResult(status='not run',
> + description='Interrupted by user',
> + interrupted=True)
And here, too.
Max
> + ret = proc.returncode
>
> elapsed = round(time.time() - t0, 1)
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] qemu-iotests: fix pylint 2.8 consider-using-with error
2021-05-07 15:39 ` Max Reitz
@ 2021-05-10 8:36 ` Emanuele Giuseppe Esposito
0 siblings, 0 replies; 3+ messages in thread
From: Emanuele Giuseppe Esposito @ 2021-05-10 8:36 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: Kevin Wolf, Paolo Bonzini, qemu-devel
On 07/05/2021 17:39, Max Reitz wrote:
> On 06.05.21 10:48, Emanuele Giuseppe Esposito wrote:
>> pylint 2.8 introduces consider-using-with error, suggesting
>> to use the 'with' block statement when possible.
>> http://pylint.pycqa.org/en/latest/whatsnew/2.8.html
>>
>> Modify all subprocess.Popen calls to use the 'with' statement,
>> except one in __init__ of QemuIoInteractive class, since
>> the return value is assigned to a class field and used in other methods.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> ---
>> tests/qemu-iotests/iotests.py | 63 ++++++++++++++++----------------
>> tests/qemu-iotests/testrunner.py | 22 +++++------
>> 2 files changed, 42 insertions(+), 43 deletions(-)
>
> Thanks, looks good, functionally. But I just can’t keep myself from
> nagging about indentation (I wouldn’t have, but flake8 says I may be
> justified):
No problem , I expected that there would be style problems :)
I will fix the indentation and send v2.
Thank you,
Emanuele
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-05-10 8:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-06 8:48 [PATCH] qemu-iotests: fix pylint 2.8 consider-using-with error Emanuele Giuseppe Esposito
2021-05-07 15:39 ` Max Reitz
2021-05-10 8:36 ` Emanuele Giuseppe Esposito
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).