[PATCH 0/6] tests/functional: Fix various problems reported by pylint

[PATCH 0/6] tests/functional: Fix various problems reported by pylint

[Thomas Huth]

Using pylint to improve the code quality in the tests/functional/qemu_test
a little bit, for example:

- put the doc strings in the right locations (after the "def" line, not
  in front of it)
- use the right indentation everywhere (4 spaces)
- use isinstance() instead of checking via type()
- use lazy logging strings

Thomas Huth (6):
  tests/functional: Fix problems in asset.py reported by pylint
  tests/functional: Fix problems in decorators.py reported by pylint
  tests/functional: Fix problems in linuxkerenl.py reported by pylint
  tests/functional: Fix problems in testcase.py reported by pylint
  tests/functional: Fix problems in uncompress.py reported by pylint
  tests/functional: Fix problems in utils.py reported by pylint

 tests/functional/qemu_test/asset.py       |  24 ++-
 tests/functional/qemu_test/decorators.py  | 176 ++++++++-------
 tests/functional/qemu_test/linuxkernel.py |  10 +-
 tests/functional/qemu_test/testcase.py    | 251 +++++++++++-----------
 tests/functional/qemu_test/uncompress.py  |  40 ++--
 tests/functional/qemu_test/utils.py       |  22 +-
 6 files changed, 262 insertions(+), 261 deletions(-)

-- 
2.51.0

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

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

The "raise" without an Exception was a real problem, the other
spots are rather cosmetics.

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

diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
index f666125bfaf..ba3771027b4 100644
--- a/tests/functional/qemu_test/asset.py
+++ b/tests/functional/qemu_test/asset.py
@@ -111,7 +111,7 @@ def _wait_for_other_download(self, tmp_cache_file):
                 return False
 
         self.log.debug("Time out while waiting for %s!", tmp_cache_file)
-        raise
+        raise TimeoutError(f"Time out while waiting for {tmp_cache_file}")
 
     def fetch(self):
         if not self.cache_dir.exists():
@@ -129,7 +129,7 @@ def fetch(self):
         self.log.info("Downloading %s to %s...", self.url, self.cache_file)
         tmp_cache_file = self.cache_file.with_suffix(".download")
 
-        for retries in range(3):
+        for _ in range(3):
             try:
                 with tmp_cache_file.open("xb") as dst:
                     with urllib.request.urlopen(self.url) as resp:
@@ -169,7 +169,7 @@ def fetch(self):
                 # server or networking problem
                 if e.code == 404:
                     raise AssetError(self, "Unable to download: "
-                                     "HTTP error %d" % e.code)
+                                     "HTTP error %d" % e.code) from e
                 continue
             except URLError as e:
                 # This is typically a network/service level error
@@ -178,7 +178,7 @@ def fetch(self):
                 self.log.error("Unable to download %s: URL error %s",
                                self.url, e.reason)
                 raise AssetError(self, "Unable to download: URL error %s" %
-                                 e.reason, transient=True)
+                                 e.reason, transient=True) from e
             except ConnectionError as e:
                 # A socket connection failure, such as dropped conn
                 # or refused conn
@@ -189,7 +189,7 @@ def fetch(self):
             except Exception as e:
                 tmp_cache_file.unlink()
                 raise AssetError(self, "Unable to download: %s" % e,
-                                 transient=True)
+                                 transient=True) from e
 
         if not os.path.exists(tmp_cache_file):
             raise AssetError(self, "Download retries exceeded", transient=True)
@@ -202,7 +202,6 @@ def fetch(self):
                         self.hash.encode('utf8'))
         except Exception as e:
             self.log.debug("Unable to set xattr on %s: %s", tmp_cache_file, e)
-            pass
 
         if not self._check(tmp_cache_file):
             tmp_cache_file.unlink()
@@ -211,9 +210,10 @@ def fetch(self):
         # Remove write perms to stop tests accidentally modifying them
         os.chmod(self.cache_file, stat.S_IRUSR | stat.S_IRGRP)
 
-        self.log.info("Cached %s at %s" % (self.url, self.cache_file))
+        self.log.info("Cached %s at %s", self.url, self.cache_file)
         return str(self.cache_file)
 
