imbajin commented on code in PR #318:
URL: https://github.com/apache/hugegraph-ai/pull/318#discussion_r3045985994
##########
hugegraph-python-client/src/tests/api/test_metric.py:
##########
@@ -63,8 +63,13 @@ def test_metrics_operations(self):
timers_metrics = self.metrics.get_timers_metrics()
self.assertIsInstance(timers_metrics, dict)
- system_metrics = self.metrics.get_system_metrics()
- self.assertIsInstance(system_metrics, dict)
+ # The current CI workflow still boots HugeGraph 1.3.0, where
+ # `/metrics/system` returns 500 in GitHub Actions. Keep the check for
+ # newer servers and leave the workflow upgrade as a separate TODO.
+ server_version = tuple(self.client.client.cfg.version)
Review Comment:
‼️ This gate currently turns a failed version probe into a false-green test:
`cfg.version` stays empty when `/versions` cannot be parsed or fetched, and
`tuple([]) >= (1, 5, 0)` is simply `False`, so we silently skip the
`/metrics/system` assertion even on environments where we meant to keep
coverage.
Could we make the incompatibility explicit instead of passing the test
without checking that endpoint?
```suggestion
server_version = tuple(self.client.client.cfg.version)
self.assertTrue(server_version, "failed to detect HugeGraph server
version")
if server_version < (1, 5, 0):
self.skipTest("HugeGraph < 1.5.0 returns 500 for /metrics/system
in CI")
system_metrics = self.metrics.get_system_metrics()
self.assertIsInstance(system_metrics, dict)
```
--
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]
