rjgoyln commented on code in PR #11022:
URL: https://github.com/apache/ozone/pull/11022#discussion_r3785905326


##########
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestTransferLeadershipShell.java:
##########
@@ -100,29 +107,53 @@ public void testOmTransfer() throws Exception {
     assertOMResetPriorities();
   }
 
-  @Test
-  public void testScmTransfer() throws Exception {
-    StorageContainerManager oldLeader = getScmLeader(cluster);
-    List<StorageContainerManager> scmList = new ArrayList<>(cluster.
-        getStorageContainerManagersList());
-    assertThat(scmList).contains(oldLeader);
-    scmList.remove(oldLeader);
-    StorageContainerManager newLeader = scmList.get(0);
+  static Stream<Named<Function<StorageContainerManager, String>>> 
scmTargetIds() {
+    return Stream.of(
+        Named.of("scmId", StorageContainerManager::getScmId),
+        Named.of("nodeId", StorageContainerManager::getSCMNodeId));
+  }
 
-    String[] args1 = {"scm", "transfer", "-n", newLeader.getScmId()};
-    ozoneAdmin.execute(args1);
+  @ParameterizedTest
+  @MethodSource("scmTargetIds")
+  public void testScmTransfer(Function<StorageContainerManager, String> 
targetId) throws Exception {
+    StorageContainerManager newLeader = pickScmFollower();
+
+    String[] args = {"scm", "transfer", "-n", targetId.apply(newLeader)};
+    ozoneAdmin.execute(args);
     cluster.waitForClusterToBeReady();
     assertEquals(newLeader, getScmLeader(cluster));
     assertSCMResetPriorities();
+  }
 
-    oldLeader = getScmLeader(cluster);
-    String[] args3 = {"scm", "transfer", "-r"};
-    ozoneAdmin.execute(args3);
+  @Test
+  public void testScmTransferToRandomFollower() throws Exception {
+    StorageContainerManager oldLeader = getScmLeader(cluster);
+
+    String[] args = {"scm", "transfer", "-r"};
+    ozoneAdmin.execute(args);
     cluster.waitForClusterToBeReady();
     assertNotSame(oldLeader, getScmLeader(cluster));
     assertSCMResetPriorities();
   }
 
+  @Test
+  public void testScmTransferToUnknownIdFails() {

Review Comment:
   Good catch, thanks. I've added it and tightened the check slightly. Instead 
of matching only `no-such-scm`, it now checks for Target `no-such-scm not found 
in group`, ensuring the test verifies the resolution path rejected the ID. 
Happy to use the simpler form if preferred.



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


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to