| Message ID | 1443470907-32335-2-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
Am 28.09.2015 um 22:08 schrieb Markus Armbruster: > We want to run qom-test for every architecture, without having to > manually add it to every architecture's list of tests. Commit 3687d53 > accomplished this by adding it to every architecture's list > automatically. > > However, some architectures inherit their tests from others, like this: > > check-qtest-x86_64-y = $(check-qtest-i386-y) > check-qtest-microblazeel-y = $(check-qtest-microblaze-y) > check-qtest-xtensaeb-y = $(check-qtest-xtensa-y) > > For such architectures, we ended up running the (slow!) test twice. > Commit 2b8419c attempted to avoid this by adding the test only when > it's not already present. Works only as long as we consider adding > the test to the architectures on the left hand side *after* the ones > on the right hand side: x86_64 after i386, microblazeel after > microblaze, xtensaeb after xtensa. > > Turns out we consider them in $(SYSEMU_TARGET_LIST) order. Defined as > > SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \ > $(wildcard $(SRC_PATH)/default-configs/*-softmmu.mak))) > > On my machine, this results in the oder xtensa, x86_64, microblazeel, > microblaze, i386. Consequently, qom-test runs twice for microblazeel > and x86_64. > > Replace this complex and flawed machinery with a much simpler one: add > generic tests (currently just qom-test) to check-qtest-generic-y > instead of check-qtest-$(target)-y for every target, then run > $(check-qtest-generic-y) for every target. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > tests/Makefile | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) Looks good to me, Reviewed-by: Andreas Färber <afaerber@suse.de> However one comment regarding your commit message: You missed that I proposed a different patch before Peter committed his. I wonder whether mine had the same issue... BTW there's an old patch from Stefan H. on the list (that I have on my queue to revisit, help appreciated), to make QTests more verbose even without V=1. Regards, Andreas
Andreas Färber <afaerber@suse.de> writes: > Am 28.09.2015 um 22:08 schrieb Markus Armbruster: >> We want to run qom-test for every architecture, without having to >> manually add it to every architecture's list of tests. Commit 3687d53 >> accomplished this by adding it to every architecture's list >> automatically. >> >> However, some architectures inherit their tests from others, like this: >> >> check-qtest-x86_64-y = $(check-qtest-i386-y) >> check-qtest-microblazeel-y = $(check-qtest-microblaze-y) >> check-qtest-xtensaeb-y = $(check-qtest-xtensa-y) >> >> For such architectures, we ended up running the (slow!) test twice. >> Commit 2b8419c attempted to avoid this by adding the test only when >> it's not already present. Works only as long as we consider adding >> the test to the architectures on the left hand side *after* the ones >> on the right hand side: x86_64 after i386, microblazeel after >> microblaze, xtensaeb after xtensa. >> >> Turns out we consider them in $(SYSEMU_TARGET_LIST) order. Defined as >> >> SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \ >> $(wildcard $(SRC_PATH)/default-configs/*-softmmu.mak))) >> >> On my machine, this results in the oder xtensa, x86_64, microblazeel, >> microblaze, i386. Consequently, qom-test runs twice for microblazeel >> and x86_64. >> >> Replace this complex and flawed machinery with a much simpler one: add >> generic tests (currently just qom-test) to check-qtest-generic-y >> instead of check-qtest-$(target)-y for every target, then run >> $(check-qtest-generic-y) for every target. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> tests/Makefile | 14 ++++++++------ >> 1 file changed, 8 insertions(+), 6 deletions(-) > > Looks good to me, > > Reviewed-by: Andreas Färber <afaerber@suse.de> > > However one comment regarding your commit message: You missed that I > proposed a different patch before Peter committed his. I wonder whether > mine had the same issue... I checked git history, I didn't check mailing list history :) I just had a peek, but can't find anything. I'm happy to improve my commit message, but I need either data to work in or a specific proposal. > BTW there's an old patch from Stefan H. on the list (that I have on my > queue to revisit, help appreciated), to make QTests more verbose even > without V=1. If you can give a pointer, I might be able to review. Thanks!
diff --git a/tests/Makefile b/tests/Makefile index 4063639..9380e14 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -86,6 +86,8 @@ check-block-$(CONFIG_POSIX) += tests/qemu-iotests-quick.sh # All QTests for now are POSIX-only, but the dependencies are # really in libqtest, not in the testcases themselves. +check-qtest-generic-y = + gcov-files-ipack-y += hw/ipack/ipack.c check-qtest-ipack-y += tests/ipoctal232-test$(EXESUF) gcov-files-ipack-y += hw/char/ipoctal232.c @@ -216,10 +218,7 @@ gcov-files-ppc64-y += ppc64-softmmu/hw/ppc/spapr_pci.c check-qtest-microblazeel-y = $(check-qtest-microblaze-y) check-qtest-xtensaeb-y = $(check-qtest-xtensa-y) -# qom-test works for all sysemu architectures: -$(foreach target,$(SYSEMU_TARGET_LIST), \ - $(if $(findstring tests/qom-test$(EXESUF), $(check-qtest-$(target)-y)),, \ - $(eval check-qtest-$(target)-y += tests/qom-test$(EXESUF)))) +check-qtest-generic-y += tests/qom-test$(EXESUF) check-qapi-schema-y := $(addprefix tests/qapi-schema/, \ comments.json empty.json enum-empty.json enum-missing-data.json \ @@ -446,8 +445,11 @@ CFLAGS += $(TEST_CFLAGS) TARGETS=$(patsubst %-softmmu,%, $(filter %-softmmu,$(TARGET_DIRS))) ifeq ($(CONFIG_POSIX),y) -QTEST_TARGETS=$(foreach TARGET,$(TARGETS), $(if $(check-qtest-$(TARGET)-y), $(TARGET),)) +QTEST_TARGETS = $(TARGETS) check-qtest-y=$(foreach TARGET,$(TARGETS), $(check-qtest-$(TARGET)-y)) +check-qtest-y += $(check-qtest-generic-y) +else +QTEST_TARGETS = endif qtest-obj-y = tests/libqtest.o $(test-util-obj-y) @@ -485,7 +487,7 @@ $(patsubst %, check-qtest-%, $(QTEST_TARGETS)): check-qtest-%: $(check-qtest-y) $(call quiet-command,QTEST_QEMU_BINARY=$*-softmmu/qemu-system-$* \ QTEST_QEMU_IMG=qemu-img$(EXESUF) \ MALLOC_PERTURB_=$${MALLOC_PERTURB_:-$$((RANDOM % 255 + 1))} \ - gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y),"GTESTER $@") + gtester $(GTESTER_OPTIONS) -m=$(SPEED) $(check-qtest-$*-y) $(check-qtest-generic-y),"GTESTER $@") $(if $(CONFIG_GCOV),@for f in $(gcov-files-$*-y); do \ echo Gcov report for $$f:;\ $(GCOV) $(GCOV_OPTIONS) $$f -o `dirname $$f`; \
We want to run qom-test for every architecture, without having to manually add it to every architecture's list of tests. Commit 3687d53 accomplished this by adding it to every architecture's list automatically. However, some architectures inherit their tests from others, like this: check-qtest-x86_64-y = $(check-qtest-i386-y) check-qtest-microblazeel-y = $(check-qtest-microblaze-y) check-qtest-xtensaeb-y = $(check-qtest-xtensa-y) For such architectures, we ended up running the (slow!) test twice. Commit 2b8419c attempted to avoid this by adding the test only when it's not already present. Works only as long as we consider adding the test to the architectures on the left hand side *after* the ones on the right hand side: x86_64 after i386, microblazeel after microblaze, xtensaeb after xtensa. Turns out we consider them in $(SYSEMU_TARGET_LIST) order. Defined as SYSEMU_TARGET_LIST := $(subst -softmmu.mak,,$(notdir \ $(wildcard $(SRC_PATH)/default-configs/*-softmmu.mak))) On my machine, this results in the oder xtensa, x86_64, microblazeel, microblaze, i386. Consequently, qom-test runs twice for microblazeel and x86_64. Replace this complex and flawed machinery with a much simpler one: add generic tests (currently just qom-test) to check-qtest-generic-y instead of check-qtest-$(target)-y for every target, then run $(check-qtest-generic-y) for every target. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- tests/Makefile | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)