From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58879) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaY86-0006gl-R3 for qemu-devel@nongnu.org; Tue, 24 Mar 2015 19:21:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaY83-0005ZI-JM for qemu-devel@nongnu.org; Tue, 24 Mar 2015 19:21:02 -0400 Received: from cantor2.suse.de ([195.135.220.15]:54756 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaY83-0005ZA-8z for qemu-devel@nongnu.org; Tue, 24 Mar 2015 19:20:59 -0400 Message-ID: <5511F159.2030506@suse.de> Date: Wed, 25 Mar 2015 00:20:57 +0100 From: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= MIME-Version: 1.0 References: <1427237148-8728-1-git-send-email-afaerber@suse.de> <1427237148-8728-3-git-send-email-afaerber@suse.de> <5511EEB1.7010306@redhat.com> In-Reply-To: <5511EEB1.7010306@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH for-2.3 2/2] i440fx-test: Fix test paths to include architecture List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: John Snow , qemu-devel@nongnu.org Cc: pbonzini@redhat.com, stefanha@redhat.com, peter.maydell@linaro.org Am 25.03.2015 um 00:09 schrieb John Snow: > On 03/24/2015 06:45 PM, Andreas F=C3=A4rber wrote: >> Replace g_test_add_func() with new qtest_add_func() and modify the pat= h >> passed to g_test_add() macro. >> >> Signed-off-by: Andreas F=C3=A4rber >> --- >> tests/i440fx-test.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c >> index a3f7279..bc3f54c 100644 >> --- a/tests/i440fx-test.c >> +++ b/tests/i440fx-test.c >> @@ -383,8 +383,10 @@ static void add_firmware_test(const char *testpat= h, >> void >> (*setup_fixture)(FirmwareTestFixture *f, >> gconstpointer >> test_data)) >> { >> - g_test_add(testpath, FirmwareTestFixture, NULL, setup_fixture, >> + char *path =3D g_strdup_printf("/%s%s", qtest_get_arch(), testpat= h); >> + g_test_add(path, FirmwareTestFixture, NULL, setup_fixture, >> test_i440fx_firmware, NULL); >> + g_free(path); >> } >> >=20 > Is it not worth adding an even more generic wrapper to prevent future > desynch from our preferred path format? As mentioned in the commit message, g_test_add() is a macro, not a function, so seemed more complicated to wrap. Can you post a patch if you have an idea? :) >> static void request_bios(FirmwareTestFixture *fixture, >> @@ -408,8 +410,8 @@ int main(int argc, char **argv) >> >> data.num_cpus =3D 1; >> >> - g_test_add_data_func("/i440fx/defaults", &data, >> test_i440fx_defaults); >> - g_test_add_data_func("/i440fx/pam", &data, test_i440fx_pam); >> + qtest_add_data_func("i440fx/defaults", &data, test_i440fx_default= s); >> + qtest_add_data_func("i440fx/pam", &data, test_i440fx_pam); >> add_firmware_test("/i440fx/firmware/bios", request_bios); >> add_firmware_test("/i440fx/firmware/pflash", request_pflash); >> >> >=20 > Similar to above, is it worth replacing other test's usage of > g_test_add_data_func to force this standard path format everywhere? I reviewed the test output from -rc0 while going through test failures. fw_cfg-test and i440fx-test were the only nits I found. I wondered whether we might poison the g_test_* versions or convert the other tests, but did not want to unnecessarily risk this for 2.3. Another issue that I've come across is that several tests misuse qmp(), ignoring the return value instead of using qmp_discard_response(). Also I had the impression that my qom-test has noticeably degraded in performance, and I wondered whether Paolo's changes to qmp(), parsing the string argument into a QObject, might be to blame, given that a lot of QMP qom-gets are performed by my test, and the number of objects and properties tested keeps increasing (MemoryRegion, qemu_irq, machines). Thanks a lot for your review. Regards, Andreas --=20 SUSE Linux GmbH, Maxfeldstr. 5, 90409 N=C3=BCrnberg, Germany GF: Felix Imend=C3=B6rffer, Jane Smithard, Jennifer Guild, Dilip Upmanyu, Graham Norton; HRB 21284 (AG N=C3=BCrnberg)