[PATCH v2 0/3] tests/functional: improve handling flaky downloads

[PATCH v2 0/3] tests/functional: improve handling flaky downloads

[Daniel P. Berrangé]

This fixes the functional-system-debian job with the currently
awol archive.openwrt.org server.

  https://gitlab.com/berrange/qemu/-/jobs/11173491503

We can see the download fail & retry, and then gracefully
skip pre-cache:

2025-08-29 15:10:11,242 - qemu-test - INFO - Attempting to cache '/var/home/berrange/.cache/qemu/download/d3a01037f33e7512d46d50975588d5c3a0e0cbf25f37afab44775c2a2be523e6'
2025-08-29 15:10:11,242 - qemu-test - INFO - Downloading https://archive.openwrt.org/chaos_calmer/15.05.1/realview/generic/openwrt-15.05.1-realview-vmlinux-initramfs.elf to /var/home/berrange/.cache/qemu/download/d3a01037f33e7512d46d50975588d5c3a0e0cbf25f37afab44775c2a2be523e6...
2025-08-29 15:12:24,116 - qemu-test - ERROR - Unable to download https://archive.openwrt.org/chaos_calmer/15.05.1/realview/generic/openwrt-15.05.1-realview-vmlinux-initramfs.elf: URL error [Errno 110] Connection timed out
2025-08-29 15:12:24,116 - qemu-test - ERROR - https://archive.openwrt.org/chaos_calmer/15.05.1/realview/generic/openwrt-15.05.1-realview-vmlinux-initramfs.elf: Unable to download: URL error [Errno 110] Connection timed out: skipping asset precache

And then later the test itself is skipped:

55/67 qemu:func-thorough+func-arm-thorough+thorough / func-arm-realview            SKIP              0.11s   0 subtests passed

Changed in v2:

 - Simplifiy patch 2 - adding the URL was NOT in fact needed,
   as AssetError adds that automatically
 - Avoid retries on URLError failures

Daniel P. Berrangé (3):
  tests/functional: enable force refresh of cached assets
  tests/functional: fix formatting of exception args
  tests/functional: handle URLError when fetching assets

 docs/devel/testing/functional.rst   |  3 +++
 tests/functional/qemu_test/asset.py | 16 ++++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

-- 
2.50.1

[PATCH v2 1/3] tests/functional: enable force refresh of cached assets

[Daniel P. Berrangé]

If the 'QEMU_TEST_REFRESH_CACHE' environment variable is set, then
ignore any existing cached asset and download a fresh copy.

This can be used to selectively refresh assets if set before running
a single test script.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 docs/devel/testing/functional.rst   | 3 +++
 tests/functional/qemu_test/asset.py | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/docs/devel/testing/functional.rst b/docs/devel/testing/functional.rst
index 3728bab6c0..fdeaebaadc 100644
--- a/docs/devel/testing/functional.rst
+++ b/docs/devel/testing/functional.rst
@@ -312,6 +312,9 @@ The cache is populated in the ``~/.cache/qemu/download`` directory by
 default, but the location can be changed by setting the
 ``QEMU_TEST_CACHE_DIR`` environment variable.
 
+To force the test suite to re-download the cache, even if still valid,
+set the ``QEMU_TEST_REFRESH_CACHE`` environment variable.
+
 Skipping tests
 --------------
 
diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
index 704b84d0ea..b5a6136d36 100644
--- a/tests/functional/qemu_test/asset.py
+++ b/tests/functional/qemu_test/asset.py
@@ -72,6 +72,10 @@ def _check(self, cache_file):
         return self.hash == hl.hexdigest()
 
     def valid(self):
+        if os.getenv("QEMU_TEST_REFRESH_CACHE", None) is not None:
+            self.log.info("Force refresh of asset %s", self.url)
+            return False
+
         return self.cache_file.exists() and self._check(self.cache_file)
 
     def fetchable(self):
-- 
2.50.1

[PATCH v2 2/3] tests/functional: fix formatting of exception args

[Daniel P. Berrangé]

The catch-all exception handler forgot the placeholder for
the exception details.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/qemu_test/asset.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
index b5a6136d36..5c74adf224 100644
--- a/tests/functional/qemu_test/asset.py
+++ b/tests/functional/qemu_test/asset.py
@@ -173,7 +173,7 @@ def fetch(self):
                 continue
             except Exception as e:
                 tmp_cache_file.unlink()
-                raise AssetError(self, "Unable to download: " % e)
+                raise AssetError(self, "Unable to download: %s" % e)
 
         if not os.path.exists(tmp_cache_file):
             raise AssetError(self, "Download retries exceeded", transient=True)
