Re: [PATCH v2 02/10] tests/functional: add skipUnlessConfig() helper

["Daniel P. Berrangé" <berrange@redhat.com>]

On Mon, Sep 07, 2026 at 02:02:29PM +0200, Thomas Huth wrote:
>  Hi!
> 
> On 07/09/2026 12.30, Marc-André Lureau wrote:
> > Read config-host.h and skip tests that rely on some CONFIG_*
> > values.
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   tests/functional/qemu_test/__init__.py   |  2 +-
> >   tests/functional/qemu_test/decorators.py | 25 +++++++++++++++++++++++++
> >   2 files changed, 26 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tests/functional/qemu_test/__init__.py b/tests/functional/qemu_test/__init__.py
> > index 03e5c73d39dc..e21fa8279281 100644
> > --- a/tests/functional/qemu_test/__init__.py
> > +++ b/tests/functional/qemu_test/__init__.py
> > @@ -16,7 +16,7 @@
> >   from .decorators import skipIfMissingCommands, skipIfNotMachine, \
> >       skipFlakyTest, skipUntrustedTest, skipBigDataTest, skipSlowTest, \
> >       skipIfMissingImports, skipIfOperatingSystem, skipUnlessOperatingSystem, \
> > -    skipLockedMemoryTest, skipIfMissingEnv
> > +    skipLockedMemoryTest, skipIfMissingEnv, skipUnlessConfig
> >   from .archive import archive_extract
> >   from .uncompress import uncompress
> >   from .gdb import GDB
> > diff --git a/tests/functional/qemu_test/decorators.py b/tests/functional/qemu_test/decorators.py
> > index aa135acc7857..7a7d30503440 100644
> > --- a/tests/functional/qemu_test/decorators.py
> > +++ b/tests/functional/qemu_test/decorators.py
> > @@ -10,6 +10,7 @@
> >   from unittest import skipIf, skipUnless
> >   from .cmd import which
> > +from .config import BUILD_DIR
> >   def skipIfMissingEnv(*vars_):
> > @@ -162,6 +163,30 @@ def skipIfMissingImports(*args):
> >       return skipUnless(has_imports, 'required import(s) "%s" not installed' %
> >                                      ", ".join(args))
> > +def _read_config_host():
> > +    config = set()
> > +    with open(BUILD_DIR / "config-host.h", "r") as f:
> > +        for line in f:
> > +            if line.startswith("#define CONFIG_"):
> > +                name = line.split()[1].removeprefix("CONFIG_")
> > +                config.add(name)
> > +    return config
> > +
> > +_CONFIG_HOST = _read_config_host()
> 
> This always reads in config-host.h, also for tests that don't need the
> decorator ... could you change it so that the file is only read (once) if a
> test calls the skipUnlessConfig decorator?
> 
> Also I'm a little bit torn whether we really need a decorator for this or
> whether we should rather fence the tests in meson.build instead (similar to
> what we do in tests/qtest/meson.build with config_all_devices.has_key('...')
> already). What do others think about this?

QMP provides a way to query what functionality it exposes, so
it should be possible to query upfront when the "screenshot"
command is exposed or not. So IMHO reading config-host.h is a
a mistake - if it were needed, it would be a sign that QMP
was missing something, because the functional tests should
be thought of the same way as a mgmt application.

Personally I'd not bother trying to query the QMP schema though,
just keep the test written the way it already works, and catch
the QMP  error for "CommandNotFound" instead or parsing HMP
output.

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|