+    @staticmethod
     def precache_test(test):
         log = logging.getLogger('qemu-test')
         log.setLevel(logging.DEBUG)
@@ -224,16 +224,17 @@ def precache_test(test):
         handler.setFormatter(formatter)
         log.addHandler(handler)
         for name, asset in vars(test.__class__).items():
-            if name.startswith("ASSET_") and type(asset) == Asset:
+            if name.startswith("ASSET_") and isinstance(asset, Asset):
                 try:
                     asset.fetch()
                 except AssetError as e:
                     if not e.transient:
                         raise
-                    log.error("%s: skipping asset precache" % e)
+                    log.error("%s: skipping asset precache", e)
 
         log.removeHandler(handler)
 
+    @staticmethod
     def precache_suite(suite):
         for test in suite:
             if isinstance(test, unittest.TestSuite):
@@ -241,9 +242,10 @@ def precache_suite(suite):
             elif isinstance(test, unittest.TestCase):
                 Asset.precache_test(test)
 
-    def precache_suites(path, cacheTstamp):
+    @staticmethod
+    def precache_suites(path, cache_tstamp):
         loader = unittest.loader.defaultTestLoader
         tests = loader.loadTestsFromNames([path], None)
 
-        with open(cacheTstamp, "w") as fh:
+        with open(cache_tstamp, "w", encoding='utf-8'):
             Asset.precache_suite(tests)
-- 
2.51.0

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

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

The documentation strings should follow the function definition
lines, not precede them.

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

diff --git a/tests/functional/qemu_test/decorators.py b/tests/functional/qemu_test/decorators.py
index b2392958041..807418359ab 100644
--- a/tests/functional/qemu_test/decorators.py
+++ b/tests/functional/qemu_test/decorators.py
@@ -10,136 +10,134 @@
 
 from .cmd import which
 
-'''
-Decorator to skip execution of a test if the provided
-environment variables are not set.
-Example:
 
-  @skipIfMissingEnv("QEMU_ENV_VAR0", "QEMU_ENV_VAR1")
-'''
 def skipIfMissingEnv(*vars_):
+    '''
+    Decorator to skip execution of a test if the provided
+    environment variables are not set.
+    Example:
+
+      @skipIfMissingEnv("QEMU_ENV_VAR0", "QEMU_ENV_VAR1")
+    '''
     missing_vars = []
     for var in vars_:
-        if os.getenv(var) == None:
+        if os.getenv(var) is None:
             missing_vars.append(var)
 
-    has_vars = True if len(missing_vars) == 0 else False
+    has_vars = len(missing_vars) == 0
 
     return skipUnless(has_vars, f"Missing env var(s): {', '.join(missing_vars)}")
 
-'''
-
-Decorator to skip execution of a test if the list
-of command binaries is not available in $PATH.
-Example:
-
-  @skipIfMissingCommands("mkisofs", "losetup")
-'''
 def skipIfMissingCommands(*args):
+    '''
+    Decorator to skip execution of a test if the list
+    of command binaries is not available in $PATH.
+    Example:
+
+      @skipIfMissingCommands("mkisofs", "losetup")
+    '''
     has_cmds = True
     for cmd in args:
-         if not which(cmd):
-             has_cmds = False
-             break
+        if not which(cmd):
+            has_cmds = False
+            break
 
     return skipUnless(has_cmds, 'required command(s) "%s" not installed' %
                                 ", ".join(args))
 
-'''
-Decorator to skip execution of a test if the current
-host operating system does match one of the prohibited
-ones.
-Example
-
-  @skipIfOperatingSystem("Linux", "Darwin")
-'''
 def skipIfOperatingSystem(*args):
+    '''
+    Decorator to skip execution of a test if the current host
+    operating system does match one of the prohibited ones.
+    Example:
+
+      @skipIfOperatingSystem("Linux", "Darwin")
+    '''
     return skipIf(platform.system() in args,
                   'running on an OS (%s) that is not able to run this test' %
                   ", ".join(args))
 
-'''
-Decorator to skip execution of a test if the current
-host machine does not match one of the permitted
-machines.
-Example
-
-  @skipIfNotMachine("x86_64", "aarch64")
-'''
 def skipIfNotMachine(*args):
