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

[Thomas Huth <thuth@redhat.com>]

On 27/08/2024 15.16, Thomas Huth wrote:
> On 23/08/2024 09.28, Philippe Mathieu-Daudé wrote:
>> 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?
> 
> I haven't seen that yet ... what did you exactly run? "make check-functional 
> -jN" ? Or "make check-functional-<target> -jN" ?

After applying some of your patches, I think I've run now into this problem, 
too: It's because test_aarch64_sbsaref.py and test_aarch64_virt.py try to 
download the same asset in parallel (alpine-standard-3.17.2-aarch64.iso).

Daniel, any ideas how to fix this in the Asset code?

  Thomas