Re: [PATCH] qemu-iotests: fix pylint 2.8 consider-using-with error
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 --- 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
Re: [PATCH] qemu-iotests: fix pylint 2.8 consider-using-with error
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
---
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)
[PATCH] qemu-iotests: fix pylint 2.8 consider-using-with error
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
---
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
+
