On Jun 11, 2019 1:24 AM, "Cleber Rosa" <cr...@redhat.com> wrote:
>
> On Mon, Jun 10, 2019 at 09:49:10PM +0200, Aleksandar Markovic wrote:
> > From: Aleksandar Markovic <amarko...@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 <amarko...@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.
>
