Copilot commented on code in PR #5950: URL: https://github.com/apache/ignite-3/pull/5950#discussion_r2120075319
########## modules/client/src/integrationTest/java/org/apache/ignite/client/compatibility/containers/IgniteServerContainer.java: ########## @@ -0,0 +1,69 @@ +/* + * 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.ignite.client.compatibility.containers; + +import java.util.concurrent.TimeUnit; +import java.util.function.Consumer; +import org.testcontainers.containers.GenericContainer; +import org.testcontainers.containers.output.OutputFrame; +import org.testcontainers.containers.wait.strategy.Wait; +import org.testcontainers.lifecycle.Startable; +import org.testcontainers.lifecycle.Startables; +import org.testcontainers.utility.DockerImageName; + +/** + * Container for an Ignite server instance. + * This container uses the Apache Ignite 3.0.0 image. + */ +public class IgniteServerContainer implements Startable { + private static final DockerImageName IGNITE_IMAGE = DockerImageName.parse("apacheignite/ignite"); + + private final GenericContainer<?> container; + + public IgniteServerContainer(String tag) { + this.container = createIgniteContainer(tag); + } + + @SuppressWarnings("resource") + private static GenericContainer<?> createIgniteContainer(String tag) { + Consumer<OutputFrame> logConsumer = outputFrame -> System.out.print(outputFrame.getUtf8String()); + + return new GenericContainer<>(IGNITE_IMAGE.withTag(tag)) + .withExposedPorts(10300, 10800) + .withLogConsumer(logConsumer) + .waitingFor(Wait.forLogMessage(".*Joining the cluster.*?", 1)); Review Comment: [nitpick] The regex uses a reluctant quantifier (".*?"), which may not match the full log message as intended; consider using ".*Joining the cluster.*" for a clearer, more reliable pattern. ```suggestion .waitingFor(Wait.forLogMessage(".*Joining the cluster.*", 1)); ``` ########## modules/client/src/integrationTest/java/org/apache/ignite/client/compatibility/AbstractCurrentClientWithOldServerCompatibilityTest.java: ########## @@ -0,0 +1,92 @@ +/* + * 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.ignite.client.compatibility; + +import static org.apache.ignite.internal.util.IgniteUtils.closeAll; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.HttpURLConnection; +import java.net.URL; +import org.apache.ignite.client.IgniteClient; +import org.apache.ignite.client.compatibility.containers.IgniteServerContainer; +import org.apache.ignite.internal.testframework.IgniteTestUtils; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; + +/** + * Tests that current Java client can work with all older server versions. + */ +public abstract class AbstractCurrentClientWithOldServerCompatibilityTest extends ClientCompatibilityTestBase { + private IgniteServerContainer serverContainer; + + abstract String serverVersion(); + + @BeforeAll + @Override + public void beforeAll() throws Exception { + serverContainer = new IgniteServerContainer(serverVersion()); + serverContainer.start(); + + activateCluster(serverContainer.restPort()); + waitForActivation(serverContainer.clientPort()); + + client = IgniteClient.builder() + .addresses("localhost:" + serverContainer.clientPort()) + .build(); + + super.beforeAll(); + } + + @AfterAll + void afterAll() throws Exception { + closeAll(client, serverContainer); + } + + private static void activateCluster(int restPort) throws IOException { + URL url = new URL("http://localhost:"; + restPort + "/management/v1/cluster/init"); + HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + conn.setRequestMethod("POST"); + conn.setRequestProperty("Content-Type", "application/json"); + conn.setDoOutput(true); + + String jsonInput = "{\"metaStorageNodes\": [\"defaultNode\"], \"clusterName\": \"myCluster\"}"; + + try (OutputStream os = conn.getOutputStream()) { + os.write(jsonInput.getBytes()); + os.flush(); + } + + int responseCode = conn.getResponseCode(); Review Comment: The HTTP activation response code is obtained but not validated; consider checking for a successful (200) status or throwing an exception to fail fast if activation fails. ```suggestion int responseCode = conn.getResponseCode(); if (responseCode != HttpURLConnection.HTTP_OK) { throw new IOException("Failed to activate cluster. HTTP response code: " + responseCode); } ``` ########## modules/client/src/integrationTest/java/org/apache/ignite/client/compatibility/AbstractCurrentClientWithOldServerCompatibilityTest.java: ########## @@ -0,0 +1,92 @@ +/* + * 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.ignite.client.compatibility; + +import static org.apache.ignite.internal.util.IgniteUtils.closeAll; + +import java.io.IOException; +import java.io.OutputStream; +import java.net.HttpURLConnection; +import java.net.URL; +import org.apache.ignite.client.IgniteClient; +import org.apache.ignite.client.compatibility.containers.IgniteServerContainer; +import org.apache.ignite.internal.testframework.IgniteTestUtils; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; + +/** + * Tests that current Java client can work with all older server versions. + */ +public abstract class AbstractCurrentClientWithOldServerCompatibilityTest extends ClientCompatibilityTestBase { + private IgniteServerContainer serverContainer; + + abstract String serverVersion(); + + @BeforeAll + @Override + public void beforeAll() throws Exception { + serverContainer = new IgniteServerContainer(serverVersion()); + serverContainer.start(); + + activateCluster(serverContainer.restPort()); + waitForActivation(serverContainer.clientPort()); + + client = IgniteClient.builder() + .addresses("localhost:" + serverContainer.clientPort()) + .build(); + + super.beforeAll(); + } + + @AfterAll + void afterAll() throws Exception { + closeAll(client, serverContainer); + } + + private static void activateCluster(int restPort) throws IOException { + URL url = new URL("http://localhost:"; + restPort + "/management/v1/cluster/init"); + HttpURLConnection conn = (HttpURLConnection) url.openConnection(); + conn.setRequestMethod("POST"); + conn.setRequestProperty("Content-Type", "application/json"); + conn.setDoOutput(true); + + String jsonInput = "{\"metaStorageNodes\": [\"defaultNode\"], \"clusterName\": \"myCluster\"}"; + + try (OutputStream os = conn.getOutputStream()) { + os.write(jsonInput.getBytes()); + os.flush(); + } + + int responseCode = conn.getResponseCode(); + System.out.println("Response Code: " + responseCode); Review Comment: [nitpick] Using System.out.println for logging in test setup may lead to inconsistent output; consider using a logger for structured and configurable logging. -- 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: notifications-unsubscr...@ignite.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
