Re: [Qemu-devel] [PATCH v2 4/4] tests: Use qtest_add_data_func() consistently

[John Snow <jsnow@redhat.com>]

On 03/25/2015 02:20 PM, Andreas Färber wrote:
> Replace uses of g_test_add_data_func() for QTest test cases.
>
> It is still valid to use it for any non-QTest test cases,
> which are not run for multiple target binaries.
>
> Suggested-by: John Snow <jsnow@redhat.com>
> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>   tests/ahci-test.c       |  9 ++-------
>   tests/e1000-test.c      |  4 ++--
>   tests/eepro100-test.c   |  5 ++---
>   tests/endianness-test.c | 18 +++++++++---------
>   tests/pc-cpu-test.c     | 13 ++++++-------
>   tests/qom-test.c        |  4 ++--
>   6 files changed, 23 insertions(+), 30 deletions(-)
>
> diff --git a/tests/ahci-test.c b/tests/ahci-test.c
> index ea62e24..4daa96e 100644
> --- a/tests/ahci-test.c
> +++ b/tests/ahci-test.c
> @@ -1045,7 +1045,6 @@ static void test_io_interface(gconstpointer opaque)
>   static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
>                                   enum BuffLen len)
>   {
> -    static const char *arch;
>       char *name;
>       AHCIIOTestOptions *opts = g_malloc(sizeof(AHCIIOTestOptions));
>
> @@ -1053,16 +1052,12 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
>       opts->address_type = addr;
>       opts->io_type = type;
>
> -    if (!arch) {
> -        arch = qtest_get_arch();
> -    }
> -
> -    name = g_strdup_printf("/%s/ahci/io/%s/%s/%s", arch,
> +    name = g_strdup_printf("ahci/io/%s/%s/%s",
>                              io_mode_str[type],
>                              addr_mode_str[addr],
>                              buff_len_str[len]);
>
> -    g_test_add_data_func(name, opts, test_io_interface);
> +    qtest_add_data_func(name, opts, test_io_interface);
>       g_free(name);
>   }
>
> diff --git a/tests/e1000-test.c b/tests/e1000-test.c
> index 81f164d..7ca6d7e 100644
> --- a/tests/e1000-test.c
> +++ b/tests/e1000-test.c
> @@ -44,8 +44,8 @@ int main(int argc, char **argv)
>       for (i = 0; i < ARRAY_SIZE(models); i++) {
>           char *path;
>
> -        path = g_strdup_printf("/%s/e1000/%s", qtest_get_arch(), models[i]);
> -        g_test_add_data_func(path, models[i], test_device);
> +        path = g_strdup_printf("e1000/%s", models[i]);
> +        qtest_add_data_func(path, models[i], test_device);
>       }
>
>       return g_test_run();
> diff --git a/tests/eepro100-test.c b/tests/eepro100-test.c
> index bf82526..8bfaccd 100644
> --- a/tests/eepro100-test.c
> +++ b/tests/eepro100-test.c
> @@ -54,9 +54,8 @@ int main(int argc, char **argv)
>       for (i = 0; i < ARRAY_SIZE(models); i++) {
>           char *path;
>
> -        path = g_strdup_printf("/%s/eepro100/%s",
> -                               qtest_get_arch(), models[i]);
> -        g_test_add_data_func(path, models[i], test_device);
> +        path = g_strdup_printf("eepro100/%s", models[i]);
> +        qtest_add_data_func(path, models[i], test_device);
>       }
>
>       return g_test_run();
> diff --git a/tests/endianness-test.c b/tests/endianness-test.c
> index 92e17d2..7482ff7 100644
> --- a/tests/endianness-test.c
> +++ b/tests/endianness-test.c
> @@ -298,17 +298,17 @@ int main(int argc, char **argv)
>           if (strcmp(test_cases[i].arch, arch) != 0) {
>               continue;
>           }
> -        path = g_strdup_printf("/%s/endianness/%s",
> -                               arch, test_cases[i].machine);
> -        g_test_add_data_func(path, &test_cases[i], test_endianness);
> +        path = g_strdup_printf("endianness/%s",
> +                               test_cases[i].machine);
> +        qtest_add_data_func(path, &test_cases[i], test_endianness);
>
> -        path = g_strdup_printf("/%s/endianness/split/%s",
> -                               arch, test_cases[i].machine);
> -        g_test_add_data_func(path, &test_cases[i], test_endianness_split);
> +        path = g_strdup_printf("endianness/split/%s",
> +                               test_cases[i].machine);
> +        qtest_add_data_func(path, &test_cases[i], test_endianness_split);
>
> -        path = g_strdup_printf("/%s/endianness/combine/%s",
> -                               arch, test_cases[i].machine);
> -        g_test_add_data_func(path, &test_cases[i], test_endianness_combine);
> +        path = g_strdup_printf("endianness/combine/%s",
> +                               test_cases[i].machine);
> +        qtest_add_data_func(path, &test_cases[i], test_endianness_combine);
>       }
>
>       ret = g_test_run();
> diff --git a/tests/pc-cpu-test.c b/tests/pc-cpu-test.c
> index a0122d3..3505c7c 100644
> --- a/tests/pc-cpu-test.c
> +++ b/tests/pc-cpu-test.c
> @@ -75,7 +75,6 @@ static void test_pc_without_cpu_add(gconstpointer data)
>
>   static void add_pc_test_cases(void)
>   {
> -    const char *arch = qtest_get_arch();
>       QDict *response, *minfo;
>       QList *list;
>       const QListEntry *p;
> @@ -119,15 +118,15 @@ static void add_pc_test_cases(void)
>               (strcmp(mname, "pc-0.12") == 0) ||
>               (strcmp(mname, "pc-0.11") == 0) ||
>               (strcmp(mname, "pc-0.10") == 0)) {
> -            path = g_strdup_printf("/%s/cpu/%s/init/%ux%ux%u&maxcpus=%u",
> -                                   arch, mname, data->sockets, data->cores,
> +            path = g_strdup_printf("cpu/%s/init/%ux%ux%u&maxcpus=%u",
> +                                   mname, data->sockets, data->cores,
>                                      data->threads, data->maxcpus);
> -            g_test_add_data_func(path, data, test_pc_without_cpu_add);
> +            qtest_add_data_func(path, data, test_pc_without_cpu_add);
>           } else {
> -            path = g_strdup_printf("/%s/cpu/%s/add/%ux%ux%u&maxcpus=%u",
> -                                   arch, mname, data->sockets, data->cores,
> +            path = g_strdup_printf("cpu/%s/add/%ux%ux%u&maxcpus=%u",
> +                                   mname, data->sockets, data->cores,
>                                      data->threads, data->maxcpus);
> -            g_test_add_data_func(path, data, test_pc_with_cpu_add);
> +            qtest_add_data_func(path, data, test_pc_with_cpu_add);
>           }
>       }
>       qtest_end();
> diff --git a/tests/qom-test.c b/tests/qom-test.c
> index 4246382..fde04e7 100644
> --- a/tests/qom-test.c
> +++ b/tests/qom-test.c
> @@ -128,8 +128,8 @@ static void add_machine_test_cases(void)
>           g_assert(qstr);
>           mname = qstring_get_str(qstr);
>           if (!is_blacklisted(arch, mname)) {
> -            path = g_strdup_printf("/%s/qom/%s", arch, mname);
> -            g_test_add_data_func(path, mname, test_machine);
> +            path = g_strdup_printf("qom/%s", mname);
> +            qtest_add_data_func(path, mname, test_machine);
>           }
>       }
>       qtest_end();
>

Seems fine to me. The time lost with the nested printfs during test 
initialization is likely not worth crying over in the glorious name of 
consistency.

((Biased.))

Also, what happened to the subject of this mail? Are only patches 1-3 
for-2.3?

All the same:

Reviewed-by: John Snow <jsnow@redhat.com>