All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: "Andreas Färber" <afaerber@suse.de>
Cc: qemu-devel@nongnu.org, stefanha@redhat.com, ehabkost@redhat.com
Subject: Re: [Qemu-devel] [PATCH 7/7] tests: Simplify how qom-test is run
Date: Wed, 23 Sep 2015 15:57:39 +0200	[thread overview]
Message-ID: <87vbb15jcc.fsf@blackfin.pond.sub.org> (raw)
In-Reply-To: <55FC2DB8.30604@suse.de> ("Andreas Färber"'s message of "Fri, 18 Sep 2015 17:28:56 +0200")

Andreas Färber <afaerber@suse.de> writes:

> Am 18.09.2015 um 16:24 schrieb Markus Armbruster:
>> Andreas Färber <afaerber@suse.de> 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 <armbru@redhat.com>
>>>> ---
>>>>  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.

      parent reply	other threads:[~2015-09-23 14:06 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-18 12:00 [Qemu-devel] [PATCH 0/7] Fix device introspection regressions Markus Armbruster
2015-09-18 12:00 ` [Qemu-devel] [PATCH 1/7] libqtest: Clean up unused QTestState member sigact_old Markus Armbruster
2015-09-18 15:36   ` Eric Blake
2015-09-18 12:00 ` [Qemu-devel] [PATCH 2/7] libqtest: New hmp() & friends Markus Armbruster
2015-09-18 15:47   ` Eric Blake
2015-09-21  5:59     ` Markus Armbruster
2015-09-18 12:00 ` [Qemu-devel] [PATCH 3/7] device-introspect-test: New, covering device introspection Markus Armbruster
2015-09-18 15:55   ` Eric Blake
2015-09-21  6:05     ` Markus Armbruster
2015-09-18 12:00 ` [Qemu-devel] [PATCH 4/7] qmp: Fix device-list-properties not to crash for abstract device Markus Armbruster
2015-09-18 15:58   ` Eric Blake
2015-09-18 12:00 ` [Qemu-devel] [PATCH 5/7] qdev: Protect device-list-properties against broken devices Markus Armbruster
2015-09-18 12:38   ` Christian Borntraeger
2015-09-21  8:30     ` David Hildenbrand
2015-09-21 15:38       ` Eduardo Habkost
2015-09-22  8:02         ` David Hildenbrand
2015-09-22  8:07         ` Markus Armbruster
2015-09-18 16:09   ` Eric Blake
2015-09-21  6:08     ` Markus Armbruster
2015-09-18 16:36   ` Eduardo Habkost
2015-09-21  6:09     ` Markus Armbruster
2015-09-21 15:13       ` Eduardo Habkost
2015-09-18 18:42   ` Thomas Huth
2015-09-18 19:32     ` Eduardo Habkost
2015-09-21  6:14       ` Markus Armbruster
2015-09-21 15:20         ` Eduardo Habkost
2015-09-21 15:48         ` Thomas Huth
2015-09-21 16:39           ` Markus Armbruster
2015-09-21 17:22             ` Thomas Huth
2015-09-21 18:19               ` Eduardo Habkost
2015-09-18 12:00 ` [Qemu-devel] [PATCH 6/7] Revert "qdev: Use qdev_get_device_class() for -device <type>, help" Markus Armbruster
2015-09-18 16:13   ` Eric Blake
2015-09-18 12:00 ` [Qemu-devel] [PATCH 7/7] tests: Simplify how qom-test is run Markus Armbruster
2015-09-18 12:53   ` Andreas Färber
2015-09-18 14:24     ` Markus Armbruster
2015-09-18 15:28       ` Andreas Färber
2015-09-21  6:15         ` Markus Armbruster
2015-09-23 13:57         ` Markus Armbruster [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87vbb15jcc.fsf@blackfin.pond.sub.org \
    --to=armbru@redhat.com \
    --cc=afaerber@suse.de \
    --cc=ehabkost@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.