Re: [PATCH 07/22] tests/acceptance/virtiofs_submounts.py: evaluate string not length

[Max Reitz]

On 04.02.21 14:23, Alex Bennée wrote:
> Cleber Rosa <crosa@redhat.com> writes:
>
> > If the vmlinuz variable is set to anything that evaluates to True,
> > then the respective arguments should be set.  If the variable contains
> > an empty string, than it will evaluate to False, and the extra
> > arguments will not be set.
> >
> > This keeps the same logic, but improves readability a bit.
> >
> > Signed-off-by: Cleber Rosa <crosa@redhat.com>
> > ---
> >   tests/acceptance/virtiofs_submounts.py | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tests/acceptance/virtiofs_submounts.py 
> > b/tests/acceptance/virtiofs_submounts.py
> > index f1b49f03bb..f25a386a19 100644
> > --- a/tests/acceptance/virtiofs_submounts.py
> > +++ b/tests/acceptance/virtiofs_submounts.py
> > @@ -241,7 +241,7 @@ class VirtiofsSubmountsTest(BootLinux):
> >   
> >           super(VirtiofsSubmountsTest, self).setUp(pubkey)
> >   
> > -        if len(vmlinuz) > 0:
> > +        if vmlinuz:
> >               self.vm.add_args('-kernel', vmlinuz,
> >                                '-append', 'console=ttyS0 root=/dev/sda1')
>
> And this is were I gave up because I can't see how to run the test:
>
>    ./tests/venv/bin/avocado run ./tests/acceptance/virtiofs_submounts.py
>    JOB ID     : b3d9bfcfcd603189a471bec7d770fca3b50a81ee
>    JOB LOG    : 
> /home/alex/avocado/job-results/job-2021-02-04T13.21-b3d9bfc/job.log
>     (1/5) 
> ./tests/acceptance/virtiofs_submounts.py:VirtiofsSubmountsTest.test_pre_virtiofsd_set_up:
>  CANCEL: vmlinuz parameter not set; you must point it to a Linux kernel binary 
> to test (to run this test with the on-image kernel, set it to an empty string) 
> (0.00 s)
>     (2/5) 
> ./tests/acceptance/virtiofs_submounts.py:VirtiofsSubmountsTest.test_pre_launch_set_up:
>  CANCEL: vmlinuz parameter not set; you must point it to a Linux kernel binary 
> to test (to run this test with the on-image kernel, set it to an empty string) 
> (0.00 s)
>     (3/5) 
> ./tests/acceptance/virtiofs_submounts.py:VirtiofsSubmountsTest.test_post_launch_set_up:
>  CANCEL: vmlinuz parameter not set; you must point it to a Linux kernel binary
>    to test (to run this test with the on-image kernel, set it to an empty 
> string) (0.00 s)
>     (4/5) 
> ./tests/acceptance/virtiofs_submounts.py:VirtiofsSubmountsTest.test_post_mount_set_up:
>  CANCEL: vmlinuz parameter not set; you must point it to a Linux kernel binary 
> to test (to run this test with the on-image kernel, set it to an empty string) 
> (0.00 s)
>     (5/5) 
> ./tests/acceptance/virtiofs_submounts.py:VirtiofsSubmountsTest.test_two_runs: 
> CANCEL: vmlinuz parameter not set; you must point it to a Linux kernel binary 
> to test (to run this test with the on-image kernel, set it to an empty string) 
> (0.00 s)
>    RESULTS    : PASS 0 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
> CANCEL 5
>    JOB TIME   : 0.56 s
>
> Given the test seems to make assumptions about an environment being
> setup for it I think we need some documentation somewhere about what
> those pre-requisites are.

I find the cancel message pretty clear, but then again it was me who 
wrote it...

Either you point the vmlinuz parameter to some Linux kernel you built 
yourself, or you set it to the empty string to just use the kernel 
that’s part of the image.  Setting Avocado parameters is done via -p, 
i.e. “-p key=value”.  So in this case
“-p vmlinuz=/path/to/linux/build/arch/x86/boot/bzImage”, or
“-p vmlinuz=''”.

Ideally, vmlinuz='' would be the default, but there’s a problem with 
that: I submitted this test along with the patches that added the 
required feature to the Linux kernel, so at that point that feature was 
not part of Linux.  That’s why you generally have to point it to a Linux 
kernel binary you built yourself that has this feature (5.10 does).

Using vmlinuz='' you can test it with the kernel that is part of the 
cloud image.  Once that kernel is sufficiently new (i.e., >=5.10), we 
can make that the default.

Max