+    '''
+    Decorator to skip execution of a test if the current
+    host machine does not match one of the permitted machines.
+    Example:
+
+      @skipIfNotMachine("x86_64", "aarch64")
+    '''
     return skipUnless(platform.machine() in args,
                       'not running on one of the required machine(s) "%s"' %
                       ", ".join(args))
 
-'''
-Decorator to skip execution of flaky tests, unless
-the $QEMU_TEST_FLAKY_TESTS environment variable is set.
-A bug URL must be provided that documents the observed
-failure behaviour, so it can be tracked & re-evaluated
-in future.
+def skipFlakyTest(bug_url):
+    '''
+    Decorator to skip execution of flaky tests, unless
+    the $QEMU_TEST_FLAKY_TESTS environment variable is set.
+    A bug URL must be provided that documents the observed
+    failure behaviour, so it can be tracked & re-evaluated
+    in future.
 
-Historical tests may be providing "None" as the bug_url
-but this should not be done for new test.
+    Historical tests may be providing "None" as the bug_url
+    but this should not be done for new test.
 
-Example:
+    Example:
 
-  @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/NNN")
-'''
-def skipFlakyTest(bug_url):
+      @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/NNN")
+    '''
     if bug_url is None:
         bug_url = "FIXME: reproduce flaky test and file bug report or remove"
     return skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'),
                       f'Test is unstable: {bug_url}')
 
-'''
-Decorator to skip execution of tests which are likely
-to execute untrusted commands on the host, or commands
-which process untrusted code, unless the
-$QEMU_TEST_ALLOW_UNTRUSTED_CODE env var is set.
-Example:
-
-  @skipUntrustedTest()
-'''
 def skipUntrustedTest():
+    '''
+    Decorator to skip execution of tests which are likely
+    to execute untrusted commands on the host, or commands
+    which process untrusted code, unless the
+    $QEMU_TEST_ALLOW_UNTRUSTED_CODE env var is set.
+    Example:
+
+      @skipUntrustedTest()
+    '''
     return skipUnless(os.getenv('QEMU_TEST_ALLOW_UNTRUSTED_CODE'),
                       'Test runs untrusted code / processes untrusted data')
 
-'''
-Decorator to skip execution of tests which need large
-data storage (over around 500MB-1GB mark) on the host,
-unless the $QEMU_TEST_ALLOW_LARGE_STORAGE environment
-variable is set
+def skipBigDataTest():
+    '''
+    Decorator to skip execution of tests which need large
+    data storage (over around 500MB-1GB mark) on the host,
+    unless the $QEMU_TEST_ALLOW_LARGE_STORAGE environment
+    variable is set
 
-Example:
+    Example:
 
-  @skipBigDataTest()
-'''
-def skipBigDataTest():
+      @skipBigDataTest()
+    '''
     return skipUnless(os.getenv('QEMU_TEST_ALLOW_LARGE_STORAGE'),
                       'Test requires large host storage space')
 
-'''
-Decorator to skip execution of tests which have a really long
-runtime (and might e.g. time out if QEMU has been compiled with
-debugging enabled) unless the $QEMU_TEST_ALLOW_SLOW
-environment variable is set
+def skipSlowTest():
+    '''
+    Decorator to skip execution of tests which have a really long
+    runtime (and might e.g. time out if QEMU has been compiled with
+    debugging enabled) unless the $QEMU_TEST_ALLOW_SLOW
+    environment variable is set
 
-Example:
+    Example:
 
-  @skipSlowTest()
-'''
-def skipSlowTest():
+      @skipSlowTest()
+    '''
     return skipUnless(os.getenv('QEMU_TEST_ALLOW_SLOW'),
                       'Test has a very long runtime and might time out')
 
-'''
-Decorator to skip execution of a test if the list
-of python imports is not available.
-Example:
-
-  @skipIfMissingImports("numpy", "cv2")
-'''
 def skipIfMissingImports(*args):
+    '''
+    Decorator to skip execution of a test if the list
+    of python imports is not available.
+    Example:
+
+      @skipIfMissingImports("numpy", "cv2")
+    '''
     has_imports = True
     for impname in args:
         try:
@@ -151,15 +149,15 @@ def skipIfMissingImports(*args):
     return skipUnless(has_imports, 'required import(s) "%s" not installed' %
                                    ", ".join(args))
 
