Re: [Qemu-devel] [PATCH 2/2] tests: correctly skip qtest on non-POSIX hosts

[Andreas Färber]

Am 27.03.2014 12:11, schrieb Stefan Hajnoczi:
> qtest test cases only work on POSIX hosts.  The following line only
> defines dependencies for qtest binaries on POSIX hosts:
> 
>   check-qtest-$(CONFIG_POSIX)=$(foreach 
> TARGET,$(TARGETS),$(check-qtest-$(TARGET)-y))
> 
> But the QTEST_TARGETS definition earlier in the Makefile fails to check
> CONFIG_POSIX.  The causes targets to be generated for qtest test cases
> even though we don't know how to build the binaries.
> 
> The following error message is printed when trying to run gtester on a
> binary that was never built:
> 
>   GLib-WARNING **: Failed to execute test binary: tests/endianness-test.exe: 
> Failed to execute child process "tests/endianness-test.exe" (No such file or 
> directory)
> 
> This patch makes QTEST_TARGETS empty on non-POSIX hosts.  This prevents
> the targets from being generated.
> 
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  tests/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/Makefile b/tests/Makefile
> index 803c8e6..e1960c1 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -277,7 +277,8 @@ tests/qemu-iotests/socket_scm_helper$(EXESUF): 
> tests/qemu-iotests/socket_scm_hel
>  # QTest rules
>  
>  TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS)))
> -QTEST_TARGETS=$(foreach TARGET,$(TARGETS), $(if $(check-qtest-$(TARGET)-y), 
> $(TARGET),))
> +QTEST_TARGETS=$(foreach TARGET,$(TARGETS), \
> +                $(if $(check-qtest-$(TARGET)-$(CONFIG_POSIX)), $(TARGET),))

This strikes me as wrong. Essentially this is working around the fact
that tests get added to -y although they should've been added for
$(CONFIG_POSIX) only. I have a patch pending that adds a virtio-9p
qtest, which is conditional on having the three ugly conditions evaluate
to y, and otherwise will evaluate to -n. So this new query would
actually find a non-empty -n variable on !CONFIG_POSIX and would still
add the target to QTEST_TARGETS.

My suggestion would be to simply put this line into ifeq
($(CONFIG_POSIX),y) to keep the logic simple and safe.

Regards,
Andreas

>  check-qtest-$(CONFIG_POSIX)=$(foreach TARGET,$(TARGETS), 
> $(check-qtest-$(TARGET)-y))
>  
>  qtest-obj-y = tests/libqtest.o libqemuutil.a libqemustub.a

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg