Re: [PATCH 1/4] tests/functional: Add a class containing Linux kernel helpers

[Thomas Huth <thuth@redhat.com>]

On 27/08/2024 11.21, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 27/8/24 07:34, Thomas Huth wrote:
>> On 27/08/2024 00.10, Philippe Mathieu-Daudé wrote:
>>> Use the 'linux_kernel' namespace to provide common helpers
>>> to functional tests booting a Linux kernel.
>>>
>>> Suggested-by: Thomas Huth <thuth@redhat.com>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> ---
>>>   tests/functional/qemu_test/linux_kernel.py | 19 +++++++++++++++++++
>>>   1 file changed, 19 insertions(+)
>>>   create mode 100644 tests/functional/qemu_test/linux_kernel.py
>>>
>>> diff --git a/tests/functional/qemu_test/linux_kernel.py 
>>> b/tests/functional/qemu_test/linux_kernel.py
>>> new file mode 100644
>>> index 0000000000..917beb7fb6
>>> --- /dev/null
>>> +++ b/tests/functional/qemu_test/linux_kernel.py
>>> @@ -0,0 +1,19 @@
>>> +# Linux kernel functional test helpers
>>> +#
>>> +# Copyright (c) 2024 Linaro Ltd.
>>> +#
>>> +# Author:
>>> +#  Philippe Mathieu-Daudé <philmd@linaro.org>
>>> +#
>>> +# SPDX-License-Identifier: GPL-2.0-or-later
>>> +
>>> +from . import wait_for_console_pattern
>>> +
>>> +KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
>>> +
>>> +KERNEL_PANIC_MESSAGE = 'Kernel panic - not syncing'
>>> +
>>> +def linux_kernel_wait_for_pattern(test, success_message):
>>> +    wait_for_console_pattern(test,
>>> +                             success_message=success_message,
>>> +                             failure_message=KERNEL_PANIC_MESSAGE)
>>
>> I'd prefer to have the whole LinuxKernelTest class here (without the 
>> BootLinuxConsole class of course), that way the modifications that we have 
>> to do to the converted tests are definitely smaller when we don't have to 
>> change the "wait_for_console_pattern" lines everywhere. And it's also 
>> nicer if the variables are properly wrapped in a class.
> 
> I'm a bit confused by your comment. I exposed my view on LinuxKernelTest
> methods (which I don't see specific to this class) here:
> https://lore.kernel.org/qemu-devel/9910ebc8-b7c4-4505-a987-3a5e308fb3d1@linaro.org/
> 
> Could your request be implemented on top on this patch, as we go (as it
> is already helpful in its current state)? Otherwise I'll ask you to
> share a snippet of what you mean, or even better to post a v2 :)

I'll post a mini patch series (converting the Q800 test) in reply to your 
mail to show what I had in mind...

  Thomas