Re: [PATCH v4 15/35] tests/functional: enable pre-emptive caching of assets

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

Hi,

On 21/8/24 10:27, 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]
> 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/Makefile.include b/tests/Makefile.include
> index 66c8cc3123..010369bd3a 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -161,7 +161,8 @@ $(FUNCTIONAL_TARGETS):
>   
>   .PHONY: check-functional
>   check-functional:
> -	@$(MAKE) SPEED=thorough check-func check-func-quick
> +	@$(NINJA) precache-functional
> +	@QEMU_TEST_NO_DOWNLOAD=1 $(MAKE) SPEED=thorough check-func check-func-quick
>   
>   # Consolidated targets
>   
> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> index 8a8fa0ab99..cef74b82a9 100644
> --- a/tests/functional/meson.build
> +++ b/tests/functional/meson.build
> @@ -33,6 +33,7 @@ tests_x86_64_quick = [
>   tests_x86_64_thorough = [
>   ]
>   
> +precache_all = []
>   foreach speed : ['quick', 'thorough']
>     foreach dir : target_dirs
>       if not dir.endswith('-softmmu')
> @@ -63,11 +64,35 @@ foreach speed : ['quick', 'thorough']
>                                  meson.current_source_dir())
>   
>       foreach test : target_tests
> -      test('func-@0@/@1@'.format(target_base, test),
> +      testname = '@0@-@1@'.format(target_base, test)
> +      testfile = 'test_' + test + '.py'
> +      testpath = meson.current_source_dir() / testfile
> +      teststamp = testname + '.tstamp'
> +      test_precache_env = environment()
> +      test_precache_env.set('QEMU_TEST_PRECACHE', meson.current_build_dir() / teststamp)
> +      test_precache_env.set('PYTHONPATH', meson.project_source_root() / 'python:' +
> +                                          meson.current_source_dir())
> +      precache = custom_target('func-precache-' + testname,
> +                               output: teststamp,
> +                               command: [python, testpath],
> +                               depend_files: files(testpath),
> +                               build_by_default: false,
> +                               env: test_precache_env)
> +      precache_all += precache
> +
> +      # Ideally we would add 'precache' to 'depends' here, such that
> +      # 'build_by_default: false' lets the pre-caching automatically
> +      # run immediately before the test runs. In practice this is
> +      # broken in meson, with it running the pre-caching in the normal
> +      # compile phase https://github.com/mesonbuild/meson/issues/2518
> +      # If the above bug ever gets fixed, when QEMU changes the min
> +      # meson version, add the 'depends' and remove the custom
> +      # 'run_target' logic below & in Makefile.include
> +      test('func-' + testname,
>              python,
>              depends: [test_deps, test_emulator, emulator_modules],
>              env: test_env,
> -           args: [meson.current_source_dir() / 'test_' + test + '.py'],
> +           args: [testpath],
>              protocol: 'tap',
>              timeout: test_timeouts.get(test, 60),
>              priority: test_timeouts.get(test, 60),
> @@ -75,3 +100,7 @@ foreach speed : ['quick', 'thorough']
>       endforeach
>     endforeach
>   endforeach
> +
> +run_target('precache-functional',
> +           depends: precache_all,
> +           command: ['true'])
> diff --git a/tests/functional/qemu_test/asset.py b/tests/functional/qemu_test/asset.py
> index cbeb6278af..9250ff2b06 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()
> +        log.removeHandler(handler)
> +
> +    def precache_suite(suite):
> +        for test in suite:
> +            if isinstance(test, unittest.TestSuite):
> +                Asset.precache_suite(test)
> +            elif isinstance(test, unittest.TestCase):
> +                Asset.precache_test(test)
> +
> +    def precache_suites(path, cacheTstamp):
> +        loader = unittest.loader.defaultTestLoader
> +        tests = loader.loadTestsFromNames([path], None)
> +
> +        with open(cacheTstamp, "w") as fh:
> +            Asset.precache_suite(tests)


When using multiple jobs (-jN) I'm observing some hangs,
apparently multiple threads trying to download the same file.
The files are eventually downloaded successfully but it takes
longer. Should we acquire some exclusive lock somewhere?

> diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py
> index b2dd863c6e..18314be9d1 100644
> --- a/tests/functional/qemu_test/testcase.py
> +++ b/tests/functional/qemu_test/testcase.py
> @@ -21,6 +21,7 @@
>   from qemu.machine import QEMUMachine
>   from qemu.utils import kvm_available, tcg_available
>   
> +from .asset import Asset
>   from .cmd import run_cmd
>   from .config import BUILD_DIR
>   
> @@ -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])