* [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py @ 2019-06-10 19:49 Aleksandar Markovic 2019-06-10 23:24 ` Cleber Rosa 0 siblings, 1 reply; 4+ messages in thread From: Aleksandar Markovic @ 2019-06-10 19:49 UTC (permalink / raw) To: qemu-devel; +Cc: ccarrara, f4bug, ehabkost, amarkovic, crosa From: Aleksandar Markovic <amarkovic@wavecomp.com> 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 <amarkovic@wavecomp.com> --- 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py 2019-06-10 19:49 [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py Aleksandar Markovic @ 2019-06-10 23:24 ` Cleber Rosa 2019-06-11 6:00 ` Aleksandar Markovic 0 siblings, 1 reply; 4+ messages in thread From: Cleber Rosa @ 2019-06-10 23:24 UTC (permalink / raw) To: Aleksandar Markovic; +Cc: amarkovic, qemu-devel, ehabkost, f4bug On Mon, Jun 10, 2019 at 09:49:10PM +0200, Aleksandar Markovic wrote: > From: Aleksandar Markovic <amarkovic@wavecomp.com> > > 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 <amarkovic@wavecomp.com> > --- > 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py 2019-06-10 23:24 ` Cleber Rosa @ 2019-06-11 6:00 ` Aleksandar Markovic 2019-06-11 14:32 ` Aleksandar Markovic 0 siblings, 1 reply; 4+ messages in thread From: Aleksandar Markovic @ 2019-06-11 6:00 UTC (permalink / raw) To: Cleber Rosa; +Cc: Aleksandar Markovic, f4bug, qemu-devel, amarkovic, ehabkost On Jun 11, 2019 1:24 AM, "Cleber Rosa" <crosa@redhat.com> wrote: > > On Mon, Jun 10, 2019 at 09:49:10PM +0200, Aleksandar Markovic wrote: > > From: Aleksandar Markovic <amarkovic@wavecomp.com> > > > > 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 <amarkovic@wavecomp.com> > > --- > > 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. > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py 2019-06-11 6:00 ` Aleksandar Markovic @ 2019-06-11 14:32 ` Aleksandar Markovic 0 siblings, 0 replies; 4+ messages in thread From: Aleksandar Markovic @ 2019-06-11 14:32 UTC (permalink / raw) To: Cleber Rosa; +Cc: Aleksandar Markovic, f4bug, qemu-devel, amarkovic, ehabkost On Jun 11, 2019 8:00 AM, "Aleksandar Markovic" <aleksandar.m.mail@gmail.com> wrote: > > > On Jun 11, 2019 1:24 AM, "Cleber Rosa" <crosa@redhat.com> wrote: > > > > On Mon, Jun 10, 2019 at 09:49:10PM +0200, Aleksandar Markovic wrote: > > > From: Aleksandar Markovic <amarkovic@wavecomp.com> > > > > > > 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 <amarkovic@wavecomp.com> > > > --- > > > 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. > > Trailing “a”s are just leftover of my testing the script (forcing it to report failures), and it is good that you spotted them, but they will disappear in v2. Thanks, Aleksandar ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-11 14:45 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-06-10 19:49 [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py Aleksandar Markovic 2019-06-10 23:24 ` Cleber Rosa 2019-06-11 6:00 ` Aleksandar Markovic 2019-06-11 14:32 ` Aleksandar Markovic
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).