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

[Alex Bennée]

Max Reitz <mreitz@redhat.com> writes:

> 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).

This is where it deviates from the rest of the check-acceptance tests.
Generally I don't have to worry about finding the right image myself. At
the least it would be worth pointing to a part of our docs on how to
build such an image.

> 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.

Good.

>
> Max


-- 
Alex Bennée