[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in
From: |
Cleber Rosa |
Subject: |
Re: [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py |
Date: |
Mon, 10 Jun 2019 19:24:09 -0400 |
User-agent: |
Mutt/1.11.4 (2019-03-13) |
On Mon, Jun 10, 2019 at 09:49:10PM +0200, Aleksandar Markovic wrote:
> From: Aleksandar Markovic <address@hidden>
>
> Rather than optputing a cryptic message:
>
> FAIL: True not found in [False],
>
> the following will be reported too, if the command output does not meet
> specified expectations:
>
> 'lspci -d 11ab:4620' output doesn't contain the word 'GT-64120'
>
> Signed-off-by: Aleksandar Markovic <address@hidden>
> ---
> tests/acceptance/linux_ssh_mips_malta.py | 21 ++++++++++++++-------
> 1 file changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/tests/acceptance/linux_ssh_mips_malta.py
> b/tests/acceptance/linux_ssh_mips_malta.py
> index aafb0c3..cbf1b34 100644
> --- a/tests/acceptance/linux_ssh_mips_malta.py
> +++ b/tests/acceptance/linux_ssh_mips_malta.py
> @@ -147,20 +147,27 @@ class LinuxSSH(Test):
>
> def run_common_commands(self):
> stdout, stderr = self.ssh_command('lspci -d 11ab:4620')
> - self.assertIn(True, ["GT-64120" in line for line in stdout])
> + self.assertIn(True, ["GT-64120a" in line for line in stdout],
Looks like there's an extra, unintended, "a" in the expected output, that is,
s/GT-64120a/GT-64120/.
> + "'lspci -d 11ab:4620' output doesn't contain "
> + "the word 'GT-64120'")
>
> stdout, stderr = self.ssh_command('cat
> /sys/bus/i2c/devices/i2c-0/name')
> - self.assertIn(True, ["SMBus PIIX4 adapter" in line
> - for line in stdout])
> + self.assertIn(True, ["SMBus PIIX4 adaptera" in line
Here too (s/adaptera/adapter/).
> + for line in stdout],
> + "cat /sys/bus/i2c/devices/i2c-0/name' doesn't contain "
> + "the words 'SMBus PIIX4 adapter'")
>
> stdout, stderr = self.ssh_command('cat /proc/mtd')
> - self.assertIn(True, ["YAMON" in line
> - for line in stdout])
> + self.assertIn(True, ["YAMONa" in line
Also here (s/YAMONa/YAMONa/).
> + for line in stdout],
> + "'cat /proc/mtd' doesn't contain the word 'YAMON'")
>
> # Empty 'Board Config'
> stdout, stderr = self.ssh_command('md5sum /dev/mtd2ro')
> - self.assertIn(True, ["0dfbe8aa4c20b52e1b8bf3cb6cbdf193" in line
> - for line in stdout])
> + self.assertIn(True, ["0dfbe8aa4c20b52e1b8bf3cb6cbdf193a" in line
> + for line in stdout],
And finnaly in the hash
(s/0dfbe8aa4c20b52e1b8bf3cb6cbdf193a/0dfbe8aa4c20b52e1b8bf3cb6cbdf193/).
> + "'md5sum /dev/mtd2ro' doesn't contain "
> + "the word '0dfbe8aa4c20b52e1b8bf3cb6cbdf193'")
>
> def do_test_mips_malta(self, endianess, kernel_path, uname_m):
> self.boot_debian_wheezy_image_and_ssh_login(endianess, kernel_path)
> --
> 2.7.4
>
>
With those changes, the tests pass for me. I'd recommend though:
1) Not related to your patch, but it's good practice to name unused
variables "_", that is:
stdout, _ = self.ssh_command('lspci -d 11ab:4620')
2) Avoid repeating the expected content (which lead to the trailing
"a"s in this patch). Something like:
cmd = 'lspci -d 11ab:4620'
stdout, _ = self.ssh_command(cmd)
exp = "GT-64120"
self.assertIn(True, [exp in line for line in stdout],
'"%s" output does not contain "%s"' % (cmd, exp))
3) Optionally, create an utility function that would make the check
more obvious and avoid looping through all lines of the output
(and creating a list, which a list comprehension will do). Example:
def ssh_command_output_contains(self, cmd, exp):
stdout, _ = self.ssh_command(cmd)
for line in stdout:
if exp in line:
break
else:
self.fail('"%s" output does not contain "%s"' % (cmd, exp))
def run_common_commands(self):
self.ssh_command_output_contains('lspci -d 11ab:4620', 'GT-64120')
self.ssh_command_output_contains('cat /sys/bus/i2c/devices/i2c-0/name',
'SMBus PIIX4 adapter')
self.ssh_command_output_contains('cat /proc/mtd', 'YAMON')
# Empty 'Board Config'
self.ssh_command_output_contains('md5sum /dev/mtd2ro',
'0dfbe8aa4c20b52e1b8bf3cb6cbdf193')
Cheers,
- Cleber.