[PATCH 3/6] tests/functional: Fix problems in linuxkerenl.py reported by pylint

Thomas Huth posted 6 patches 1 month ago
Maintainers: Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, "Daniel P. Berrangé" <berrange@redhat.com>
[PATCH 3/6] tests/functional: Fix problems in linuxkerenl.py reported by pylint
Posted by Thomas Huth 1 month ago
From: Thomas Huth <thuth@redhat.com>

Use proper indentation and lazy logging here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/functional/qemu_test/linuxkernel.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tests/functional/qemu_test/linuxkernel.py b/tests/functional/qemu_test/linuxkernel.py
index 2aca0ee3cd0..a20278966be 100644
--- a/tests/functional/qemu_test/linuxkernel.py
+++ b/tests/functional/qemu_test/linuxkernel.py
@@ -24,12 +24,12 @@ def launch_kernel(self, kernel, initrd=None, dtb=None, console_index=0,
         self.vm.set_console(console_index=console_index)
         self.vm.add_args('-kernel', kernel)
         if initrd:
-                self.vm.add_args('-initrd', initrd)
+            self.vm.add_args('-initrd', initrd)
         if dtb:
-                self.vm.add_args('-dtb', dtb)
+            self.vm.add_args('-dtb', dtb)
         self.vm.launch()
         if wait_for:
-                self.wait_for_console_pattern(wait_for)
+            self.wait_for_console_pattern(wait_for)
 
     def check_http_download(self, filename, hashsum, guestport=8080,
                             pythoncmd='python3 -m http.server'):
@@ -39,7 +39,7 @@ def check_http_download(self, filename, hashsum, guestport=8080,
         hl = hashlib.sha256()
         hostport = get_usernet_hostfwd_port(self.vm)
         url = f'http://localhost:{hostport}{filename}'
-        self.log.info(f'Downloading {url} ...')
+        self.log.info('Downloading %s ...', url)
         with urllib.request.urlopen(url) as response:
             while True:
                 chunk = response.read(1 << 20)
@@ -48,5 +48,5 @@ def check_http_download(self, filename, hashsum, guestport=8080,
                 hl.update(chunk)
 
         digest = hl.hexdigest()
-        self.log.info(f'sha256sum of download is {digest}.')
+        self.log.info('sha256sum of download is %s.', digest)
         self.assertEqual(digest, hashsum)
-- 
2.51.0
Re: [PATCH 3/6] tests/functional: Fix problems in linuxkerenl.py reported by pylint
Posted by Philippe Mathieu-Daudé 3 weeks, 2 days ago
Typo "linuxkernel" in subject.

On 15/10/25 11:54, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> Use proper indentation and lazy logging here.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/functional/qemu_test/linuxkernel.py | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)


> @@ -48,5 +48,5 @@ def check_http_download(self, filename, hashsum, guestport=8080,
>                   hl.update(chunk)
>   
>           digest = hl.hexdigest()
> -        self.log.info(f'sha256sum of download is {digest}.')
> +        self.log.info('sha256sum of download is %s.', digest)
>           self.assertEqual(digest, hashsum)

TBH I don't understand why 'lazy logging' is better than f-strings.
I'd rather have an unified formatting style over our Python files.

https://pylint.readthedocs.io/en/latest/user_guide/messages/warning/logging-format-interpolation.html

   Another reasonable option is to use f-string. If you want to do
   that, you need to enable logging-format-interpolation and disable
   logging-fstring-interpolation.

What about this pylint option?
Re: [PATCH 3/6] tests/functional: Fix problems in linuxkerenl.py reported by pylint
Posted by Thomas Huth 3 weeks, 1 day ago
On 22/10/2025 21.14, Philippe Mathieu-Daudé wrote:
> Typo "linuxkernel" in subject.
> 
> On 15/10/25 11:54, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> Use proper indentation and lazy logging here.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/functional/qemu_test/linuxkernel.py | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> 
>> @@ -48,5 +48,5 @@ def check_http_download(self, filename, hashsum, 
>> guestport=8080,
>>                   hl.update(chunk)
>>           digest = hl.hexdigest()
>> -        self.log.info(f'sha256sum of download is {digest}.')
>> +        self.log.info('sha256sum of download is %s.', digest)
>>           self.assertEqual(digest, hashsum)
> 
> TBH I don't understand why 'lazy logging' is better than f-strings.

If I got this right, it's about a small performance improvement: If the 
logging function decides that the log output could be dropped since the 
logging level does not match, there is no need to format the string in that 
case.

But thinking about this, I guess that's not a valuable argument for the 
tests, since we're not calling these functions again and again, so the 
performance impact is negligible here. So fine for me if we ignore this 
warning from pylint. ... i.e. we might want to introduce a pylintrc file 
now, similar to what we have in scripts/qapi/pylintrc or 
tests/qemu-iotests/pylintrc already ?

  Thomas


Re: [PATCH 3/6] tests/functional: Fix problems in linuxkerenl.py reported by pylint
Posted by John Snow 3 weeks, 1 day ago
On Thu, Oct 23, 2025 at 2:46 AM Thomas Huth <thuth@redhat.com> wrote:
>
> On 22/10/2025 21.14, Philippe Mathieu-Daudé wrote:
> > Typo "linuxkernel" in subject.
> >
> > On 15/10/25 11:54, Thomas Huth wrote:
> >> From: Thomas Huth <thuth@redhat.com>
> >>
> >> Use proper indentation and lazy logging here.
> >>
> >> Signed-off-by: Thomas Huth <thuth@redhat.com>
> >> ---
> >>   tests/functional/qemu_test/linuxkernel.py | 10 +++++-----
> >>   1 file changed, 5 insertions(+), 5 deletions(-)
> >
> >
> >> @@ -48,5 +48,5 @@ def check_http_download(self, filename, hashsum,
> >> guestport=8080,
> >>                   hl.update(chunk)
> >>           digest = hl.hexdigest()
> >> -        self.log.info(f'sha256sum of download is {digest}.')
> >> +        self.log.info('sha256sum of download is %s.', digest)
> >>           self.assertEqual(digest, hashsum)
> >
> > TBH I don't understand why 'lazy logging' is better than f-strings.
>
> If I got this right, it's about a small performance improvement: If the
> logging function decides that the log output could be dropped since the
> logging level does not match, there is no need to format the string in that
> case.

Yeah, it just doesn't interpolate the strings until it actually goes
to log; and the logging API was established a long time before
f-strings existed. You can actually switch the logging format to use
{} style placeholders instead, but the issue with this (AFAIUI - I
have not actually researched this) is that all logging messages in the
running process need to be updated to match, so it's hard to dictate
this on a per-library basis.

Maybe you can change it per-logger, though, and it's fine. I am not really sure.

>
> But thinking about this, I guess that's not a valuable argument for the
> tests, since we're not calling these functions again and again, so the
> performance impact is negligible here. So fine for me if we ignore this
> warning from pylint. ... i.e. we might want to introduce a pylintrc file
> now, similar to what we have in scripts/qapi/pylintrc or
> tests/qemu-iotests/pylintrc already ?

So far, we don't suppress this warning anywhere that I am aware of. I
have adopted a "when in rome" approach and just follow the principle
of least surprise; i.e. "the defaults in pylint are probably the
default for a reason, may as well just follow along until and unless
there's a strong reason to switch"

That said, go ahead and ignore them if you'd like.

>
>   Thomas
>