From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60234) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaYFT-0000AF-S4 for qemu-devel@nongnu.org; Tue, 24 Mar 2015 19:28:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YaYFP-0007UZ-QB for qemu-devel@nongnu.org; Tue, 24 Mar 2015 19:28:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43387) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YaYFP-0007UG-IX for qemu-devel@nongnu.org; Tue, 24 Mar 2015 19:28:35 -0400 Message-ID: <5511F320.2000203@redhat.com> Date: Tue, 24 Mar 2015 19:28:32 -0400 From: John Snow 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> <5511F159.2030506@suse.de> In-Reply-To: <5511F159.2030506@suse.de> Content-Type: text/plain; charset=utf-8; format=flowed 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: =?UTF-8?B?QW5kcmVhcyBGw6RyYmVy?= , qemu-devel@nongnu.org Cc: pbonzini@redhat.com, stefanha@redhat.com, peter.maydell@linaro.org On 03/24/2015 07:20 PM, Andreas F=C3=A4rber wrote: > 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 pa= th >>> 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 *testpa= th, >>> 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(), testpa= th); >>> + g_test_add(path, FirmwareTestFixture, NULL, setup_fixture, >>> test_i440fx_firmware, NULL); >>> + g_free(path); >>> } >>> >> >> 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? :) > Not enough of one to bother delaying this for 2.3 -- I did forget this=20 was a macro. Reviewed-by: John Snow >>> 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_defaul= ts); >>> + 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); >>> >>> >> >> 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. > I can get behind "change as little as possible." > 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). I can always fire up valgrind and figure out something to point a finger=20 at :) > Thanks a lot for your review. > > Regards, > Andreas >