PakhomovAlexander commented on code in PR #1404:
URL: https://github.com/apache/ignite-3/pull/1404#discussion_r1038163388


##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/questions/ItConnectToClusterTest.java:
##########
@@ -132,6 +138,33 @@ void connectOnStartAndSave() throws IOException {
                 .isEqualTo("http://localhost:10300";);
     }
 
+    @Test
+    @DisplayName("Should ask to connect to different URL")
+    void connectToAnotherUrl() throws IOException {
+        // Given prompt before connect
+        String promptBefore = Ansi.OFF.string(promptProvider.getPrompt());
+        assertThat(promptBefore).isEqualTo("[disconnected]> ");
+
+        // And connected
+        execute("connect");
+
+        // And answer is "y"
+        bindAnswers("y");
+
+        // When connect to different URL
+        execute("connect", "http://localhost:10301";);
+
+        // Then
+        assertAll(
+                this::assertErrOutputIsEmpty,
+                () -> assertOutputIs("Connected to http://localhost:10300"; + 
System.lineSeparator()
+                + "Connected to http://localhost:10301"; + 
System.lineSeparator())

Review Comment:
   That looks weird... Say I am connected to http://localhost:10300 and want to 
switch to another node and type `connect http://localhost:10301` and see 
   
   ```
   Connected to http://localhost:10300
   Connected to http://localhost:10301
   ```
   
   It looks like a bug for the user. I suggest dropping the first line.



##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/connect/ItConnectCommandTest.java:
##########
@@ -107,6 +107,29 @@ void disconnect() {
         assertThat(promptAfter).isEqualTo("[disconnected]> ");
     }
 
+    @Test
+    @DisplayName("Should state that already connected")
+    void connectTwice() {
+        // Given connected to cluster
+        execute("connect");
+        // And prompt is
+        String promptBefore = Ansi.OFF.string(promptProvider.getPrompt());
+        assertThat(promptBefore).isEqualTo("[" + nodeName() + "]> ");
+
+        // When connect again
+        execute("connect");
+
+        // Then
+        assertAll(
+                this::assertErrOutputIsEmpty,
+                () -> assertOutputIs("Connected to http://localhost:10300"; + 
System.lineSeparator()
+                        + "You are already connected to 
http://localhost:10300"; + System.lineSeparator())

Review Comment:
   I think it is enough just to say `You are already connected to 
http://localhost:10300` without the first line.



-- 
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]

Reply via email to