Copilot commented on code in PR #342:
URL: https://github.com/apache/hugegraph-ai/pull/342#discussion_r3333587440
##########
hugegraph-python-client/src/tests/api/test_metric.py:
##########
@@ -70,10 +105,45 @@ 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:
+ # { "DEFAULT-hugegraph": { "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")
+
+ # Assert top-level graph key exists
+ graph_keys = list(backend_metrics.keys())
+ self.assertGreaterEqual(
+ len(graph_keys),
+ 1,
+ f"Expected at least 1 graph key, got: {graph_keys}",
+ )
+
+ graph_entry = backend_metrics[graph_keys[0]]
+ self.assertIsInstance(graph_entry, dict)
Review Comment:
`graph_keys[0]` depends on the response/dict insertion order and may
validate the wrong graph if multiple entries exist. Since this test suite uses
`ClientUtils.GRAPH == "hugegraph"`, select the matching graph key
deterministically (exact match or cluster-prefixed "<cluster>-<graph>") and
assert it exists before validating its shape.
##########
hugegraph-python-client/src/tests/api/test_metric.py:
##########
@@ -70,10 +105,45 @@ 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:
+ # { "DEFAULT-hugegraph": { "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")
+
+ # Assert top-level graph key exists
+ graph_keys = list(backend_metrics.keys())
+ self.assertGreaterEqual(
+ len(graph_keys),
+ 1,
+ f"Expected at least 1 graph key, got: {graph_keys}",
+ )
+
+ graph_entry = backend_metrics[graph_keys[0]]
+ 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 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())
+ self.assertFalse(
+ missing_keys,
+ f"backend_metrics server entry for {server_name} missing
expected keys: {missing_keys}",
+ )
Review Comment:
The failure message for missing metric keys prints a raw `set`, which can
render in arbitrary order and makes test output non-deterministic. Sorting the
keys produces stable, easier-to-read assertion output.
##########
hugegraph-python-client/src/tests/api/test_metric.py:
##########
@@ -70,10 +105,45 @@ 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:
+ # { "DEFAULT-hugegraph": { "backend": str, "nodes": int, "cluster_id":
str,
+ # "servers": { "<server_name>": { <metrics> }
} } }
Review Comment:
The shape comment hard-codes the top-level key as `"DEFAULT-hugegraph"`, but
the key can vary (e.g., different cluster prefix). Updating the comment to
describe the key pattern avoids misleading documentation.
--
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]
