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

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(-)

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>

Thomas Huth <thuth@redhat.com> writes:

> 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>

Queued to pr/100720-testing-and-misc-2 in lieu of gitlab: split
build-disabled into two phases, thanks.

> ---
>  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);