Re: [kvm-unit-tests PATCH v2 17/18] unittest: Add disabled_if parameter and use it for kvmtool

[Andrew Jones <andrew.jones@linux.dev>]

On Mon, Jan 20, 2025 at 04:43:15PM +0000, Alexandru Elisei wrote:
> The pci-test is qemu specific. Other tests perform migration, which
> isn't supported by kvmtool. In general, kvmtool is not as feature-rich
> as qemu, so add a new unittest parameter, disabled_if, that causes a
> test to be skipped if the condition evaluates to true.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arm/unittests.cfg    |  7 +++++++
>  docs/unittests.txt   | 13 +++++++++++++
>  scripts/common.bash  |  8 ++++++--
>  scripts/runtime.bash |  6 ++++++
>  4 files changed, 32 insertions(+), 2 deletions(-)
> 
> diff --git a/arm/unittests.cfg b/arm/unittests.cfg
> index 974a5a9e4113..9b1df5e02a58 100644
> --- a/arm/unittests.cfg
> +++ b/arm/unittests.cfg
> @@ -44,6 +44,7 @@ groups = selftest
>  # Test PCI emulation
>  [pci-test]
>  file = pci-test.flat
> +disabled_if = [[ "$TARGET" != qemu ]]
>  groups = pci
>  
>  # Test PMU support
> @@ -208,6 +209,7 @@ file = gic.flat
>  smp = $MAX_SMP
>  extra_params = -machine gic-version=3 -append 'its-migration'
>  groups = its migration
> +disabled_if = [[ "$TARGET" != qemu ]]
>  arch = arm64
>  
>  [its-pending-migration]
> @@ -215,6 +217,7 @@ file = gic.flat
>  smp = $MAX_SMP
>  extra_params = -machine gic-version=3 -append 'its-pending-migration'
>  groups = its migration
> +disabled_if = [[ "$TARGET" != qemu ]]
>  arch = arm64
>  
>  [its-migrate-unmapped-collection]
> @@ -222,6 +225,7 @@ file = gic.flat
>  smp = $MAX_SMP
>  extra_params = -machine gic-version=3 -append 'its-migrate-unmapped-collection'
>  groups = its migration
> +disabled_if = [[ "$TARGET" != qemu ]]
>  arch = arm64
>  
>  # Test PSCI emulation
> @@ -263,6 +267,7 @@ groups = debug
>  file = debug.flat
>  arch = arm64
>  extra_params = -append 'bp-migration'
> +disabled_if = [[ "$TARGET" != qemu ]]
>  groups = debug migration
>  
>  [debug-wp]
> @@ -276,6 +281,7 @@ groups = debug
>  file = debug.flat
>  arch = arm64
>  extra_params = -append 'wp-migration'
> +disabled_if = [[ "$TARGET" != qemu ]]
>  groups = debug migration
>  
>  [debug-sstep]
> @@ -289,6 +295,7 @@ groups = debug
>  file = debug.flat
>  arch = arm64
>  extra_params = -append 'ss-migration'
> +disabled_if = [[ "$TARGET" != qemu ]]
>  groups = debug migration
>  
>  # FPU/SIMD test
> diff --git a/docs/unittests.txt b/docs/unittests.txt
> index ebb6994cab77..58d1a29146a3 100644
> --- a/docs/unittests.txt
> +++ b/docs/unittests.txt
> @@ -115,3 +115,16 @@ parameter needs to be of the form <path>=<value>
>  The path and value cannot contain space, =, or shell wildcard characters.
>  
>  Can be overwritten with the CHECK environment variable with the same syntax.
> +
> +disabled_if
> +------
> +disabled_if = <condition>
> +
> +Do not run the test if <condition> is met. <condition> will be fed unmodified
> +to a bash 'if' statement and follows the same syntax.
> +
> +This can be used to prevent running a test when kvm-unit-tests is configured a
> +certain way. For example, it can be used to skip a qemu specific test when
> +using another VMM and using UEFI:
> +
> +disabled_if = [[ "$TARGET" != qemu ]] && [[ "$CONFIG_EFI" = y ]]
> diff --git a/scripts/common.bash b/scripts/common.bash
> index f54ffbd7a87b..c0ea2eabeda6 100644
> --- a/scripts/common.bash
> +++ b/scripts/common.bash
> @@ -38,6 +38,7 @@ function for_each_unittest()
>  	local accel
>  	local timeout
>  	local kvmtool_opts
> +	local disabled_cond
>  	local rematch
>  
>  	exec {fd}<"$unittests"
> @@ -46,7 +47,7 @@ function for_each_unittest()
>  		if [[ "$line" =~ ^\[(.*)\]$ ]]; then
>  			rematch=${BASH_REMATCH[1]}
>  			if [ -n "${testname}" ]; then
> -				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"
> +				$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts" "$disabled_cond"
>  			fi
>  			testname=$rematch
>  			smp=1
> @@ -59,6 +60,7 @@ function for_each_unittest()
>  			accel=""
>  			timeout=""
>  			kvmtool_opts=""
> +			disabled_cond=""
>  		elif [[ $line =~ ^file\ *=\ *(.*)$ ]]; then
>  			kernel=$TEST_DIR/${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^smp\ *=\ *(.*)$ ]]; then
> @@ -79,6 +81,8 @@ function for_each_unittest()
>  			machine=${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^check\ *=\ *(.*)$ ]]; then
>  			check=${BASH_REMATCH[1]}
> +		elif [[ $line =~ ^disabled_if\ *=\ *(.*)$ ]]; then
> +			disabled_cond=${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^accel\ *=\ *(.*)$ ]]; then
>  			accel=${BASH_REMATCH[1]}
>  		elif [[ $line =~ ^timeout\ *=\ *(.*)$ ]]; then
> @@ -86,7 +90,7 @@ function for_each_unittest()
>  		fi
>  	done
>  	if [ -n "${testname}" ]; then
> -		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts"
> +		$(arch_cmd) "$cmd" "$testname" "$groups" "$smp" "$kernel" "$qemu_opts" "$arch" "$machine" "$check" "$accel" "$timeout" "$kvmtool_opts" "$disabled_cond"
>  	fi
>  	exec {fd}<&-
>  }
> diff --git a/scripts/runtime.bash b/scripts/runtime.bash
> index abfd1e67b2ef..002bd2744d6b 100644
> --- a/scripts/runtime.bash
> +++ b/scripts/runtime.bash
> @@ -108,6 +108,7 @@ function run()
>      local accel="$9"
>      local timeout="${10:-$TIMEOUT}" # unittests.cfg overrides the default
>      local kvmtool_opts="${11}"
> +    local disabled_cond="${12}"
>  
>      case "$TARGET" in
>      qemu)
> @@ -186,6 +187,11 @@ function run()
>          done
>      fi
>  
> +    if [[ "$disabled_cond" ]] && (eval $disabled_cond); then
> +		print_result "SKIP" $testname "" "disabled because: $disabled_cond"
> +		return 2
> +	fi
> +
>      log=$(premature_failure) && {
>          skip=true
>          if [ "${CONFIG_EFI}" == "y" ]; then
> -- 
> 2.47.1
>

I like disabled_if because I like the lambda-like thing it's doing, but I
wonder if it wouldn't be better to make TARGET a first class citizen by
adding a 'targets' unittest parameter which allows listing all targets the
test can run on, e.g.

[selftest-setup]
file = selftest.flat
smp = 2
extra_params = -m 256 -append 'setup smp=2 mem=256'
targets = qemu,kvmtool
groups = selftest

[pci-test]
file = pci-test.flat
targets = qemu
groups = pci

If targets isn't present then the default is only qemu.

Thanks,
drew