qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 18/22] tests/functional: add 'archive_extract' to QemuBaseTes


From: Thomas Huth
Subject: Re: [PATCH 18/22] tests/functional: add 'archive_extract' to QemuBaseTest
Date: Fri, 6 Dec 2024 14:10:32 +0100
User-agent: Mozilla Thunderbird

On 02/12/2024 13.52, Thomas Huth wrote:
On 02/12/2024 13.13, Daniel P. Berrangé wrote:
On Mon, Dec 02, 2024 at 11:30:28AM +0100, Thomas Huth wrote:
On 29/11/2024 18.31, Daniel P. Berrangé wrote:
This helper wrappers utils.archive_extract, forcing the use of the
scratch directory, to ensure any extracted files are cleaned at test
termination. If a specific member is requested, then the path to the
extracted file is also returned.

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

diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/ qemu_test/testcase.py
index 2f32742387..31d06f0172 100644
--- a/tests/functional/qemu_test/testcase.py
+++ b/tests/functional/qemu_test/testcase.py
@@ -28,6 +28,8 @@
   from .asset import Asset
   from .cmd import run_cmd
   from .config import BUILD_DIR
+from .utils import (archive_extract as utils_archive_extract,
+                    guess_archive_format)
   class QemuBaseTest(unittest.TestCase):
@@ -39,6 +41,40 @@ class QemuBaseTest(unittest.TestCase):
       log = None
       logdir = None
+    '''
+    @params archive: filename, Asset, or file-like object to extract
+    @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 @member is non-None, returns the fully qualified
+    path to @member
+    '''
+    def archive_extract(self, archive, format=None, sub_dir=None, member=None):
+        if type(archive) == Asset:
+            if format is None:
+                format = guess_archive_format(archive.url)
+            archive = archive.fetch()
+        elif format is None:
+            format = guess_archive_format(archive)
+
+        if member is not None:
+            if os.path.isabs(member):
+                member = os.path.relpath(member, '/')
+
+        if sub_dir is None:
+            utils_archive_extract(archive, self.scratch_file(), format, member)
+        else:
+            utils_archive_extract(archive, self.scratch_file(sub_dir),
+                                  format, member)
+
+        if member is not None:
+            return self.scratch_file(member)
+        return None

Ah, ok, so the guessing is done here ...

But somehow it feels wrong to have a "archive_extract" function in the
QemuBaseTest class that also does asset fetching under the hood.

Could you maybe rather move this into the asset.py file and rename the
function to "fetch_and_extract()" to make it clearer what it does?

We can't move it into asset.py because not all callers are passing in an
Asset object - there are some cases where we've just got a local file.
eg when the asset we extracted contains other archives that need to be
extracted.

Couldn't those spots rather use the function from utils.py directly for the second extraction?

Anyway, I guess now it's likely better to keep it here since you want to have access to self.scratch_file() in this function, too.

FWIW, I think I just hit another reason for having the function in testcase.py: We want to skip tests if the download of an asset failed with a temporary server failure - for this we need a test object reference for calling skipTest(). Checking for HTTP errors within assets.py does not work since we cannot call skipTest() there without a reference to a test object. So please keep the function in testcase.py!

 Thomas




reply via email to

[Prev in Thread] Current Thread [Next in Thread]