Message ID | 20240821082748.65853-15-thuth@redhat.com |
---|---|
State | New |
Headers | show |
Series | Convert avocado tests to normal Python unittests | expand |
On 21/8/24 10:27, Thomas Huth wrote: > From: Daniel P. Berrangé <berrange@redhat.com> > > The 'Asset' class is a simple module that declares a downloadable > asset that can be cached locally. Downloads are stored in the user's > home dir at ~/.cache/qemu/download, using a sha256 sum of the URL. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org> > [thuth: Drop sha1 support, use hash on file content for naming instead of URL, > add the possibility to specify the cache dir via environment variable] > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > tests/functional/qemu_test/__init__.py | 1 + > tests/functional/qemu_test/asset.py | 97 ++++++++++++++++++++++++++ > 2 files changed, 98 insertions(+) > create mode 100644 tests/functional/qemu_test/asset.py > > diff --git a/tests/functional/qemu_test/__init__.py b/tests/functional/qemu_test/__init__.py > index 2f1e0bc70d..db05c8f412 100644 > --- a/tests/functional/qemu_test/__init__.py > +++ b/tests/functional/qemu_test/__init__.py > @@ -6,6 +6,7 @@ > # later. See the COPYING file in the top-level directory. > > > +from .asset import Asset > from .config import BUILD_DIR > from .cmd import has_cmd, has_cmds, run_cmd, is_readable_executable_file, \ > interrupt_interactive_console_until_pattern, wait_for_console_pattern, \ > diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py > new file mode 100644 > index 0000000000..cbeb6278af > --- /dev/null > +++ b/tests/functional/qemu_test/asset.py > @@ -0,0 +1,97 @@ > +# Test utilities for fetching & caching assets > +# > +# 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. > + > +import hashlib > +import logging > +import os > +import subprocess > +import urllib.request > +from pathlib import Path > +from shutil import copyfileobj > + > + > +# 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: > + > + def __init__(self, url, hashsum): > + self.url = url > + self.hash = hashsum > + cache_dir_env = os.getenv('QEMU_TEST_CACHE_DIR') > + if cache_dir_env: > + self.cache_dir = Path(cache_dir_env, "download") > + else: > + self.cache_dir = Path(Path("~").expanduser(), > + ".cache", "qemu", "download") > + self.cache_file = Path(self.cache_dir, 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) > + > + def _check(self, cache_file): > + if self.hash is None: > + return True > + if len(self.hash) == 64: > + sum_prog = 'sha256sum' > + elif len(self.hash) == 128: > + sum_prog = 'sha512sum' > + else: > + raise Exception("unknown hash type") > + > + checksum = subprocess.check_output( > + [sum_prog, str(cache_file)]).split()[0] > + return self.hash == checksum.decode("utf-8") > + > + def valid(self): > + return self.cache_file.exists() and self._check(self.cache_file) > + > + def fetch(self): > + if not self.cache_dir.exists(): > + self.cache_dir.mkdir(parents=True, exist_ok=True) This doesn't work with QEMU_TEST_CACHE_DIR set to someone else: File "/home/philippe.mathieu-daude/qemu/tests/functional/qemu_test/asset.py", line 60, in fetch self.cache_dir.mkdir(parents=True, exist_ok=True) File "/usr/lib/python3.10/pathlib.py", line 1175, in mkdir self._accessor.mkdir(self, mode) PermissionError: [Errno 13] Permission denied: '/home/alex.bennee/.cache/qemu/download' ninja: build stopped: subcommand failed. Maybe use a getter which falls back to Path("~").expanduser() when no access on QEMU_TEST_CACHE_DIR? This happens when downloading a new file (the recent MIPS tests I converted) which isn't in Alex's cache: 2024-08-21 15:45:48,896 - qemu-test - INFO - Attempting to cache 'Asset: url=https://s3-eu-west-1.amazonaws.com/downloads-mips/mips-downloads/YAMON/yamon-bin-02.22.zip hash=eef86f0eed0ef554f041dcd47b87eebea0e6f9f1184ed31f7e9e8b4a803860ab cache=/home/alex.bennee/.cache/download/eef86f0eed0ef554f041dcd47b87eebea0e6f9f1184ed31f7e9e8b4a803860ab' > + if self.valid(): > + self.log.debug("Using cached asset %s for %s", > + self.cache_file, self.url) > + return str(self.cache_file) > + > + self.log.info("Downloading %s to %s...", self.url, self.cache_file) > + tmp_cache_file = self.cache_file.with_suffix(".download") > + > + try: > + resp = urllib.request.urlopen(self.url) > + except Exception as e: > + self.log.error("Unable to download %s: %s", self.url, e) > + raise > + > + try: > + with tmp_cache_file.open("wb+") as dst: > + copyfileobj(resp, dst) > + except: > + tmp_cache_file.unlink() > + raise > + try: > + # Set these just for informational purposes > + os.setxattr(str(tmp_cache_file), "user.qemu-asset-url", > + self.url.encode('utf8')) > + os.setxattr(str(tmp_cache_file), "user.qemu-asset-hash", > + self.hash.encode('utf8')) > + except Exception as e: > + self.log.info("Unable to set xattr on %s: %s", tmp_cache_file, e) > + pass > + > + if not self._check(tmp_cache_file): > + tmp_cache_file.unlink() > + raise Exception("Hash of %s does not match %s" % > + (self.url, self.hash)) > + tmp_cache_file.replace(self.cache_file) > + > + self.log.info("Cached %s at %s" % (self.url, self.cache_file)) > + return str(self.cache_file)
Hi, On 21/8/24 10:27, Thomas Huth wrote: > From: Daniel P. Berrangé <berrange@redhat.com> > > The 'Asset' class is a simple module that declares a downloadable > asset that can be cached locally. Downloads are stored in the user's > home dir at ~/.cache/qemu/download, using a sha256 sum of the URL. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org> > [thuth: Drop sha1 support, use hash on file content for naming instead of URL, > add the possibility to specify the cache dir via environment variable] > Signed-off-by: Thomas Huth <thuth@redhat.com> > --- > tests/functional/qemu_test/__init__.py | 1 + > tests/functional/qemu_test/asset.py | 97 ++++++++++++++++++++++++++ > 2 files changed, 98 insertions(+) > create mode 100644 tests/functional/qemu_test/asset.py > + def fetch(self): > + 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) > + return str(self.cache_file) > + > + self.log.info("Downloading %s to %s...", self.url, self.cache_file) > + tmp_cache_file = self.cache_file.with_suffix(".download") > + > + try: > + resp = urllib.request.urlopen(self.url) > + except Exception as e: > + self.log.error("Unable to download %s: %s", self.url, e) > + raise > + > + try: > + with tmp_cache_file.open("wb+") as dst: > + copyfileobj(resp, dst) > + except: > + tmp_cache_file.unlink() > + raise > + try: > + # Set these just for informational purposes > + os.setxattr(str(tmp_cache_file), "user.qemu-asset-url", > + self.url.encode('utf8')) > + os.setxattr(str(tmp_cache_file), "user.qemu-asset-hash", > + self.hash.encode('utf8')) > + except Exception as e: > + self.log.info("Unable to set xattr on %s: %s", tmp_cache_file, e) This line is annoying on macOS as it is logged for each file downloaded. Is it really useful? Can we demote to DEBUG level or log it just once, given all tmp_cache_files will always be on the same cache_dir thus filesystem? > + pass
On Wed, Aug 21, 2024 at 04:49:42PM +0200, Philippe Mathieu-Daudé wrote: > On 21/8/24 10:27, Thomas Huth wrote: > > From: Daniel P. Berrangé <berrange@redhat.com> > > > > The 'Asset' class is a simple module that declares a downloadable > > asset that can be cached locally. Downloads are stored in the user's > > home dir at ~/.cache/qemu/download, using a sha256 sum of the URL. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > [thuth: Drop sha1 support, use hash on file content for naming instead of URL, > > add the possibility to specify the cache dir via environment variable] > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > --- > > tests/functional/qemu_test/__init__.py | 1 + > > tests/functional/qemu_test/asset.py | 97 ++++++++++++++++++++++++++ > > 2 files changed, 98 insertions(+) > > create mode 100644 tests/functional/qemu_test/asset.py > > > > diff --git a/tests/functional/qemu_test/__init__.py b/tests/functional/qemu_test/__init__.py > > index 2f1e0bc70d..db05c8f412 100644 > > --- a/tests/functional/qemu_test/__init__.py > > +++ b/tests/functional/qemu_test/__init__.py > > @@ -6,6 +6,7 @@ > > # later. See the COPYING file in the top-level directory. > > +from .asset import Asset > > from .config import BUILD_DIR > > from .cmd import has_cmd, has_cmds, run_cmd, is_readable_executable_file, \ > > interrupt_interactive_console_until_pattern, wait_for_console_pattern, \ > > diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py > > new file mode 100644 > > index 0000000000..cbeb6278af > > --- /dev/null > > +++ b/tests/functional/qemu_test/asset.py > > @@ -0,0 +1,97 @@ > > +# Test utilities for fetching & caching assets > > +# > > +# 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. > > + > > +import hashlib > > +import logging > > +import os > > +import subprocess > > +import urllib.request > > +from pathlib import Path > > +from shutil import copyfileobj > > + > > + > > +# 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: > > + > > + def __init__(self, url, hashsum): > > + self.url = url > > + self.hash = hashsum > > + cache_dir_env = os.getenv('QEMU_TEST_CACHE_DIR') > > + if cache_dir_env: > > + self.cache_dir = Path(cache_dir_env, "download") > > + else: > > + self.cache_dir = Path(Path("~").expanduser(), > > + ".cache", "qemu", "download") > > + self.cache_file = Path(self.cache_dir, 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) > > + > > + def _check(self, cache_file): > > + if self.hash is None: > > + return True > > + if len(self.hash) == 64: > > + sum_prog = 'sha256sum' > > + elif len(self.hash) == 128: > > + sum_prog = 'sha512sum' > > + else: > > + raise Exception("unknown hash type") > > + > > + checksum = subprocess.check_output( > > + [sum_prog, str(cache_file)]).split()[0] > > + return self.hash == checksum.decode("utf-8") > > + > > + def valid(self): > > + return self.cache_file.exists() and self._check(self.cache_file) > > + > > + def fetch(self): > > + if not self.cache_dir.exists(): > > + self.cache_dir.mkdir(parents=True, exist_ok=True) > > This doesn't work with QEMU_TEST_CACHE_DIR set to someone else: > > File > "/home/philippe.mathieu-daude/qemu/tests/functional/qemu_test/asset.py", > line 60, in fetch > self.cache_dir.mkdir(parents=True, exist_ok=True) > File "/usr/lib/python3.10/pathlib.py", line 1175, in mkdir > self._accessor.mkdir(self, mode) > PermissionError: [Errno 13] Permission denied: > '/home/alex.bennee/.cache/qemu/download' > ninja: build stopped: subcommand failed. > > Maybe use a getter which falls back to Path("~").expanduser() when > no access on QEMU_TEST_CACHE_DIR? This happens when downloading a > new file (the recent MIPS tests I converted) which isn't in Alex's cache: Is it really valid to point QEMU_TEST_CACHE_DIR to a directory that you don't have permission to access ? This feels like it could be classed as invalid usage to me, rather than needing a workaround. > > 2024-08-21 15:45:48,896 - qemu-test - INFO - Attempting to cache 'Asset: url=https://s3-eu-west-1.amazonaws.com/downloads-mips/mips-downloads/YAMON/yamon-bin-02.22.zip > hash=eef86f0eed0ef554f041dcd47b87eebea0e6f9f1184ed31f7e9e8b4a803860ab cache=/home/alex.bennee/.cache/download/eef86f0eed0ef554f041dcd47b87eebea0e6f9f1184ed31f7e9e8b4a803860ab' > > > + if self.valid(): > > + self.log.debug("Using cached asset %s for %s", > > + self.cache_file, self.url) > > + return str(self.cache_file) > > + > > + self.log.info("Downloading %s to %s...", self.url, self.cache_file) > > + tmp_cache_file = self.cache_file.with_suffix(".download") > > + > > + try: > > + resp = urllib.request.urlopen(self.url) > > + except Exception as e: > > + self.log.error("Unable to download %s: %s", self.url, e) > > + raise > > + > > + try: > > + with tmp_cache_file.open("wb+") as dst: > > + copyfileobj(resp, dst) > > + except: > > + tmp_cache_file.unlink() > > + raise > > + try: > > + # Set these just for informational purposes > > + os.setxattr(str(tmp_cache_file), "user.qemu-asset-url", > > + self.url.encode('utf8')) > > + os.setxattr(str(tmp_cache_file), "user.qemu-asset-hash", > > + self.hash.encode('utf8')) > > + except Exception as e: > > + self.log.info("Unable to set xattr on %s: %s", tmp_cache_file, e) > > + pass > > + > > + if not self._check(tmp_cache_file): > > + tmp_cache_file.unlink() > > + raise Exception("Hash of %s does not match %s" % > > + (self.url, self.hash)) > > + tmp_cache_file.replace(self.cache_file) > > + > > + self.log.info("Cached %s at %s" % (self.url, self.cache_file)) > > + return str(self.cache_file) > With regards, Daniel
On Fri, Aug 23, 2024 at 08:24:45AM +0200, Philippe Mathieu-Daudé wrote: > Hi, > > On 21/8/24 10:27, Thomas Huth wrote: > > From: Daniel P. Berrangé <berrange@redhat.com> > > > > The 'Asset' class is a simple module that declares a downloadable > > asset that can be cached locally. Downloads are stored in the user's > > home dir at ~/.cache/qemu/download, using a sha256 sum of the URL. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > [thuth: Drop sha1 support, use hash on file content for naming instead of URL, > > add the possibility to specify the cache dir via environment variable] > > Signed-off-by: Thomas Huth <thuth@redhat.com> > > --- > > tests/functional/qemu_test/__init__.py | 1 + > > tests/functional/qemu_test/asset.py | 97 ++++++++++++++++++++++++++ > > 2 files changed, 98 insertions(+) > > create mode 100644 tests/functional/qemu_test/asset.py > > > > + def fetch(self): > > + 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) > > + return str(self.cache_file) > > + > > + self.log.info("Downloading %s to %s...", self.url, self.cache_file) > > + tmp_cache_file = self.cache_file.with_suffix(".download") > > + > > + try: > > + resp = urllib.request.urlopen(self.url) > > + except Exception as e: > > + self.log.error("Unable to download %s: %s", self.url, e) > > + raise > > + > > + try: > > + with tmp_cache_file.open("wb+") as dst: > > + copyfileobj(resp, dst) > > + except: > > + tmp_cache_file.unlink() > > + raise > > + try: > > + # Set these just for informational purposes > > + os.setxattr(str(tmp_cache_file), "user.qemu-asset-url", > > + self.url.encode('utf8')) > > + os.setxattr(str(tmp_cache_file), "user.qemu-asset-hash", > > + self.hash.encode('utf8')) > > + except Exception as e: > > + self.log.info("Unable to set xattr on %s: %s", tmp_cache_file, e) > > This line is annoying on macOS as it is logged for each file downloaded. > Is it really useful? Can we demote to DEBUG level or log it just once, > given all tmp_cache_files will always be on the same cache_dir thus > filesystem? Yeah, DEBUG would be fine. With regards, Daniel
diff --git a/tests/functional/qemu_test/__init__.py b/tests/functional/qemu_test/__init__.py index 2f1e0bc70d..db05c8f412 100644 --- a/tests/functional/qemu_test/__init__.py +++ b/tests/functional/qemu_test/__init__.py @@ -6,6 +6,7 @@ # later. See the COPYING file in the top-level directory. +from .asset import Asset from .config import BUILD_DIR from .cmd import has_cmd, has_cmds, run_cmd, is_readable_executable_file, \ interrupt_interactive_console_until_pattern, wait_for_console_pattern, \ diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py new file mode 100644 index 0000000000..cbeb6278af --- /dev/null +++ b/tests/functional/qemu_test/asset.py @@ -0,0 +1,97 @@ +# Test utilities for fetching & caching assets +# +# 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. + +import hashlib +import logging +import os +import subprocess +import urllib.request +from pathlib import Path +from shutil import copyfileobj + + +# 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: + + def __init__(self, url, hashsum): + self.url = url + self.hash = hashsum + cache_dir_env = os.getenv('QEMU_TEST_CACHE_DIR') + if cache_dir_env: + self.cache_dir = Path(cache_dir_env, "download") + else: + self.cache_dir = Path(Path("~").expanduser(), + ".cache", "qemu", "download") + self.cache_file = Path(self.cache_dir, 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) + + def _check(self, cache_file): + if self.hash is None: + return True + if len(self.hash) == 64: + sum_prog = 'sha256sum' + elif len(self.hash) == 128: + sum_prog = 'sha512sum' + else: + raise Exception("unknown hash type") + + checksum = subprocess.check_output( + [sum_prog, str(cache_file)]).split()[0] + return self.hash == checksum.decode("utf-8") + + def valid(self): + return self.cache_file.exists() and self._check(self.cache_file) + + def fetch(self): + 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) + return str(self.cache_file) + + self.log.info("Downloading %s to %s...", self.url, self.cache_file) + tmp_cache_file = self.cache_file.with_suffix(".download") + + try: + resp = urllib.request.urlopen(self.url) + except Exception as e: + self.log.error("Unable to download %s: %s", self.url, e) + raise + + try: + with tmp_cache_file.open("wb+") as dst: + copyfileobj(resp, dst) + except: + tmp_cache_file.unlink() + raise + try: + # Set these just for informational purposes + os.setxattr(str(tmp_cache_file), "user.qemu-asset-url", + self.url.encode('utf8')) + os.setxattr(str(tmp_cache_file), "user.qemu-asset-hash", + self.hash.encode('utf8')) + except Exception as e: + self.log.info("Unable to set xattr on %s: %s", tmp_cache_file, e) + pass + + if not self._check(tmp_cache_file): + tmp_cache_file.unlink() + raise Exception("Hash of %s does not match %s" % + (self.url, self.hash)) + tmp_cache_file.replace(self.cache_file) + + self.log.info("Cached %s at %s" % (self.url, self.cache_file)) + return str(self.cache_file)