-- 
2.50.1

[PATCH v2 3/3] tests/functional: handle URLError when fetching assets

[Daniel P. Berrangé]

We treat most HTTP errors as non-fatal when fetching assets,
but forgot to handle network level errors. This adds catching
of URLError so that we retry on failure, and will ultimately
trigger graceful skipping in the pre-cache task.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/functional/qemu_test/asset.py | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
index 5c74adf224..2dd32bf28d 100644
--- a/tests/functional/qemu_test/asset.py
+++ b/tests/functional/qemu_test/asset.py
@@ -15,7 +15,7 @@
 from time import sleep
 from pathlib import Path
 from shutil import copyfileobj
-from urllib.error import HTTPError
+from urllib.error import HTTPError, URLError
 
 class AssetError(Exception):
     def __init__(self, asset, msg, transient=False):
@@ -171,6 +171,14 @@ def fetch(self):
                     raise AssetError(self, "Unable to download: "
                                      "HTTP error %d" % e.code)
                 continue
+            except URLError as e:
+                # This is typically a network/service level error
+                # eg urlopen error [Errno 110] Connection timed out>
+                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)
             except Exception as e:
                 tmp_cache_file.unlink()
                 raise AssetError(self, "Unable to download: %s" % e)
-- 
2.50.1

Re: [PATCH v2 3/3] tests/functional: handle URLError when fetching assets

[Thomas Huth]

On 29/08/2025 16.26, Daniel P. Berrangé wrote:
> We treat most HTTP errors as non-fatal when fetching assets,
> but forgot to handle network level errors. This adds catching
> of URLError so that we retry on failure, and will ultimately
> trigger graceful skipping in the pre-cache task.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/asset.py | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v2 2/3] tests/functional: fix formatting of exception args

[Thomas Huth]

On 29/08/2025 16.26, Daniel P. Berrangé wrote:
> The catch-all exception handler forgot the placeholder for
> the exception details.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/asset.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
> index b5a6136d36..5c74adf224 100644
> --- a/tests/functional/qemu_test/asset.py
> +++ b/tests/functional/qemu_test/asset.py
> @@ -173,7 +173,7 @@ def fetch(self):
>                   continue
>               except Exception as e:
>                   tmp_cache_file.unlink()
> -                raise AssetError(self, "Unable to download: " % e)
> +                raise AssetError(self, "Unable to download: %s" % e)
>   
>           if not os.path.exists(tmp_cache_file):
>               raise AssetError(self, "Download retries exceeded", transient=True)

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH v2 1/3] tests/functional: enable force refresh of cached assets

[Richard Henderson]

On 8/30/25 00:26, Daniel P. Berrangé wrote:
> If the 'QEMU_TEST_REFRESH_CACHE' environment variable is set, then
> ignore any existing cached asset and download a fresh copy.
> 
> This can be used to selectively refresh assets if set before running
> a single test script.
> 
> Reviewed-by: Thomas Huth<thuth@redhat.com>
> Signed-off-by: Daniel P. Berrangé<berrange@redhat.com>
> ---
>   docs/devel/testing/functional.rst   | 3 +++
>   tests/functional/qemu_test/asset.py | 4 ++++
>   2 files changed, 7 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v2 2/3] tests/functional: fix formatting of exception args

[Richard Henderson]

On 8/30/25 00:26, Daniel P. Berrangé wrote:
> The catch-all exception handler forgot the placeholder for
> the exception details.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   tests/functional/qemu_test/asset.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
> index b5a6136d36..5c74adf224 100644
> --- a/tests/functional/qemu_test/asset.py
> +++ b/tests/functional/qemu_test/asset.py
> @@ -173,7 +173,7 @@ def fetch(self):
>                   continue
>               except Exception as e:
>                   tmp_cache_file.unlink()
> -                raise AssetError(self, "Unable to download: " % e)
> +                raise AssetError(self, "Unable to download: %s" % e)
>   
>           if not os.path.exists(tmp_cache_file):
>               raise AssetError(self, "Download retries exceeded", transient=True)

Isn't this exactly why f-strings are better, because you can't mess up the placeholders?

Anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH v2 3/3] tests/functional: handle URLError when fetching assets

[Richard Henderson]

On 8/30/25 00:26, Daniel P. Berrangé wrote:
> We treat most HTTP errors as non-fatal when fetching assets,
> but forgot to handle network level errors. This adds catching
> of URLError so that we retry on failure, and will ultimately
> trigger graceful skipping in the pre-cache task.
> 
> Signed-off-by: Daniel P. Berrangé<berrange@redhat.com>
> ---
>   tests/functional/qemu_test/asset.py | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~