-'''
-Decorator to skip execution of a test if the system's
-locked memory limit is below the required threshold.
-Takes required locked memory threshold in kB.
-Example:
-
-  @skipLockedMemoryTest(2_097_152)
-'''
 def skipLockedMemoryTest(locked_memory):
+    '''
+    Decorator to skip execution of a test if the system's
+    locked memory limit is below the required threshold.
+    Takes required locked memory threshold in kB.
+    Example:
+
+      @skipLockedMemoryTest(2_097_152)
+    '''
     # get memlock hard limit in bytes
     _, ulimit_memory = resource.getrlimit(resource.RLIMIT_MEMLOCK)
 
-- 
2.51.0

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

[Thomas Huth]

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

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

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

- put 3rd party "import pycotap" after the standard imports
- "help" is a built-in function in Python, don't use it as a variable name
- put the doc strings in the right locations (after the "def" line)
- use isinstance() instead of checking via type()
- use lazy logging strings

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

diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
index 2c0abde3957..47fb738a595 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -14,7 +14,6 @@
 import logging
 import os
 from pathlib import Path
-import pycotap
 import shutil
 from subprocess import run
 import sys
@@ -23,6 +22,8 @@
 import unittest
 import uuid
 
+import pycotap
+
 from qemu.machine import QEMUMachine
 from qemu.utils import hvf_available, kvm_available, tcg_available
 
@@ -34,50 +35,50 @@
 
 class QemuBaseTest(unittest.TestCase):
 
-    '''
-    @params compressed: filename, Asset, or file-like object to uncompress
-    @params format: optional compression format (gzip, lzma)
+    def uncompress(self, compressed, format=None):
+        '''
+        @params compressed: filename, Asset, or file-like object to uncompress
+        @params format: optional compression format (gzip, lzma)
 
-    Uncompresses @compressed into the scratch directory.
+        Uncompresses @compressed into the scratch directory.
 
-    If @format is None, heuristics will be applied to guess the format
-    from the filename or Asset URL. @format must be non-None if @uncompressed
-    is a file-like object.
+        If @format is None, heuristics will be applied to guess the
+        format from the filename or Asset URL. @format must be non-None
+        if @uncompressed is a file-like object.
 
-    Returns the fully qualified path to the uncompressed file
-    '''
-    def uncompress(self, compressed, format=None):
-        self.log.debug(f"Uncompress {compressed} format={format}")
-        if type(compressed) == Asset:
+        Returns the fully qualified path to the uncompressed file
+        '''
+        self.log.debug("Uncompress %s format=%s", compressed, format)
+        if isinstance(compressed, Asset):
             compressed.fetch()
 
-        (name, ext) = os.path.splitext(str(compressed))
+        (name, _ext) = os.path.splitext(str(compressed))
         uncompressed = self.scratch_file(os.path.basename(name))
 
         uncompress(compressed, uncompressed, format)
 
         return uncompressed
 
-    '''
-    @params archive: filename, Asset, or file-like object to extract
-    @params format: optional archive format (tar, zip, deb, cpio)
-    @params sub_dir: optional sub-directory to extract into
-    @params member: optional member file to limit extraction to
-
-    Extracts @archive into the scratch directory, or a directory beneath
-    named by @sub_dir. All files are extracted unless @member specifies
-    a limit.
-
-    If @format is None, heuristics will be applied to guess the format
-    from the filename or Asset URL. @format must be non-None if @archive
-    is a file-like object.
-
-    If @member is non-None, returns the fully qualified path to @member
-    '''
     def archive_extract(self, archive, format=None, sub_dir=None, member=None):
-        self.log.debug(f"Extract {archive} format={format}" +
-                       f"sub_dir={sub_dir} member={member}")
-        if type(archive) == Asset:
+        '''
+        @params archive: filename, Asset, or file-like object to extract
+        @params format: optional archive format (tar, zip, deb, cpio)
+        @params sub_dir: optional sub-directory to extract into
+        @params member: optional member file to limit extraction to
+
+        Extracts @archive into the scratch directory, or a directory beneath
+        named by @sub_dir. All files are extracted unless @member specifies
+        a limit.
+
+        If @format is None, heuristics will be applied to guess the
+        format from the filename or Asset URL. @format must be non-None
+        if @archive is a file-like object.
+
+        If @member is non-None, returns the fully qualified path to @member
+        '''
+        self.log.debug("Extract %s format=%s sub_dir=%s member=%s",
+                       archive, format, sub_dir, member)
+        if isinstance(archive, Asset):
             archive.fetch()
         if sub_dir is None:
             archive_extract(archive, self.scratch_file(), format, member)
