Re: [kvm-unit-tests PATCH v3 15/16] scripts: Add 'disabled_if' test definition parameter for kvmtool to use

Andrew Jones
2025-05-07T16:56:50Z

On Wed, May 07, 2025 at 04:12:55PM +0100, 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>
> ---
>
> This is what Drew said about the patch in the previous iteration [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 [..]
>
> If targets isn't present then the default is only qemu.'
>
> Like I've said on the cover letter, I think making qemu the default (if
> 'targets' isn't specified in the test definition) will mean that new tests
> will not run with kvmtool. I was thinking something along the lines
> 'excluded_targets', with the default (when left unspecified) being that the
> tests run with all the vmms that the architecture support (or, to put it
> another way, no vmms are excluded).
>
> Or we could go with 'targets' and say that when left empty it means 'all
> the vmms that the architecture supports' - though in my opinion this
> semantic is somewhat better conveyed with the name 'excluded_targets'.

excluded_targets sounds good, but disabled_if is growing on me. So, unless
you or others also prefer excluded_targets, then

Reviewed-by: Andrew Jones <andrew.jones@linux.dev>

Thanks,
drew