On 14.04.21 19:03, Emanuele Giuseppe Esposito wrote:
> When using -valgrind on the script tests, it generates a log file
> in $TEST_DIR that is either read (if valgrind finds problems) or
> otherwise deleted. Provide the same exact behavior when using
> -valgrind on the python tests.
>
> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
> ---
> tests/qemu-iotests/iotests.py | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 94597433fa..aef67e3a86 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -600,6 +600,26 @@ def __init__(self, path_suffix=''):
> sock_dir=sock_dir)
> self._num_drives = 0
>
> + def subprocess_check_valgrind(self, valgrind) -> None:
A type annotation would be nice. (I.e. List[str], or we make it a bool
and the caller uses bool(qemu_valgrind).)
> +
I’d drop this empty line.
> + if not valgrind:
> + return
> +
> + valgrind_filename = test_dir + "/" + str(self._popen.pid) + ".valgrind"
mypy (iotest 297) complains that _popen is Optional[], so .pid might not
exist. Perhaps this should be safeguarded in the "if not valgrind"
condition (i.e. "if not valgrind or not self._popen").
Also, pylint complains about the line being too long (79 is the maximum
length). Can be fixed with an f-string:
valgrind_filename = f"{test_dir}/{self._popen.pid}.valgrind"
> +
> + if self.exitcode() == 99:
> + with open(valgrind_filename) as f:
> + content = f.readlines()
> + for line in content:
> + print(line, end ="")
'end=""' would be better, I think. (flake8 complains.)
Also, would this be better as:
with open(valgrind_filename) as f:
for line in f.readlines():
print(line, end="")
?
(Or just
with open(valgrind_filename) as f:
print(f.read())
– wouldn’t that work, too?)
Max
> + print("")
> + else:
> + os.remove(valgrind_filename)
> +
> + def _post_shutdown(self) -> None:
> + super()._post_shutdown()
> + self.subprocess_check_valgrind(qemu_valgrind)
> +
> def add_object(self, opts):
> self._args.append('-object')
> self._args.append(opts)
>