Muawiya-contact commented on code in PR #342:
URL: https://github.com/apache/hugegraph-ai/pull/342#discussion_r3347712321


##########
hugegraph-python-client/src/tests/api/test_metric.py:
##########
@@ -74,10 +109,48 @@ def test_metrics_operations(self):
         self.assertIsInstance(statistics, dict)
 
         backend_metrics = self.metrics.get_backend_metrics()
-        # In HugeGraph 1.7.0+, the backend_metrics structure changed
-        # It's still a dict, but the "hugegraph" key may not exist in the same 
format
-        self.assertIsInstance(backend_metrics, dict)
-        self.assertTrue(backend_metrics, "backend metrics should not be empty")
-        # Only assert on the "hugegraph" key if it exists (for backward 
compatibility)
-        if "hugegraph" in backend_metrics:
-            self.assertGreater(len(backend_metrics["hugegraph"]), 1)
+
+        # HugeGraph 1.7.0 backend_metrics shape (top-level key may be 
"<cluster>-<graph>"):
+        # { "<cluster>-<graph>": { "backend": str, "nodes": int, "cluster_id": 
str,
+        #                          "servers": { "<server_name>": { <metrics> } 
} } }
+        self.assertIsInstance(backend_metrics, dict, "backend_metrics should 
be a dict")
+        self.assertTrue(backend_metrics, "backend_metrics should not be empty")
+
+        # Select the graph entry deterministically using the configured graph 
name
+        graph_name = ClientUtils.GRAPH
+        graph_key = next(
+            (k for k in backend_metrics if graph_name in k),
+            None,
+        )
+        self.assertIsNotNone(
+            graph_key,
+            f"Expected a key containing '{graph_name}' in backend_metrics, 
got: {list(backend_metrics.keys())}",
+        )
+
+        graph_entry = backend_metrics[graph_key]
+        self.assertIsInstance(graph_entry, dict)
+
+        # Assert required top-level fields in graph entry
+        self.assertIn("backend", graph_entry, "Missing 'backend' field")
+        self.assertIn("nodes", graph_entry, "Missing 'nodes' field")
+        self.assertIn("cluster_id", graph_entry, "Missing 'cluster_id' field")
+        self.assertIn("servers", graph_entry, "Missing 'servers' field")
+        self.assertIsInstance(graph_entry["backend"], str)
+        self.assertIsInstance(graph_entry["nodes"], int)
+        self.assertIsInstance(graph_entry["cluster_id"], str)
+        self.assertIsInstance(graph_entry["servers"], dict)
+
+        # Assert every server entry contains expected rocksdb metric keys
+        servers = graph_entry["servers"]
+        self.assertTrue(servers, "servers should not be empty")
+        for server_name, server_entry in servers.items():
+            self.assertIsInstance(
+                server_entry,
+                dict,
+                f"backend_metrics server entry for {server_name} should be a 
dict",
+            )
+            missing_keys = EXPECTED_BACKEND_SERVER_KEYS - 
set(server_entry.keys())

Review Comment:
   Thanks @imbajin! Fixed both — branched server key assertions by 
`graph_entry["backend"]` so RocksDB keys are only checked on `RocksDB` 
deployments, and other backends fall back to a non-empty dict check. Also fixed 
the graph key selection to use `self.client.GRAPH` with exact/suffix matching. 
Will keep an eye on #358 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to