* [PATCH 1/4] tests/functional/qemu_test: Silence warnings from pylint in tesseract.py
2026-03-24 16:35 [PATCH 0/4] Silence some pylint warnings in tests/functional/qemu_test/ Thomas Huth
@ 2026-03-24 16:35 ` Thomas Huth
2026-03-25 10:01 ` Philippe Mathieu-Daudé
2026-03-24 16:35 ` [PATCH 2/4] tests/functional/qemu_test: Silence (most) warnings from pylint in asset.py Thomas Huth
` (2 subsequent siblings)
3 siblings, 1 reply; 8+ messages in thread
From: Thomas Huth @ 2026-03-24 16:35 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Philippe Mathieu-Daudé, Thomas Huth
From: Thomas Huth <thuth@redhat.com>
Pylint complains:
tesseract.py:1:0: C0114: Missing module docstring (missing-module-docstring)
tesseract.py:12:0: C0116: Missing function or method docstring (missing-function-docstring)
tesseract.py:15:11: W1510: 'subprocess.run' used without explicitly defining the value for 'check'. (subprocess-run-check)
tesseract.py:12:30: W0613: Unused argument 'tesseract_args' (unused-argument)
Thus add the missing bits and remove the unused tesseract_args argument.
While we're at it, also add a SPDX identifier instead of the weird three
dots at the beginning of the file.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/functional/qemu_test/tesseract.py | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tests/functional/qemu_test/tesseract.py b/tests/functional/qemu_test/tesseract.py
index ede6c6501e2..1f955d32b76 100644
--- a/tests/functional/qemu_test/tesseract.py
+++ b/tests/functional/qemu_test/tesseract.py
@@ -1,19 +1,24 @@
-# ...
+# SPDX-License-Identifier: GPL-2.0-or-later
#
# Copyright (c) 2019 Philippe Mathieu-Daudé <f4bug@amsat.org>
#
# This work is licensed under the terms of the GNU GPL, version 2 or
# later. See the COPYING file in the top-level directory.
+'''
+Tesseract is an program for doing Optical Character Recognition (OCR),
+which can be used in the tests to extract text from screenshots.
+'''
import logging
from subprocess import run
-def tesseract_ocr(image_path, tesseract_args=''):
+def tesseract_ocr(image_path):
+ ''' Run the tesseract OCR to extract text from a screenshot '''
console_logger = logging.getLogger('console')
console_logger.debug(image_path)
proc = run(['tesseract', image_path, 'stdout'],
- capture_output=True, encoding='utf8')
+ capture_output=True, encoding='utf8', check=False)
if proc.returncode:
return None
lines = []
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/4] tests/functional/qemu_test: Silence (most) warnings from pylint in asset.py
2026-03-24 16:35 [PATCH 0/4] Silence some pylint warnings in tests/functional/qemu_test/ Thomas Huth
2026-03-24 16:35 ` [PATCH 1/4] tests/functional/qemu_test: Silence warnings from pylint in tesseract.py Thomas Huth
@ 2026-03-24 16:35 ` Thomas Huth
2026-03-24 16:35 ` [PATCH 3/4] tests/functional/qemu_test: Split huge fetch() function " Thomas Huth
2026-03-24 16:35 ` [PATCH 4/4] tests/functional/qemu_test: Silence warnings from pylint in config.py Thomas Huth
3 siblings, 0 replies; 8+ messages in thread
From: Thomas Huth @ 2026-03-24 16:35 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Philippe Mathieu-Daudé, Thomas Huth
From: Thomas Huth <thuth@redhat.com>
Pylint currently complains:
asset.py:1:0: C0114: Missing module docstring (missing-module-docstring)
asset.py:21:0: C0115: Missing class docstring (missing-class-docstring)
asset.py:28:15: C0209: Formatting a regular string which could be an f-string (consider-using-f-string)
asset.py:34:0: C0115: Missing class docstring (missing-class-docstring)
asset.py:49:15: C0209: Formatting a regular string which could be an f-string (consider-using-f-string)
asset.py:73:4: C0116: Missing function or method docstring (missing-function-docstring)
asset.py:80:4: C0116: Missing function or method docstring (missing-function-docstring)
asset.py:83:4: C0116: Missing function or method docstring (missing-function-docstring)
asset.py:125:4: C0116: Missing function or method docstring (missing-function-docstring)
asset.py:181:43: C0209: Formatting a regular string which could be an f-string (consider-using-f-string)
asset.py:190:39: C0209: Formatting a regular string which could be an f-string (consider-using-f-string)
asset.py:201:39: C0209: Formatting a regular string which could be an f-string (consider-using-f-string)
asset.py:213:15: W0718: Catching too general exception Exception (broad-exception-caught)
asset.py:218:35: C0209: Formatting a regular string which could be an f-string (consider-using-f-string)
asset.py:125:4: R0912: Too many branches (16/12) (too-many-branches)
asset.py:125:4: R0915: Too many statements (64/50) (too-many-statements)
asset.py:228:4: C0116: Missing function or method docstring (missing-function-docstring)
asset.py:249:4: C0116: Missing function or method docstring (missing-function-docstring)
asset.py:257:4: C0116: Missing function or method docstring (missing-function-docstring)
Fix all the warnings except for the R0912 and R0915 which will be tackled
in a later commit.
And while we're at it, also add a proper SPDX license identifier.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/functional/qemu_test/asset.py | 53 +++++++++++++++++++++--------
1 file changed, 38 insertions(+), 15 deletions(-)
diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
index 45a2e01e2e6..1cd03c2a9a9 100644
--- a/tests/functional/qemu_test/asset.py
+++ b/tests/functional/qemu_test/asset.py
@@ -1,9 +1,12 @@
-# Test utilities for fetching & caching assets
+# SPDX-License-Identifier: GPL-2.0-or-later
#
# Copyright 2024 Red Hat, Inc.
#
# This work is licensed under the terms of the GNU GPL, version 2 or
# later. See the COPYING file in the top-level directory.
+'''
+Test utilities for fetching & caching assets
+'''
import hashlib
import logging
@@ -18,21 +21,27 @@
from shutil import copyfileobj
from urllib.error import HTTPError, URLError
+
class AssetError(Exception):
+ '''This exception will be raised if an asset is not usable'''
def __init__(self, asset, msg, transient=False):
self.url = asset.url
self.msg = msg
self.transient = transient
def __str__(self):
- return "%s: %s" % (self.url, self.msg)
+ return f"{self.url}: {self.msg}"
-# Instances of this class must be declared as class level variables
-# starting with a name "ASSET_". This enables the pre-caching logic
-# to easily find all referenced assets and download them prior to
-# execution of the tests.
-class Asset:
+class Asset:
+ '''
+ This class is used to represent an asset that gets downloaded from
+ the internet and will be stored in the local asset cache.
+ Instances of this class must be declared as class level variables
+ starting with a name "ASSET_". This enables the pre-caching logic
+ to easily find all referenced assets and download them prior to
+ execution of the tests.
+ '''
def __init__(self, url, hashsum):
self.url = url
self.hash = hashsum
@@ -46,8 +55,7 @@ def __init__(self, url, hashsum):
self.log = logging.getLogger('qemu-test')
def __repr__(self):
- return "Asset: url=%s hash=%s cache=%s" % (
- self.url, self.hash, self.cache_file)
+ return f"Asset: url={self.url} hash={self.hash} cache={self.cache_file}"
def __str__(self):
return str(self.cache_file)
@@ -71,6 +79,7 @@ def _check(self, cache_file):
return self.hash == hl.hexdigest()
def valid(self):
+ '''Check whether the file exists in the cache and has the right hash'''
if os.getenv("QEMU_TEST_REFRESH_CACHE", None) is not None:
self.log.info("Force refresh of asset %s", self.url)
return False
@@ -78,9 +87,11 @@ def valid(self):
return self.cache_file.exists() and self._check(self.cache_file)
def fetchable(self):
+ '''Check whether we are allowed to download assets from the internet'''
return not os.environ.get("QEMU_TEST_NO_DOWNLOAD", False)
def available(self):
+ '''Check whether the asset is either in the cache or fetchable'''
return self.valid() or self.fetchable()
def _wait_for_other_download(self, tmp_cache_file):
@@ -123,6 +134,7 @@ def _save_time_stamp(self):
self.cache_file.with_suffix(".stamp").write_text(f"{int(time.time())}")
def fetch(self):
+ '''Download the asset from the internet'''
if not self.cache_dir.exists():
self.cache_dir.mkdir(parents=True, exist_ok=True)
@@ -179,7 +191,7 @@ def fetch(self):
# server or networking problem
if e.code == 404:
raise AssetError(self, "Unable to download: "
- "HTTP error %d" % e.code) from e
+ f"HTTP error {e.code}") from e
continue
except URLError as e:
# This is typically a network/service level error
@@ -187,8 +199,9 @@ def fetch(self):
tmp_cache_file.unlink()
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) from e
+ raise AssetError(self,
+ f"Unable to download: URL error{e.reason}",
+ transient=True) from e
except ConnectionError as e:
# A socket connection failure, such as dropped conn
# or refused conn
@@ -198,7 +211,7 @@ def fetch(self):
continue
except Exception as e:
tmp_cache_file.unlink()
- raise AssetError(self, "Unable to download: %s" % e,
+ raise AssetError(self, f"Unable to download: {e}",
transient=True) from e
if not os.path.exists(tmp_cache_file):
@@ -210,12 +223,12 @@ def fetch(self):
self.url.encode('utf8'))
os.setxattr(str(tmp_cache_file), "user.qemu-asset-hash",
self.hash.encode('utf8'))
- except Exception as e:
+ except OSError as e:
self.log.debug("Unable to set xattr on %s: %s", tmp_cache_file, e)
if not self._check(tmp_cache_file):
tmp_cache_file.unlink()
- raise AssetError(self, "Hash does not match %s" % self.hash)
+ raise AssetError(self, f"Hash does not match {self.hash}")
tmp_cache_file.replace(self.cache_file)
self._save_time_stamp()
# Remove write perms to stop tests accidentally modifying them
@@ -226,6 +239,10 @@ def fetch(self):
@staticmethod
def precache_test(test):
+ '''
+ Look for variables starting with "ASSET_" and try to fetch the asset
+ that is specified there.
+ '''
log = logging.getLogger('qemu-test')
log.setLevel(logging.DEBUG)
handler = logging.StreamHandler(sys.stdout)
@@ -247,6 +264,9 @@ def precache_test(test):
@staticmethod
def precache_suite(suite):
+ '''
+ Iterate through all tests/suites in a suite and precache their assets
+ '''
for test in suite:
if isinstance(test, unittest.TestSuite):
Asset.precache_suite(test)
@@ -255,6 +275,9 @@ def precache_suite(suite):
@staticmethod
def precache_suites(path, cache_tstamp):
+ '''
+ Get the available test suite and precache their assets
+ '''
loader = unittest.loader.defaultTestLoader
tests = loader.loadTestsFromNames([path], None)
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 3/4] tests/functional/qemu_test: Split huge fetch() function in asset.py
2026-03-24 16:35 [PATCH 0/4] Silence some pylint warnings in tests/functional/qemu_test/ Thomas Huth
2026-03-24 16:35 ` [PATCH 1/4] tests/functional/qemu_test: Silence warnings from pylint in tesseract.py Thomas Huth
2026-03-24 16:35 ` [PATCH 2/4] tests/functional/qemu_test: Silence (most) warnings from pylint in asset.py Thomas Huth
@ 2026-03-24 16:35 ` Thomas Huth
2026-03-25 10:04 ` Philippe Mathieu-Daudé
2026-03-24 16:35 ` [PATCH 4/4] tests/functional/qemu_test: Silence warnings from pylint in config.py Thomas Huth
3 siblings, 1 reply; 8+ messages in thread
From: Thomas Huth @ 2026-03-24 16:35 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Philippe Mathieu-Daudé, Thomas Huth
From: Thomas Huth <thuth@redhat.com>
The fetch() function has become really huge and pylint complains about
that. Extract the internal retry-three-times-download loop into a
separate function to make it a little bit more readable and to make
pylint happy about this file again.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/functional/qemu_test/asset.py | 43 ++++++++++++++++-------------
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
index 1cd03c2a9a9..51a434b2b70 100644
--- a/tests/functional/qemu_test/asset.py
+++ b/tests/functional/qemu_test/asset.py
@@ -133,24 +133,7 @@ def _save_time_stamp(self):
'''
self.cache_file.with_suffix(".stamp").write_text(f"{int(time.time())}")
- def fetch(self):
- '''Download the asset from the internet'''
- if not self.cache_dir.exists():
- self.cache_dir.mkdir(parents=True, exist_ok=True)
-
- if self.valid():
- self.log.debug("Using cached asset %s for %s",
- self.cache_file, self.url)
- self._save_time_stamp()
- return str(self.cache_file)
-
- if not self.fetchable():
- raise AssetError(self,
- "Asset cache is invalid and downloads disabled")
-
- self.log.info("Downloading %s to %s...", self.url, self.cache_file)
- tmp_cache_file = self.cache_file.with_suffix(".download")
-
+ def _try_to_fetch(self, tmp_cache_file):
for _retries in range(3):
try:
with tmp_cache_file.open("xb") as dst:
@@ -176,7 +159,7 @@ def fetch(self):
"waiting for other thread to finish...",
tmp_cache_file)
if self._wait_for_other_download(tmp_cache_file):
- return str(self.cache_file)
+ return True
self.log.debug("%s seems to be stale, "
"deleting and retrying download...",
tmp_cache_file)
@@ -213,6 +196,28 @@ def fetch(self):
tmp_cache_file.unlink()
raise AssetError(self, f"Unable to download: {e}",
transient=True) from e
+ return False
+
+ def fetch(self):
+ '''Download the asset from the internet'''
+ if not self.cache_dir.exists():
+ self.cache_dir.mkdir(parents=True, exist_ok=True)
+
+ if self.valid():
+ self.log.debug("Using cached asset %s for %s",
+ self.cache_file, self.url)
+ self._save_time_stamp()
+ return str(self.cache_file)
+
+ if not self.fetchable():
+ raise AssetError(self,
+ "Asset cache is invalid and downloads disabled")
+
+ self.log.info("Downloading %s to %s...", self.url, self.cache_file)
+ tmp_cache_file = self.cache_file.with_suffix(".download")
+
+ if self._try_to_fetch(tmp_cache_file):
+ return str(self.cache_file)
if not os.path.exists(tmp_cache_file):
raise AssetError(self, "Download retries exceeded", transient=True)
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 4/4] tests/functional/qemu_test: Silence warnings from pylint in config.py
2026-03-24 16:35 [PATCH 0/4] Silence some pylint warnings in tests/functional/qemu_test/ Thomas Huth
` (2 preceding siblings ...)
2026-03-24 16:35 ` [PATCH 3/4] tests/functional/qemu_test: Split huge fetch() function " Thomas Huth
@ 2026-03-24 16:35 ` Thomas Huth
2026-03-25 10:05 ` Philippe Mathieu-Daudé
3 siblings, 1 reply; 8+ messages in thread
From: Thomas Huth @ 2026-03-24 16:35 UTC (permalink / raw)
To: qemu-devel
Cc: Daniel P. Berrangé, Philippe Mathieu-Daudé, Thomas Huth
From: Thomas Huth <thuth@redhat.com>
Pylint complains here:
config.py:1:0: C0114: Missing module docstring (missing-module-docstring)
config.py:28:4: W0719: Raising too general exception: Exception (broad-exception-raised)
Add a module description and replace the general Exception to fix this.
And while we're at it, and since we've got a proper module description
string now, also replace the copy-n-pasted comment at the top of the file
with a proper SPDX identifier.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
tests/functional/qemu_test/config.py | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/tests/functional/qemu_test/config.py b/tests/functional/qemu_test/config.py
index e0893f630ee..5d44b6fa4c0 100644
--- a/tests/functional/qemu_test/config.py
+++ b/tests/functional/qemu_test/config.py
@@ -1,4 +1,4 @@
-# Test class and utilities for functional tests
+# SPDX-License-Identifier: GPL-2.0-or-later
#
# Copyright 2018, 2024 Red Hat, Inc.
#
@@ -10,6 +10,9 @@
#
# This work is licensed under the terms of the GNU GPL, version 2 or
# later. See the COPYING file in the top-level directory.
+'''
+Functions related to the configuration of the tests and of the host system
+'''
import os
from pathlib import Path
@@ -25,9 +28,9 @@ def _build_dir():
if root is not None:
return Path(root)
- raise Exception("Missing MESON_BUILD_ROOT environment variable. " +
- "Please use the '<BUILD-DIR>/run' script if invoking " +
- "directly instead of via make/meson")
+ raise RuntimeError("Missing MESON_BUILD_ROOT environment variable. " +
+ "Please use the '<BUILD-DIR>/run' script if invoking " +
+ "directly instead of via make/meson")
BUILD_DIR = _build_dir()
--
2.53.0
^ permalink raw reply related [flat|nested] 8+ messages in thread