Re: [PULL 15/42] tests/functional: enable pre-emptive caching of assets

[Daniel P . Berrangé]

On Tue, Nov 26, 2024 at 05:44:29PM +0100, Philippe Mathieu-Daudé wrote:
> Hi,
> 
> On 4/9/24 12:38, Thomas Huth wrote:
> > From: Daniel P. Berrangé <berrange@redhat.com>
> > 
> > Many tests need to access assets stored on remote sites. We don't want
> > to download these during test execution when run by meson, since this
> > risks hitting test timeouts when data transfers are slow.
> > 
> > Add support for pre-emptive caching of assets by setting the env var
> > QEMU_TEST_PRECACHE to point to a timestamp file. When this is set,
> > instead of running the test, the assets will be downloaded and saved
> > to the cache, then the timestamp file created.
> > 
> > A meson custom target is created as a dependency of each test suite
> > to trigger the pre-emptive caching logic before the test runs.
> > 
> > When run in caching mode, it will locate assets by looking for class
> > level variables with a name prefix "ASSET_", and type "Asset".
> > 
> > At the ninja level
> > 
> >     ninja test --suite functional
> > 
> > will speculatively download any assets that are not already cached,
> > so it is advisable to set a timeout multiplier.
> > 
> >     QEMU_TEST_NO_DOWNLOAD=1 ninja test --suite functional
> > 
> > will fail the test if a required asset is not already cached
> > 
> >     ninja precache-functional
> > 
> > will download and cache all assets required by the functional
> > tests
> > 
> > At the make level, precaching is always done by
> > 
> >     make check-functional
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > Tested-by: Richard Henderson <richard.henderson@linaro.org>
> > [thuth: Remove the duplicated "path = os.path.basename(...)" line]
> > Message-ID: <20240830133841.142644-16-thuth@redhat.com>
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >   tests/Makefile.include                 |  3 ++-
> >   tests/functional/meson.build           | 33 +++++++++++++++++++++++--
> >   tests/functional/qemu_test/asset.py    | 34 ++++++++++++++++++++++++++
> >   tests/functional/qemu_test/testcase.py |  7 ++++++
> >   4 files changed, 74 insertions(+), 3 deletions(-)
> 
> 
> > diff --git a/tests/functional/qemu_test/asset.py 
> > b/tests/functional/qemu_test/asset.py
> > index c0e675d847..b329ab7dbe 100644
> > --- a/tests/functional/qemu_test/asset.py
> > +++ b/tests/functional/qemu_test/asset.py
> > @@ -9,6 +9,8 @@
> >   import logging
> >   import os
> >   import subprocess
> > +import sys
> > +import unittest
> >   import urllib.request
> >   from pathlib import Path
> >   from shutil import copyfileobj
> > @@ -62,6 +64,9 @@ def fetch(self):
> >                              self.cache_file, self.url)
> >               return str(self.cache_file)
> > +        if os.environ.get("QEMU_TEST_NO_DOWNLOAD", False):
> > +            raise Exception("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")
> > @@ -95,3 +100,32 @@ def fetch(self):
> >           self.log.info("Cached %s at %s" % (self.url, self.cache_file))
> >           return str(self.cache_file)
> > +
> > +    def precache_test(test):
> > +        log = logging.getLogger('qemu-test')
> > +        log.setLevel(logging.DEBUG)
> > +        handler = logging.StreamHandler(sys.stdout)
> > +        handler.setLevel(logging.DEBUG)
> > +        formatter = logging.Formatter(
> > +            '%(asctime)s - %(name)s - %(levelname)s - %(message)s')
> > +        handler.setFormatter(formatter)
> > +        log.addHandler(handler)
> > +        for name, asset in vars(test.__class__).items():
> > +            if name.startswith("ASSET_") and type(asset) == Asset:
> > +                log.info("Attempting to cache '%s'" % asset)
> > +                asset.fetch()
> 
> fetch() can fail [*] (see previous patch, various Exceptions returned).
> 
> What should we do in this case? If we ignore a missing artifact,
> the tests will eventually fail. Better bail out early and save
> credit minutes?

We already do what you describe - 'fetch' will raise an exception
which causes the precache task to fail, and the CI job gets marked
as failed. We don't attempt to run tests if assets are missing.


> > @@ -58,6 +59,12 @@ def tearDown(self):
> >       def main():
> >           path = os.path.basename(sys.argv[0])[:-3]
> > +
> > +        cache = os.environ.get("QEMU_TEST_PRECACHE", None)
> > +        if cache is not None:
> > +            Asset.precache_suites(path, cache)
> > +            return
> > +
> >           tr = pycotap.TAPTestRunner(message_log = 
> > pycotap.LogMode.LogToError,
> >                                      test_output_log = 
> > pycotap.LogMode.LogToError)
> >           unittest.main(module = None, testRunner = tr, argv=["__dummy__", 
> > path])
> 
> [*] Peter reported the following CI failure:
> 
>   https://gitlab.com/qemu-project/qemu/-/jobs/8474928266
> 
> 2024-11-26 14:58:53,170 - qemu-test - ERROR - Unable to download 
> https://apt.armbian.com/pool/main/l/linux-6.6.16/linux-image-current-sunxi_24.2.1_armhf__6.6.16-Seb3e-D6b4a-P2359-Ce96bHfe66-HK01ba-V014b-B067e-R448a.deb:

This looks to be working as intended. We failed to cache
the asset, and so we stopped the job, without trying to
run the tests.

The only problem I see is that our private CI runner
config is still broken, with no persistent cache provided


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|