Phillippko commented on code in PR #4249:
URL: https://github.com/apache/ignite-3/pull/4249#discussion_r1723063524
##########
modules/runner/src/testFixtures/java/org/apache/ignite/internal/Cluster.java:
##########
@@ -231,31 +235,30 @@ private void startAndInit(
TestIgnitionManager.init(metaStorageAndCmgNodes.get(0),
builder.build());
- for (IgniteServer node : nodes) {
- assertThat(node.waitForInitAsync(), willCompleteSuccessfully());
+ for (ServerRegistration registration : nodeRegistrations) {
+ assertThat(registration.registrationFuture(),
willCompleteSuccessfully());
}
started = true;
}
/**
- * Starts a cluster node with the default bootstrap config template and
returns its startup future.
+ * Starts a cluster node with the default bootstrap config template and
returns it and its registation future.
Review Comment:
```suggestion
* Starts a cluster node with the default bootstrap config template and
returns it and its registration future.
```
And "return" part of javadoc is not changed
##########
modules/runner/src/testFixtures/java/org/apache/ignite/internal/Cluster.java:
##########
@@ -231,31 +235,30 @@ private void startAndInit(
TestIgnitionManager.init(metaStorageAndCmgNodes.get(0),
builder.build());
- for (IgniteServer node : nodes) {
- assertThat(node.waitForInitAsync(), willCompleteSuccessfully());
+ for (ServerRegistration registration : nodeRegistrations) {
+ assertThat(registration.registrationFuture(),
willCompleteSuccessfully());
}
started = true;
}
/**
- * Starts a cluster node with the default bootstrap config template and
returns its startup future.
+ * Starts a cluster node with the default bootstrap config template and
returns it and its registation future.
*
* @param nodeIndex Index of the node to start.
* @return Future that will be completed when the node starts.
*/
- public IgniteServer startEmbeddedNode(int nodeIndex) {
+ public ServerRegistration startEmbeddedNode(int nodeIndex) {
return startEmbeddedNode(nodeIndex,
defaultNodeBootstrapConfigTemplate);
}
/**
- * Starts a cluster node and returns its startup future.
+ * Starts a cluster node and returns it with its registration future.
*
* @param nodeIndex Index of the node to start.
* @param nodeBootstrapConfigTemplate Bootstrap config template to use for
this node.
- * @return Future that will be completed when the node starts.
Review Comment:
let's return "return" part))
##########
modules/security/src/integrationTest/java/org/apache/ignite/internal/ssl/ItSslTest.java:
##########
@@ -521,20 +521,20 @@ void incompatibleCiphersNodes(TestInfo testInfo) {
String sslEnabledWithCipher1BoostrapConfig =
createBoostrapConfig("TLS_AES_256_GCM_SHA384");
String sslEnabledWithCipher2BoostrapConfig =
createBoostrapConfig("TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384");
- IgniteServer node1 = incompatibleTestCluster.startEmbeddedNode(10,
sslEnabledWithCipher1BoostrapConfig);
+ ServerRegistration registration1 =
incompatibleTestCluster.startEmbeddedNode(10,
sslEnabledWithCipher1BoostrapConfig);
Review Comment:
```suggestion
ServerRegistration successfulRegistration =
incompatibleTestCluster.startEmbeddedNode(10,
sslEnabledWithCipher1BoostrapConfig);
```
##########
modules/security/src/integrationTest/java/org/apache/ignite/internal/ssl/ItSslTest.java:
##########
@@ -521,20 +521,20 @@ void incompatibleCiphersNodes(TestInfo testInfo) {
String sslEnabledWithCipher1BoostrapConfig =
createBoostrapConfig("TLS_AES_256_GCM_SHA384");
String sslEnabledWithCipher2BoostrapConfig =
createBoostrapConfig("TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384");
- IgniteServer node1 = incompatibleTestCluster.startEmbeddedNode(10,
sslEnabledWithCipher1BoostrapConfig);
+ ServerRegistration registration1 =
incompatibleTestCluster.startEmbeddedNode(10,
sslEnabledWithCipher1BoostrapConfig);
InitParameters initParameters = InitParameters.builder()
- .metaStorageNodes(node1)
+ .metaStorageNodes(registration1.server())
.clusterName("cluster")
.build();
- TestIgnitionManager.init(node1, initParameters);
+ TestIgnitionManager.init(registration1.server(), initParameters);
// First node will initialize the cluster with single node
successfully since the second node can't connect to it.
- assertThat(node1.waitForInitAsync(), willCompleteSuccessfully());
+ assertThat(registration1.registrationFuture(),
willCompleteSuccessfully());
- IgniteServer node2 = incompatibleTestCluster.startEmbeddedNode(11,
sslEnabledWithCipher2BoostrapConfig);
- assertThat(node2.waitForInitAsync(), willTimeoutIn(1,
TimeUnit.SECONDS));
+ ServerRegistration registration2 =
incompatibleTestCluster.startEmbeddedNode(11,
sslEnabledWithCipher2BoostrapConfig);
+ assertThat(registration2.registrationFuture(), willTimeoutIn(1,
TimeUnit.SECONDS));
Review Comment:
```suggestion
assertThat(failingRegistration.registrationFuture(),
willTimeoutIn(1, TimeUnit.SECONDS));
```
##########
modules/runner/src/testFixtures/java/org/apache/ignite/internal/Cluster.java:
##########
@@ -348,9 +352,9 @@ public Ignite startNode(int index, String
nodeBootstrapConfigTemplate) {
Ignite newIgniteNode;
try {
- IgniteServer node = startEmbeddedNode(index,
nodeBootstrapConfigTemplate);
- node.waitForInitAsync().get(20, TimeUnit.SECONDS);
- newIgniteNode = node.api();
+ ServerRegistration registration = startEmbeddedNode(index,
nodeBootstrapConfigTemplate);
+ registration.registrationFuture().get(20, TimeUnit.SECONDS);
Review Comment:
Why not "willSucceedIn"? Then we could get rid of exceptions
```suggestion
assertThat(registration.registrationFuture(), willSucceedIn(20,
TimeUnit.SECONDS));
```
--
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]
