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

[Tao Xu]

On 11/28/2019 7:53 PM, Thomas Huth wrote:
On 28/11/2019 12.49, Markus Armbruster wrote:
Tao Xu <tao3...@intel.com> writes:
Check configuring HMAT usecase

Reviewed-by: Igor Mammedov <imamm...@redhat.com>
Suggested-by: Igor Mammedov <imamm...@redhat.com>
Signed-off-by: Tao Xu <tao3...@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.

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

Thank you for your suggestion. I will use g_assert_true and 
g_assert_false to replace g_assert