Re: [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py
On Jun 11, 2019 8:00 AM, "Aleksandar Markovic"
wrote:
>
>
> On Jun 11, 2019 1:24 AM, "Cleber Rosa" wrote:
> >
> > On Mon, Jun 10, 2019 at 09:49:10PM +0200, Aleksandar Markovic wrote:
> > > From: Aleksandar Markovic
> > >
> > > 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
> > > ---
> > > 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')
> >
>
> Thank you for your review, Cleber! I am almost certain that in v2 (that I
am going to send soon), I
Re: [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py
On Jun 11, 2019 1:24 AM, "Cleber Rosa" wrote:
>
> On Mon, Jun 10, 2019 at 09:49:10PM +0200, Aleksandar Markovic wrote:
> > From: Aleksandar Markovic
> >
> > 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
> > ---
> > 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')
>
Thank you for your review, Cleber! I am almost certain that in v2 (that I
am going to send soon), I will adopt the approach from point “3” of your
mail.
Yours,
Aleksandar
> Cheers,
> - Cleber.
>
Re: [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py
On Mon, Jun 10, 2019 at 09:49:10PM +0200, Aleksandar Markovic wrote:
> From: Aleksandar Markovic
>
> 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
> ---
> 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.
[Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py
From: Aleksandar Markovic
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
---
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],
+ "'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
+ 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
+ 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],
+ "'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
