Copilot commented on code in PR #3185:
URL: https://github.com/apache/hugegraph/pull/3185#discussion_r3949052379
##########
hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java:
##########
@@ -62,6 +62,56 @@ public void testQueryClusterInfo() throws
URISyntaxException, IOException, Inter
assert obj.getInt("status") == 0;
}
+ @Test
+ public void testHealthNeedsNoAuth() throws URISyntaxException, IOException,
+ InterruptedException {
+ String url = pdRestAddr + "/v1/health";
+ HttpRequest request = HttpRequest.newBuilder().uri(new
URI(url)).GET().build();
+ HttpResponse<String> response = client.send(request,
HttpResponse.BodyHandlers.ofString());
+ assert response.statusCode() == 200;
+ // A 200 alone does not prove the path is anonymous: as of 1.7.0 the
auth interceptor
+ // refuses with 200 and an error envelope. checkHealthy() returns an
empty body, which
+ // separates the two whichever status a refusal carries.
+ assert response.body().isEmpty() : "expected an empty body, got " +
response.body();
+ }
+
+ @Test
+ public void testReadyNeedsNoAuthAndReflectsRaft() throws
URISyntaxException, IOException,
+
InterruptedException, JSONException {
+ // The CI PD is a single-node raft group, so it is its own leader and
must be ready
+ String url = pdRestAddr + "/v1/ready";
+ HttpRequest request = HttpRequest.newBuilder().uri(new
URI(url)).GET().build();
+ HttpResponse<String> response = client.send(request,
HttpResponse.BodyHandlers.ofString());
+ assert response.statusCode() == 200 : "expected 200, got " +
response.statusCode() +
+ " body=" + response.body();
+ JSONObject obj = new JSONObject(response.body());
+ assert obj.getBoolean("ready");
+ assert obj.getBoolean("isLeader");
+ assert "STATE_LEADER".equals(obj.getString("state"));
+ // Unauthenticated, so it must not disclose cluster addresses
+ assert !obj.has("leader") : "the anonymous body must not carry the
leader address";
+ }
+
+ @Test
+ public void testRaftGaugesExported() throws URISyntaxException,
IOException,
+ InterruptedException {
+ String url = pdRestAddr + "/actuator/prometheus";
+ HttpRequest request = HttpRequest.newBuilder().uri(new
URI(url)).GET().build();
+ HttpResponse<String> response = client.send(request,
HttpResponse.BodyHandlers.ofString());
+ assert response.statusCode() == 200;
+ String body = response.body();
+ assert body.contains("hg_raft_leader{") : "missing hg_raft_leader
gauge";
+ assert body.contains("hg_raft_has_leader{") : "missing
hg_raft_has_leader gauge";
+ assert body.contains("hg_raft_alive_peers{") : "missing
hg_raft_alive_peers gauge";
+ // Single-node CI cluster: this PD is the leader and hears from itself
+ assert body.matches("(?s).*hg_raft_leader\\{[^}]*\\} 1\\.0.*") :
+ "hg_raft_leader should be 1 on a single-node leader";
+ assert body.matches("(?s).*hg_raft_has_leader\\{[^}]*\\} 1\\.0.*") :
+ "hg_raft_has_leader should be 1 on a single-node leader";
+ assert body.matches("(?s).*hg_raft_alive_peers\\{[^}]*\\} 1\\.0.*") :
+ "hg_raft_alive_peers should be 1 on a single-node leader";
Review Comment:
These checks assume the Prometheus exposition always includes a `{...}`
label block (e.g., `hg_raft_leader{...}`), but Micrometer may emit
`hg_raft_leader 1.0` when there are no tags. To avoid brittle failures across
registry/library versions, match the metric name with an optional `{...}`
section (and keep the value assertion).
##########
hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java:
##########
@@ -62,6 +62,56 @@ public void testQueryClusterInfo() throws
URISyntaxException, IOException, Inter
assert obj.getInt("status") == 0;
}
+ @Test
+ public void testHealthNeedsNoAuth() throws URISyntaxException, IOException,
+ InterruptedException {
+ String url = pdRestAddr + "/v1/health";
+ HttpRequest request = HttpRequest.newBuilder().uri(new
URI(url)).GET().build();
+ HttpResponse<String> response = client.send(request,
HttpResponse.BodyHandlers.ofString());
+ assert response.statusCode() == 200;
+ // A 200 alone does not prove the path is anonymous: as of 1.7.0 the
auth interceptor
+ // refuses with 200 and an error envelope. checkHealthy() returns an
empty body, which
+ // separates the two whichever status a refusal carries.
+ assert response.body().isEmpty() : "expected an empty body, got " +
response.body();
Review Comment:
Java `assert` statements are disabled unless the JVM runs with `-ea`, which
can make these tests silently pass in some build setups. Prefer JUnit
assertions (`Assert.assertEquals`, `Assert.assertTrue`, etc.) so failures are
always enforced regardless of JVM flags.
##########
hugegraph-server/hugegraph-dist/src/assembly/travis/test-start-hugegraph-pd.sh:
##########
@@ -111,6 +111,23 @@ wait_for_pd() {
return 1
}
+# Wait until PD readiness answers, or timeout. Same gate the docs recommend:
+# -f rejects the 503, the body match rejects a 200 that is an auth envelope
rather
+# than a readiness answer. No credentials, and a single-node group elects
itself.
+# Captured rather than piped: under pipefail a grep -q SIGPIPE could misread a
+# ready PD, and wait_for_pd() above uses the same shape.
+wait_for_pd_ready() {
+ local elapsed=0
+ while (( elapsed < STARTUP_WAIT )); do
+ local body
+ body=$(curl -fsS "$PD_URL/v1/ready" 2>/dev/null || true)
+ grep -q '"ready":true' <<<"$body" && return 0
Review Comment:
The readiness check matches the exact substring `"ready":true`, which will
fail if the JSON serializer ever outputs whitespace (e.g., `"ready": true`) or
pretty-printed JSON. Use a whitespace-tolerant match (e.g., `grep -Eq` with a
pattern that allows spaces) to make the probe resilient to formatting changes.
##########
hugegraph-pd/hg-pd-test/src/main/java/org/apache/hugegraph/pd/rest/RestApiTest.java:
##########
@@ -62,6 +62,56 @@ public void testQueryClusterInfo() throws
URISyntaxException, IOException, Inter
assert obj.getInt("status") == 0;
}
+ @Test
+ public void testHealthNeedsNoAuth() throws URISyntaxException, IOException,
+ InterruptedException {
+ String url = pdRestAddr + "/v1/health";
+ HttpRequest request = HttpRequest.newBuilder().uri(new
URI(url)).GET().build();
+ HttpResponse<String> response = client.send(request,
HttpResponse.BodyHandlers.ofString());
+ assert response.statusCode() == 200;
+ // A 200 alone does not prove the path is anonymous: as of 1.7.0 the
auth interceptor
+ // refuses with 200 and an error envelope. checkHealthy() returns an
empty body, which
+ // separates the two whichever status a refusal carries.
+ assert response.body().isEmpty() : "expected an empty body, got " +
response.body();
+ }
+
+ @Test
+ public void testReadyNeedsNoAuthAndReflectsRaft() throws
URISyntaxException, IOException,
+
InterruptedException, JSONException {
+ // The CI PD is a single-node raft group, so it is its own leader and
must be ready
+ String url = pdRestAddr + "/v1/ready";
+ HttpRequest request = HttpRequest.newBuilder().uri(new
URI(url)).GET().build();
+ HttpResponse<String> response = client.send(request,
HttpResponse.BodyHandlers.ofString());
+ assert response.statusCode() == 200 : "expected 200, got " +
response.statusCode() +
+ " body=" + response.body();
+ JSONObject obj = new JSONObject(response.body());
+ assert obj.getBoolean("ready");
+ assert obj.getBoolean("isLeader");
+ assert "STATE_LEADER".equals(obj.getString("state"));
+ // Unauthenticated, so it must not disclose cluster addresses
+ assert !obj.has("leader") : "the anonymous body must not carry the
leader address";
Review Comment:
Java `assert` statements are disabled unless the JVM runs with `-ea`, which
can make these tests silently pass in some build setups. Prefer JUnit
assertions (`Assert.assertEquals`, `Assert.assertTrue`, etc.) so failures are
always enforced regardless of JVM flags.
--
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]
