Re: [PATCH v19 7/8] tests/numa: Add case for QMP build HMAT

[Markus Armbruster <armbru@redhat.com>]

Tao Xu <tao3.xu@intel.com> writes:

> Check configuring HMAT usecase
>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> Suggested-by: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: Tao Xu <tao3.xu@intel.com>
> ---
>
> Changes in v19:
>     - Add some fail cases for hmat-cache when level=0
>
> Changes in v18:
>     - Rewrite the lines over 80 characters
>
> Chenges in v17:
>     - Add some fail test cases (Igor)
> ---
>  tests/numa-test.c | 213 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 213 insertions(+)
>
> diff --git a/tests/numa-test.c b/tests/numa-test.c
> index 8de8581231..aed7b2f31b 100644
> --- a/tests/numa-test.c
> +++ b/tests/numa-test.c
> @@ -327,6 +327,216 @@ static void pc_dynamic_cpu_cfg(const void *data)
>      qtest_quit(qs);
>  }
>  
> +static void pc_hmat_build_cfg(const void *data)
> +{
> +    QTestState *qs = qtest_initf("%s -nodefaults --preconfig -machine hmat=on "
> +                     "-smp 2,sockets=2 "
> +                     "-m 128M,slots=2,maxmem=1G "
> +                     "-object memory-backend-ram,size=64M,id=m0 "
> +                     "-object memory-backend-ram,size=64M,id=m1 "
> +                     "-numa node,nodeid=0,memdev=m0 "
> +                     "-numa node,nodeid=1,memdev=m1,initiator=0 "
> +                     "-numa cpu,node-id=0,socket-id=0 "
> +                     "-numa cpu,node-id=0,socket-id=1",
> +                     data ? (char *)data : "");
> +
> +    /* Fail: Initiator should be less than the number of nodes */
> +    g_assert(qmp_rsp_is_err(qtest_qmp(qs, "{ 'execute': 'set-numa-node',"
> +        " 'arguments': { 'type': 'hmat-lb', 'initiator': 2, 'target': 0,"
> +        " 'hierarchy': \"memory\", 'data-type': \"access-latency\" } }")));

Code smell: side effect within assert().

Harmless here, because compiling tests with NDEBUG is pointless.  Still,
it sets a bad example.  Not your idea, the pattern seems to go back to
commit c35665e1ee3 and fb1e58f72ba.

The non-smelly pattern would be

    resp = qtest_qmp(...);
    g_assert(resp);
    g_assert(qdict_haskey(rsp, "error"));
    qobject_unref(resp);

It's a bit longwinded.  We could create a suitable function, but then
the assertion points to the function, which is less useful than pointing
to the test.  A macro could avoid that.

Can be cleaned up on top.

[...]