@@ -89,110 +90,110 @@ def archive_extract(self, archive, format=None, sub_dir=None, member=None):
             return self.scratch_file(member)
         return None
 
-    '''
-    Create a temporary directory suitable for storing UNIX
-    socket paths.
-
-    Returns: a tempfile.TemporaryDirectory instance
-    '''
     def socket_dir(self):
+        '''
+        Create a temporary directory suitable for storing UNIX
+        socket paths.
+
+        Returns: a tempfile.TemporaryDirectory instance
+        '''
         if self.socketdir is None:
             self.socketdir = tempfile.TemporaryDirectory(
                 prefix="qemu_func_test_sock_")
         return self.socketdir
 
-    '''
-    @params args list of zero or more subdirectories or file
-
-    Construct a path for accessing a data file located
-    relative to the source directory that is the root for
-    functional tests.
-
-    @args may be an empty list to reference the root dir
-    itself, may be a single element to reference a file in
-    the root directory, or may be multiple elements to
-    reference a file nested below. The path components
-    will be joined using the platform appropriate path
-    separator.
-
-    Returns: string representing a file path
-    '''
     def data_file(self, *args):
+        '''
+        @params args list of zero or more subdirectories or file
+
+        Construct a path for accessing a data file located
+        relative to the source directory that is the root for
+        functional tests.
+
+        @args may be an empty list to reference the root dir
+        itself, may be a single element to reference a file in
+        the root directory, or may be multiple elements to
+        reference a file nested below. The path components
+        will be joined using the platform appropriate path
+        separator.
+
+        Returns: string representing a file path
+        '''
         return str(Path(Path(__file__).parent.parent, *args))
 
-    '''
-    @params args list of zero or more subdirectories or file
-
-    Construct a path for accessing a data file located
-    relative to the build directory root.
-
-    @args may be an empty list to reference the build dir
-    itself, may be a single element to reference a file in
-    the build directory, or may be multiple elements to
-    reference a file nested below. The path components
-    will be joined using the platform appropriate path
-    separator.
-
-    Returns: string representing a file path
-    '''
     def build_file(self, *args):
-        return str(Path(BUILD_DIR, *args))
+        '''
+        @params args list of zero or more subdirectories or file
 
-    '''
-    @params args list of zero or more subdirectories or file
+        Construct a path for accessing a data file located
+        relative to the build directory root.
 
-    Construct a path for accessing/creating a scratch file
-    located relative to a temporary directory dedicated to
-    this test case. The directory and its contents will be
-    purged upon completion of the test.
+        @args may be an empty list to reference the build dir
+        itself, may be a single element to reference a file in
+        the build directory, or may be multiple elements to
+        reference a file nested below. The path components
+        will be joined using the platform appropriate path
+        separator.
 
-    @args may be an empty list to reference the scratch dir
-    itself, may be a single element to reference a file in
-    the scratch directory, or may be multiple elements to
-    reference a file nested below. The path components
-    will be joined using the platform appropriate path
-    separator.
+        Returns: string representing a file path
+        '''
+        return str(Path(BUILD_DIR, *args))
 
-    Returns: string representing a file path
-    '''
     def scratch_file(self, *args):
+        '''
+        @params args list of zero or more subdirectories or file
+
+        Construct a path for accessing/creating a scratch file
+        located relative to a temporary directory dedicated to
+        this test case. The directory and its contents will be
+        purged upon completion of the test.
+
+        @args may be an empty list to reference the scratch dir
+        itself, may be a single element to reference a file in
+        the scratch directory, or may be multiple elements to
+        reference a file nested below. The path components
+        will be joined using the platform appropriate path
+        separator.
+
+        Returns: string representing a file path
+        '''
         return str(Path(self.workdir, *args))
 
