Re: [Qemu-devel] [PATCH] tests/acceptance: Improve failure reporting in linux_ssh_mips_malta.py

[Cleber Rosa]

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.