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

[Thomas Huth]

On 28/11/2019 12.49, Markus Armbruster wrote:
> Tao Xu <address@hidden> writes:
>
> > Check configuring HMAT usecase
> >
> > Reviewed-by: Igor Mammedov <address@hidden>
> > Suggested-by: Igor Mammedov <address@hidden>
> > Signed-off-by: Tao Xu <address@hidden>
> > ---
> >
> > 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.

... maybe best to use g_assert_true() which can't be disabled and thus 
should be used in tests. See:

 https://developer.gnome.org/glib/unstable/glib-Testing.html#g-assert-true

 Thomas