[GitHub] [ignite-3] tkalkirill commented on a diff in pull request #1309: IGNITE-18065 Fix incorrect node start order in ItIgniteNodeRestartTest#testTwoNodesRestartReverse

[GitBox <gi...@apache.org>]

tkalkirill commented on code in PR #1309:
URL: https://github.com/apache/ignite-3/pull/1309#discussion_r1013698994


##########
modules/network/src/testFixtures/java/org/apache/ignite/utils/ClusterServiceTestUtils.java:
##########
@@ -56,29 +57,47 @@ public class ClusterServiceTestUtils {
     private static final TestScaleCubeClusterServiceFactory SERVICE_FACTORY = new TestScaleCubeClusterServiceFactory();
 
     /** Registry initializers collected via reflection. */

Review Comment:
   It may be worth mentioning that we are looking for methods here: `org.apache.ignite.network.serialization.MessageSerializationRegistry#registerFactory` ?



##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java:
##########
@@ -143,16 +140,20 @@ public class ItIgniteNodeRestartTest extends IgniteAbstractTest {
             + "  network.nodeFinder.netClusterNodes: {}\n"
             + "}";
 
-    /** Cluster nodes. */
-    private static final List<Ignite> CLUSTER_NODES = new ArrayList<>();
+    @InjectConfiguration
+    private static RaftConfiguration raftConfiguration;
 
-    private static final List<String> CLUSTER_NODES_NAMES = new ArrayList<>();
+    private final List<String> clusterNodesNames = new ArrayList<>();

Review Comment:
   Maybe you should use a concurrent collection?



##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java:
##########
@@ -180,27 +178,27 @@ public void afterEach() throws Exception {
     /**
      * Start some of Ignite components that are able to serve as Ignite node for test purposes.
      *
-     * @param name Node name.
      * @param cfgString Configuration string.
      * @return List of started components.
      */
-    private List<IgniteComponent> startPartialNode(String name, @Language("HOCON") String cfgString) throws NodeStoppingException {
-        return startPartialNode(name, cfgString, null);
+    private List<IgniteComponent> startPartialNode(int idx, @Nullable @Language("HOCON") String cfgString) {
+        return startPartialNode(idx, cfgString, null);
     }
 
     /**
      * Start some of Ignite components that are able to serve as Ignite node for test purposes.
      *
-     * @param name Node name.
      * @param cfgString Configuration string.
      * @param revisionCallback Callback on storage revision update.
      * @return List of started components.
      */
     private List<IgniteComponent> startPartialNode(
-            String name,
-            @Language("HOCON") String cfgString,
+            int idx,
+            @Nullable @Language("HOCON") String cfgString,

Review Comment:
   Missing mention of `null` in javadoc



##########
modules/runner/src/integrationTest/java/org/apache/ignite/internal/runner/app/ItIgniteNodeRestartTest.java:
##########
@@ -180,27 +178,27 @@ public void afterEach() throws Exception {
     /**
      * Start some of Ignite components that are able to serve as Ignite node for test purposes.
      *
-     * @param name Node name.
      * @param cfgString Configuration string.
      * @return List of started components.
      */
-    private List<IgniteComponent> startPartialNode(String name, @Language("HOCON") String cfgString) throws NodeStoppingException {
-        return startPartialNode(name, cfgString, null);
+    private List<IgniteComponent> startPartialNode(int idx, @Nullable @Language("HOCON") String cfgString) {
+        return startPartialNode(idx, cfgString, null);
     }
 
     /**
      * Start some of Ignite components that are able to serve as Ignite node for test purposes.
      *
-     * @param name Node name.
      * @param cfgString Configuration string.
      * @param revisionCallback Callback on storage revision update.
      * @return List of started components.
      */
     private List<IgniteComponent> startPartialNode(
-            String name,
-            @Language("HOCON") String cfgString,
+            int idx,

Review Comment:
   Missing javadoc



-- 
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-unsubscribe@ignite.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org