Re: [Qemu-devel] [PATCH 7/7] tests: Simplify how qom-test is run

[Markus Armbruster]

Andreas Färber <address@hidden> writes:

> Am 18.09.2015 um 16:24 schrieb Markus Armbruster:
>> Andreas Färber <address@hidden> writes:
>>> Am 18.09.2015 um 14:00 schrieb Markus Armbruster:
>>>> Add it to check-qtest-generic-y instead of check-qtest-$(target)-y for
>>>> every target.
>>>>
>>>> Signed-off-by: Markus Armbruster <address@hidden>
>>>> ---
>>>>  tests/Makefile | 5 +----
>>>>  1 file changed, 1 insertion(+), 4 deletions(-)
>>>>
>>>> diff --git a/tests/Makefile b/tests/Makefile
>>>> index 4559045..28c5f93 100644
>>>> --- a/tests/Makefile
>>>> +++ b/tests/Makefile
>>>> @@ -219,10 +219,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)
>>>
>>> Does this -generic- have the same filtering code to avoid running the
>>> tests twice for x86_64, aarch64, ppc64, etc.? Please don't regress.
>> 
>> I'm dense today.  Can you explain the filtering code to me?
>
> For practical purpose,s x86_64 adds all tests from i386, that included
> qom-test then. If we now add it for x86_64 too, it got executed twice,
> which the above $(if ...) fixes by not adding it for x86_64 if it's
> already in. Just checking whether -generic- has equivalent filtering or
> other code somewhere else?

The code in master works only sometimes.  Here's the explanation copied
from my revised patch's commit message:

    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.
    
After my patch v2 (to be sent soon), it runs exactly once per target.