rpuch commented on code in PR #3668:
URL: https://github.com/apache/ignite-3/pull/3668#discussion_r1587162843


##########
modules/cli/src/integrationTest/java/org/apache/ignite/internal/cli/commands/recovery/ItPartitionStatesTest.java:
##########
@@ -128,54 +169,88 @@ void testPartitionStatesMissingZone(boolean global) {
     void testPartitionStatesMissingPartition(boolean global) {
         String unknownPartition = "-1";
 
-        execute("recovery", "partition-states",
-                CLUSTER_URL_OPTION, NODE_URL,
+        execute(CLUSTER_URL_OPTION, NODE_URL,
                 RECOVERY_PARTITION_IDS_OPTION, unknownPartition,
-                global ? RECOVERY_PARTITION_GLOBAL_OPTION : 
RECOVERY_LOCAL_OPTION);
+                global ? RECOVERY_PARTITION_GLOBAL_OPTION : 
RECOVERY_PARTITION_LOCAL_OPTION,
+                PLAIN_OPTION
+        );
 
         assertErrOutputContains("Some partitions are missing: [-1]");
 
         assertOutputIsEmpty();
     }
 
-    @ParameterizedTest
-    @ValueSource(booleans = {false, true})
+    @Test
     void testPartitionStatesMissingNode() {
         String unknownNode = "unknown_node";
 
-        execute("recovery", "partition-states",
-                CLUSTER_URL_OPTION, NODE_URL,
+        execute(CLUSTER_URL_OPTION, NODE_URL,
                 RECOVERY_NODE_NAMES_OPTION, unknownNode,
-                RECOVERY_LOCAL_OPTION);
+                RECOVERY_PARTITION_LOCAL_OPTION, PLAIN_OPTION
+        );
 
         assertErrOutputContains("Some nodes are missing: [unknown_node]");
 
         assertOutputIsEmpty();
     }
 
-
     @ParameterizedTest
     @ValueSource(booleans = {false, true})
     void testPartitionStatesEmptyResult(boolean global) {
-        execute("recovery", "partition-states",
-                CLUSTER_URL_OPTION, NODE_URL,
-                RECOVERY_ZONE_NAMES_OPTION, "empty_zone",
-                global ? RECOVERY_PARTITION_GLOBAL_OPTION : 
RECOVERY_LOCAL_OPTION
+        execute(CLUSTER_URL_OPTION, NODE_URL,
+                RECOVERY_ZONE_NAMES_OPTION, EMPTY_ZONE,
+                global ? RECOVERY_PARTITION_GLOBAL_OPTION : 
RECOVERY_PARTITION_LOCAL_OPTION,
+                PLAIN_OPTION
         );
 
-        checkOutput(global, Set.of(), Set.of());
+        checkOutput(global, Set.of(), Set.of(), 0);
+    }
+
+    @Test
+    void testOutputFormatGlobal() {
+        String zoneName = ZONES.stream().findAny().get();
+
+        execute(CLUSTER_URL_OPTION, NODE_URL,
+                RECOVERY_PARTITION_GLOBAL_OPTION,
+                RECOVERY_ZONE_NAMES_OPTION, zoneName,
+                RECOVERY_PARTITION_IDS_OPTION, "1",
+                PLAIN_OPTION);
+
+        assertErrOutputIsEmpty();
+        assertOutputMatches(String.format(

Review Comment:
   Headers order is checked; is the same done for non-header columns? Will 
these tests catch a situation when we swap data columns, but forget to swap 
header columns, or vice versa? Or if the number of data columns does not match 
the number of the header columns?



##########
modules/table/src/main/java/org/apache/ignite/internal/table/distributed/disaster/DisasterRecoveryManager.java:
##########
@@ -575,6 +576,15 @@ private static Map<TablePartitionId, GlobalPartitionState> 
assembleGlobal(
                     return assembleGlobalStateFromLocal(catalog, 
tablePartitionId, map);
                 }));
 
+        if (partitionIds.isEmpty()) {

Review Comment:
   Why don't we do this 'make missing partitions unavailable' trick if we are 
given explicit partition IDs?



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