-    '''
-    @params args list of zero or more subdirectories or file
-
-    Construct a path for accessing/creating a log file
-    located relative to a temporary directory dedicated to
-    this test case. The directory and its log files will be
-    preserved upon completion of the test.
-
-    @args may be an empty list to reference the log dir
-    itself, may be a single element to reference a file in
-    the log directory, or may be multiple elements to
-    reference a file nested below. The path components
-    will be joined using the platform appropriate path
-    separator.
-
-    Returns: string representing a file path
-    '''
     def log_file(self, *args):
+        '''
+        @params args list of zero or more subdirectories or file
+
+        Construct a path for accessing/creating a log file
+        located relative to a temporary directory dedicated to
+        this test case. The directory and its log files will be
+        preserved upon completion of the test.
+
+        @args may be an empty list to reference the log dir
+        itself, may be a single element to reference a file in
+        the log directory, or may be multiple elements to
+        reference a file nested below. The path components
+        will be joined using the platform appropriate path
+        separator.
+
+        Returns: string representing a file path
+        '''
         return str(Path(self.outputdir, *args))
 
-    '''
-    @params plugin name
-
-    Return the full path to the plugin taking into account any host OS
-    specific suffixes.
-    '''
     def plugin_file(self, plugin_name):
+        '''
+        @params plugin name
+
+        Return the full path to the plugin taking into account any host OS
+        specific suffixes.
+        '''
         sfx = dso_suffix()
         return os.path.join('tests', 'tcg', 'plugins', f'{plugin_name}.{sfx}')
 
     def assets_available(self):
         for name, asset in vars(self.__class__).items():
-            if name.startswith("ASSET_") and type(asset) == Asset:
+            if name.startswith("ASSET_") and isinstance(asset, Asset):
                 if not asset.available():
-                    self.log.debug(f"Asset {asset.url} not available")
+                    self.log.debug("Asset %s not available", asset.url)
                     return False
         return True
 
@@ -216,9 +217,9 @@ def setUp(self):
         self.log.setLevel(logging.DEBUG)
         self._log_fh = logging.FileHandler(self.log_filename, mode='w')
         self._log_fh.setLevel(logging.DEBUG)
