bitflicker64 commented on code in PR #3221: URL: https://github.com/apache/hugegraph/pull/3221#discussion_r4155691187
########## hugegraph-server/hugegraph-test/src/main/java/org/apache/hugegraph/api/ReadinessApiTest.java: ########## @@ -0,0 +1,71 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with this + * work for additional information regarding copyright ownership. The ASF + * licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the + * License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ + +package org.apache.hugegraph.api; + +import java.util.Map; + +import org.apache.hugegraph.testutil.Assert; +import org.apache.hugegraph.util.JsonUtil; +import org.junit.Test; + +import jakarta.ws.rs.client.ClientBuilder; +import jakarta.ws.rs.core.Response; + +/** + * The readiness endpoint answers 200 on a healthy server whatever the + * backend: "embedded" for the in-process backends of the API suite, "hstore" + * (with the storage fields) on the hstore job. + */ +public class ReadinessApiTest extends BaseApiTest { + + private static final String PATH = "/readiness"; + + @Test + public void testReadyOnAHealthyServer() { + Response r = client().get(PATH); + String result = assertResponseStatus(200, r); + Map<String, Object> body = JsonUtil.fromJson(result, Map.class); + Assert.assertEquals(true, body.get("ready")); + Assert.assertTrue(String.valueOf(body.get("storage")), + "embedded".equals(body.get("storage")) || + "hstore".equals(body.get("storage"))); Review Comment: Important: This assertion fails on the hbase API job after this round. `StorageReadiness.firstRemoteGraph()` now picks a graph whose backend is `hbase`, and `probeOnce()` writes `graph.backend()` into the body as `storage`. `HbaseStoreProvider.type()` returns `"hbase"`, so the body on that job is `{"ready":true,"storage":"hbase",...}` and `assertTrue` fails with message `hbase`. The storage field alone decides this, so the test fails even when the HBase probe itself succeeds. `server-ci.yml` runs `run-api-test.sh` for `memory`, `rocksdb` and `hbase`, and `ApiTestSuite` includes `ReadinessApiTest`. The round 5 reply says the HBase path is covered by this test on that job, but as written the job goes red instead. No CI has run on 7fa8020 yet because the branch conflicts with master, so this has not shown up. Requested change: accept `"hbase"` here and check the HBase fields the same way as the hstore ones, for example `reason` is `"ok"` and `hbase_millis` is present. Please also update the class Javadoc, which still lists only embedded and hstore. -- 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]
