Re: [PATCH v2] tests: improve performance of device-introspect-test

[Laurent Vivier]

On 10/07/2020 08:07, Thomas Huth wrote:
> From: Daniel P. Berrangé <berrange@redhat.com>
> 
> Total execution time with "-m slow" and x86_64 QEMU, drops from 3
> minutes 15 seconds, down to 54 seconds.
> 
> Individual tests drop from 17-20 seconds, down to 3-4 seconds.
> 
> The cost of this change is that any QOM bugs resulting in the test
> failure will not be directly associated with the device that caused
> the failure. The test case is not frequently identifying such bugs
> though, and the cause is likely easily visible in the patch series
> that causes the failure. So overall the shorter running time is
> considered the more important factor.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> [thuth: Add the tree check to test_device_intro_none() and
>  test_device_intro_abstract(), too, just to be sure...]
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  v2: Add the tree check to test_device_intro_none() and
>      test_device_intro_abstract(), too
> 
>  When I run the following command, the test time drops from more
>  than 20 minutes to 50 seconds now (wow!):
>  
>  QTEST_QEMU_BINARY=ppc64-softmmu/qemu-system-ppc64 \
>   time tests/qtest/device-introspect-test -m slow > /dev/null
> 
>  tests/qtest/device-introspect-test.c | 60 ++++++++++++++++++----------
>  1 file changed, 38 insertions(+), 22 deletions(-)
> 
> diff --git a/tests/qtest/device-introspect-test.c 
> b/tests/qtest/device-introspect-test.c
> index 9abb5ec889..d68b7856a7 100644
> --- a/tests/qtest/device-introspect-test.c
> +++ b/tests/qtest/device-introspect-test.c
> @@ -105,14 +105,9 @@ static void test_one_device(QTestState *qts, const char 
> *type)
>  {
>      QDict *resp;
>      char *help;
> -    char *qom_tree_start, *qom_tree_end;
> -    char *qtree_start, *qtree_end;
>  
>      g_test_message("Testing device '%s'", type);
>  
> -    qom_tree_start = qtest_hmp(qts, "info qom-tree");
> -    qtree_start = qtest_hmp(qts, "info qtree");
> -
>      resp = qtest_qmp(qts, "{'execute': 'device-list-properties',"
>                            " 'arguments': {'typename': %s}}",
>                 type);
> @@ -120,21 +115,6 @@ static void test_one_device(QTestState *qts, const char 
> *type)
>  
>      help = qtest_hmp(qts, "device_add \"%s,help\"", type);
>      g_free(help);
> -
> -    /*
> -     * Some devices leave dangling pointers in QOM behind.
> -     * "info qom-tree" or "info qtree" have a good chance at crashing then.
> -     * Also make sure that the tree did not change.
> -     */
> -    qom_tree_end = qtest_hmp(qts, "info qom-tree");
> -    g_assert_cmpstr(qom_tree_start, ==, qom_tree_end);
> -    g_free(qom_tree_start);
> -    g_free(qom_tree_end);
> -
> -    qtree_end = qtest_hmp(qts, "info qtree");
> -    g_assert_cmpstr(qtree_start, ==, qtree_end);
> -    g_free(qtree_start);
> -    g_free(qtree_end);
>  }
>  
>  static void test_device_intro_list(void)
> @@ -213,16 +193,38 @@ static void test_qom_list_fields(void)
>  static void test_device_intro_none(void)
>  {
>      QTestState *qts = qtest_init(common_args);
> +    g_autofree char *qom_tree_start = qtest_hmp(qts, "info qom-tree");
> +    g_autofree char *qom_tree_end = NULL;
> +    g_autofree char *qtree_start = qtest_hmp(qts, "info qtree");
> +    g_autofree char *qtree_end = NULL;
>  
>      test_one_device(qts, "nonexistent");
> +
> +    /* Make sure that really nothing changed in the trees */
> +    qom_tree_end = qtest_hmp(qts, "info qom-tree");
> +    g_assert_cmpstr(qom_tree_start, ==, qom_tree_end);
> +    qtree_end = qtest_hmp(qts, "info qtree");
> +    g_assert_cmpstr(qtree_start, ==, qtree_end);
> +
>      qtest_quit(qts);
>  }
>  
>  static void test_device_intro_abstract(void)
>  {
>      QTestState *qts = qtest_init(common_args);
> +    g_autofree char *qom_tree_start = qtest_hmp(qts, "info qom-tree");
> +    g_autofree char *qom_tree_end = NULL;
> +    g_autofree char *qtree_start = qtest_hmp(qts, "info qtree");
> +    g_autofree char *qtree_end = NULL;
>  
>      test_one_device(qts, "device");
> +
> +    /* Make sure that really nothing changed in the trees */
> +    qom_tree_end = qtest_hmp(qts, "info qom-tree");
> +    g_assert_cmpstr(qom_tree_start, ==, qom_tree_end);
> +    qtree_end = qtest_hmp(qts, "info qtree");
> +    g_assert_cmpstr(qtree_start, ==, qtree_end);
> +
>      qtest_quit(qts);
>  }
>  
> @@ -231,9 +233,12 @@ static void test_device_intro_concrete(const void *args)
>      QList *types;
>      QListEntry *entry;
>      const char *type;
> -    QTestState *qts;
> +    QTestState *qts = qtest_init(args);
> +    g_autofree char *qom_tree_start = qtest_hmp(qts, "info qom-tree");
> +    g_autofree char *qom_tree_end = NULL;
> +    g_autofree char *qtree_start = qtest_hmp(qts, "info qtree");
> +    g_autofree char *qtree_end = NULL;
>  
> -    qts = qtest_init(args);
>      types = device_type_list(qts, false);
>  
>      QLIST_FOREACH_ENTRY(types, entry) {
> @@ -243,6 +248,17 @@ static void test_device_intro_concrete(const void *args)
>          test_one_device(qts, type);
>      }
>  
> +    /*
> +     * Some devices leave dangling pointers in QOM behind.
> +     * "info qom-tree" or "info qtree" have a good chance at crashing then.
> +     * Also make sure that the tree did not change.
> +     */
> +    qom_tree_end = qtest_hmp(qts, "info qom-tree");
> +    g_assert_cmpstr(qom_tree_start, ==, qom_tree_end);
> +
> +    qtree_end = qtest_hmp(qts, "info qtree");
> +    g_assert_cmpstr(qtree_start, ==, qtree_end);
> +
>      qobject_unref(types);
>      qtest_quit(qts);
>      g_free((void *)args);
> 

Reviewed-by: Laurent Vivier <lvivier@redhat.com>