-        fileFormatter = logging.Formatter(
+        file_formatter = logging.Formatter(
             '%(asctime)s - %(levelname)s: %(message)s')
-        self._log_fh.setFormatter(fileFormatter)
+        self._log_fh.setFormatter(file_formatter)
         self.log.addHandler(self._log_fh)
 
         # Capture QEMUMachine logging
@@ -256,7 +257,7 @@ def main():
         res = unittest.main(module = None, testRunner = tr, exit = False,
                             argv=[sys.argv[0], path] + sys.argv[1:])
         failed = {}
-        for (test, message) in res.result.errors + res.result.failures:
+        for (test, _message) in res.result.errors + res.result.failures:
             if hasattr(test, "log_filename") and not test.id() in failed:
                 print('More information on ' + test.id() + ' could be found here:'
                       '\n %s' % test.log_filename, file=sys.stderr)
@@ -275,7 +276,9 @@ def setUp(self):
     def add_ldpath(self, ldpath):
         self._ldpath.append(os.path.abspath(ldpath))
 
-    def run_cmd(self, bin_path, args=[]):
+    def run_cmd(self, bin_path, args=None):
+        if args is None:
+            args = []
         return run([self.qemu_bin]
                    + ["-L %s" % ldpath for ldpath in self._ldpath]
                    + [bin_path]
@@ -300,8 +303,8 @@ def setUp(self):
         self._console_log_fh = logging.FileHandler(self.console_log_name,
                                                    mode='w')
         self._console_log_fh.setLevel(logging.DEBUG)
-        fileFormatter = logging.Formatter('%(asctime)s: %(message)s')
-        self._console_log_fh.setFormatter(fileFormatter)
+        file_formatter = logging.Formatter('%(asctime)s: %(message)s')
+        self._console_log_fh.setFormatter(file_formatter)
         console_log.addHandler(self._console_log_fh)
 
     def set_machine(self, machinename):
@@ -339,17 +342,15 @@ def require_accelerator(self, accelerator):
                           "available" % accelerator)
 
     def require_netdev(self, netdevname):
-        help = run([self.qemu_bin,
-                    '-M', 'none', '-netdev', 'help'],
-                   capture_output=True, check=True, encoding='utf8').stdout;
-        if help.find('\n' + netdevname + '\n') < 0:
+        helptxt = run([self.qemu_bin, '-M', 'none', '-netdev', 'help'],
+                      capture_output=True, check=True, encoding='utf8').stdout
+        if helptxt.find('\n' + netdevname + '\n') < 0:
             self.skipTest('no support for " + netdevname + " networking')
 
     def require_device(self, devicename):
-        help = run([self.qemu_bin,
-                    '-M', 'none', '-device', 'help'],
-                   capture_output=True, check=True, encoding='utf8').stdout;
-        if help.find(devicename) < 0:
+        helptxt = run([self.qemu_bin, '-M', 'none', '-device', 'help'],
+                   capture_output=True, check=True, encoding='utf8').stdout
+        if helptxt.find(devicename) < 0:
             self.skipTest('no support for device ' + devicename)
 
     def _new_vm(self, name, *args):
@@ -411,7 +412,7 @@ def tearDown(self):
             try:
                 vm.shutdown()
             except Exception as ex:
-                self.log.error("Failed to teardown VM: %s" % ex)
+                self.log.error("Failed to teardown VM: %s", ex)
         logging.getLogger('console').removeHandler(self._console_log_fh)
         self._console_log_fh.close()
         super().tearDown()
-- 
2.51.0

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

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

- put the doc strings in the right locations (after the "def" line)
- use isinstance() instead of checking via type()

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

diff --git a/tests/functional/qemu_test/uncompress.py b/tests/functional/qemu_test/uncompress.py
index b7ef8f759b7..5bbdf8fe323 100644
--- a/tests/functional/qemu_test/uncompress.py
+++ b/tests/functional/qemu_test/uncompress.py
@@ -58,20 +58,20 @@ def zstd_uncompress(zstd_path, output_path):
     os.chmod(output_path, stat.S_IRUSR | stat.S_IWUSR)
 
 
-'''
-@params compressed: filename, Asset, or file-like object to uncompress
-@params uncompressed: filename to uncompress into
-@params format: optional compression format (gzip, lzma)
+def uncompress(compressed, uncompressed, format=None):
+    '''
+    @params compressed: filename, Asset, or file-like object to uncompress
+    @params uncompressed: filename to uncompress into
+    @params format: optional compression format (gzip, lzma)
 
-Uncompresses @compressed into @uncompressed
+    Uncompresses @compressed into @uncompressed
 
-If @format is None, heuristics will be applied to guess the format
-from the filename or Asset URL. @format must be non-None if @uncompressed
-is a file-like object.
+    If @format is None, heuristics will be applied to guess the
+    format from the filename or Asset URL. @format must be non-None
+    if @uncompressed is a file-like object.
 
-Returns the fully qualified path to the uncompessed file
-'''
-def uncompress(compressed, uncompressed, format=None):
+    Returns the fully qualified path to the uncompessed file
+    '''
     if format is None:
         format = guess_uncompress_format(compressed)
 
@@ -84,19 +84,19 @@ def uncompress(compressed, uncompressed, format=None):
     else:
         raise Exception(f"Unknown compression format {format}")
 
-'''
-@params compressed: filename, Asset, or file-like object to guess
-
-Guess the format of @compressed, raising an exception if
-no format can be determined
-'''
 def guess_uncompress_format(compressed):
-    if type(compressed) == Asset:
+    '''
+    @params compressed: filename, Asset, or file-like object to guess
+
+    Guess the format of @compressed, raising an exception if
+    no format can be determined
+    '''
+    if isinstance(compressed, Asset):
         compressed = urlparse(compressed.url).path
-    elif type(compressed) != str:
+    elif not isinstance(compressed, str):
         raise Exception(f"Unable to guess compression cformat for {compressed}")
 
-    (name, ext) = os.path.splitext(compressed)
+    (_name, ext) = os.path.splitext(compressed)
     if ext == ".xz":
         return "xz"
     elif ext == ".gz":
-- 
2.51.0

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

[Thomas Huth]

From: Thomas Huth <thuth@redhat.com>

- put the doc strings in the right locations (after the "def" line)
- use the right indentation (4 spaces)

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

diff --git a/tests/functional/qemu_test/utils.py b/tests/functional/qemu_test/utils.py
index e7c8de81654..826c267785b 100644
--- a/tests/functional/qemu_test/utils.py
+++ b/tests/functional/qemu_test/utils.py
@@ -17,10 +17,10 @@ def get_usernet_hostfwd_port(vm):
     res = vm.cmd('human-monitor-command', command_line='info usernet')
     return get_info_usernet_hostfwd_port(res)
 
-"""
-Round up to next power of 2
-"""
 def pow2ceil(x):
+    """
+    Round up to next power of 2
+    """
     return 1 if x == 0 else 2**(x - 1).bit_length()
 
 def file_truncate(path, size):
@@ -28,12 +28,12 @@ def file_truncate(path, size):
         with open(path, 'ab+') as fd:
             fd.truncate(size)
 
-"""
-Expand file size to next power of 2
-"""
 def image_pow2ceil_expand(path):
-        size = os.path.getsize(path)
-        size_aligned = pow2ceil(size)
-        if size != size_aligned:
-            with open(path, 'ab+') as fd:
-                fd.truncate(size_aligned)
+    """
+    Expand file size to next power of 2
+    """
+    size = os.path.getsize(path)
+    size_aligned = pow2ceil(size)
+    if size != size_aligned:
+        with open(path, 'ab+') as fd:
+            fd.truncate(size_aligned)
-- 
2.51.0

Re: [PATCH 5/6] tests/functional: Fix problems in uncompress.py reported by pylint

[Philippe Mathieu-Daudé]

On 15/10/25 11:54, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> - put the doc strings in the right locations (after the "def" line)
> - use isinstance() instead of checking via type()
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/functional/qemu_test/uncompress.py | 40 ++++++++++++------------
>   1 file changed, 20 insertions(+), 20 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

[Philippe Mathieu-Daudé]

On 15/10/25 11:54, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> - put the doc strings in the right locations (after the "def" line)
> - use the right indentation (4 spaces)
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/functional/qemu_test/utils.py | 22 +++++++++++-----------
>   1 file changed, 11 insertions(+), 11 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 4/6] tests/functional: Fix problems in testcase.py reported by pylint

