On 29/11/2024 18.31, Daniel P. Berrangé wrote:
> Reduce repeated boilerplate with some helper decorators:
>
> @skipIfNotPlatform("x86_64", "aarch64")
>
> => Skip unless the build host platform matches
>
> @skipIfMissingCommands("mkisofs", "losetup")
>
> => Skips unless all listed commands are found in $PATH
>
> @skipIfMissingImports("numpy", "cv2")
>
> => Skips unless all listed modules can be imported
>
> @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/NNN")
>
> => Skips unless env var requests flaky tests with the
> reason documented in the referenced gitlab bug
>
> @skipBigData
>
> => Skips unless env var permits tests creating big data files
>
> @skipUntrustedTest
>
> => Skips unless env var permits tests which are potentially
> dangerous to the host
That are good ideas! And certainly less error prone than specifying the
names of the environment variables over and over again.
> diff --git a/tests/functional/qemu_test/__init__.py b/tests/functional/qemu_test/__init__.py
> index 8fddddbe67..7dee3522f2 100644
> --- a/tests/functional/qemu_test/__init__.py
> +++ b/tests/functional/qemu_test/__init__.py
> @@ -13,3 +13,6 @@
> exec_command, exec_command_and_wait_for_pattern, get_qemu_img, which
> from .testcase import QemuBaseTest, QemuUserTest, QemuSystemTest
> from .linuxkernel import LinuxKernelTest
> +from .decorators import skipIfMissingCommands, skipIfNotMachine, \
> + skipFlakyTest, skipUntrustedTest, skipBigDataTest, \
> + skipIfMissingImports
> diff --git a/tests/functional/qemu_test/decorators.py b/tests/functional/qemu_test/decorators.py
> new file mode 100644
> index 0000000000..d25fec7b2d
> --- /dev/null
> +++ b/tests/functional/qemu_test/decorators.py
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +#
> +# Decorators useful in functional tests
> +
> +import os
> +import platform
> +from unittest import skipUnless
> +
> +from .cmd import which
> +
> +'''
> +Decorator to skip execution of a test if the list
> +of command binaries is not available in $PATH.
> +Example:
> +
> + @skipIfMissingCommands("mkisofs", "losetup")
> +'''
> +def skipIfMissingCommands(*args):
> + def has_cmds(cmdlist):
> + for cmd in cmdlist:
> + if not which(cmd):
> + return False
> + return True
> +
> + return skipUnless(lambda: has_cmds(args),
> + 'required commands(s) "%s" not installed' %
s/commands(s)/command(s)/ ?
> + ", ".join(args))
> +
> +'''
> +Decorator to skip execution of a test if the current
> +host machine does not match one of the permitted
> +machines.
> +Example
> +
> + @skipIfNotMachine("x86_64", "aarch64")
> +'''
> +def skipIfNotMachine(*args):
> + return skipUnless(lambda: platform.machine() in args,
> + 'not running on required machine(s) "%s"' %
plural sounds strange here (like all machines would be required at the same
time), I'd maybe say "not running on one of the required machine(s)" ?
> + ", ".join(args))
> +
> +'''
> +Decorator to skip execution of flaky tests, unless
> +the $QEMU_TEST_FLAKY_TESTS env var is set. A bug URL
Since it is the "official" documentation of this decorator, I'd maybe rather
use the full words: "environment variable" instead of "env var"
> +must be provided that documents the observed failure
> +behaviour, so it can be tracked & re-evaluated in future.
> +
> +Historical tests may be providing "None" as the bug_url
> +but this should not be done for new test.
> +
> +Example:
> +
> + @skipFlakyTest("https://gitlab.com/qemu-project/qemu/-/issues/NNN")
> +'''
> +def skipFlakyTest(bug_url):
> + if bug_url is None:
> + bug_url = "FIXME: reproduce flaky test and file bug report or remove"
> + return skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'),
> + f'Test is unstable: {bug_url}')
> +
> +'''
> +Decorator to skip execution of tests which are likely
> +to execute untrusted commands on the host, or commands
> +which process untrusted code, unles the
s/unles/unless/
> +$QEMU_TEST_ALLOW_UNTRUSTED_CODE env var is set.
> +Example:
> +
> + @skipUntrustedTest()
> +'''
> +def skipUntrustedTest():
> + return skipUnless(os.getenv('QEMU_TEST_ALLOW_UNTRUSTED_CODE'),
> + 'Test runs untrusted code / processes untrusted data')
> +
> +'''
> +Decorator to skip execution of tests which need large
> +data storage on the host, unless the
> +$QEMU_TEST_ALLOW_LARGE_STORAGE env var is set
Maybe we should also provide some direction what is meant with large
storage. I've seen some tests that are skipped since they create a disk file
with 128 MiB. And others are always executed though they create a disk file
with 512 MiB or even more. What would be a good recommendation here?
(My gut feeling is maybe ~ 1 GiB? Or better less?)
> +Example:
> +
> + @skipBigDataTest()
> +'''
> +def skipBigDataTest():
> + return skipUnless(os.getenv('QEMU_TEST_ALLOW_LARGE_STORAGE'),
> + 'Test required large host storage space')
s/required/requires/ ?
(the other decorators use present tense, too)
> +'''
> +Decorator to skip execution of a test if the list
> +of python imports is not available.
> +Example:
> +
> + @skipIfMissingImports("numpy", "cv2")
> +'''
> +def skipIfMissingImports(*args):
> + def has_imports(importlist):
> + for impname in importlist:
> + try:
> + import impname
> + except ImportError:
> + return False
> + return True
> +
> + return skipUnless(lambda: has_imports(args),
> + 'required imports(s) "%s" not installed' %
s/imports(s)/import(s)/ ?
> + ", ".join(args))
Thomas