[Philippe Mathieu-Daudé]

On 15/10/25 11:54, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> - put 3rd party "import pycotap" after the standard imports
> - "help" is a built-in function in Python, don't use it as a variable name
> - put the doc strings in the right locations (after the "def" line)
> - use isinstance() instead of checking via type()
> - use lazy logging strings
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/functional/qemu_test/testcase.py | 251 +++++++++++++------------
>   1 file changed, 126 insertions(+), 125 deletions(-)


> -    def run_cmd(self, bin_path, args=[]):
> +    def run_cmd(self, bin_path, args=None):
> +        if args is None:
> +            args = []

For my own education, what is the issue reported here?

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

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

[Philippe Mathieu-Daudé]

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 2/6] tests/functional: Fix problems in decorators.py reported by pylint

[Philippe Mathieu-Daudé]

On 15/10/25 11:54, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> The documentation strings should follow the function definition
> lines, not precede them.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/functional/qemu_test/decorators.py | 176 +++++++++++------------
>   1 file changed, 87 insertions(+), 89 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 4/6] tests/functional: Fix problems in testcase.py reported by pylint

[Thomas Huth]

On 22/10/2025 21.10, Philippe Mathieu-Daudé wrote:
> On 15/10/25 11:54, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> - put 3rd party "import pycotap" after the standard imports
>> - "help" is a built-in function in Python, don't use it as a variable name
>> - put the doc strings in the right locations (after the "def" line)
>> - use isinstance() instead of checking via type()
>> - use lazy logging strings
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/functional/qemu_test/testcase.py | 251 +++++++++++++------------
>>   1 file changed, 126 insertions(+), 125 deletions(-)
> 
> 
>> -    def run_cmd(self, bin_path, args=[]):
>> +    def run_cmd(self, bin_path, args=None):
>> +        if args is None:
>> +            args = []
> 
> For my own education, what is the issue reported here?

https://pylint.pycqa.org/en/latest/user_guide/messages/warning/dangerous-default-value.html

> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!

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

[Thomas Huth]

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

[John Snow]

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
>