steveloughran commented on code in PR #5434:
URL: https://github.com/apache/hadoop/pull/5434#discussion_r1120154903


##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSAdmin.java:
##########
@@ -1238,36 +1240,44 @@ public void testAllDatanodesReconfig()
     when(reconfigurationUtil.parseChangedProperties(any(Configuration.class),
         any(Configuration.class))).thenReturn(changes);
 
-    assertEquals(0, admin.startReconfiguration("datanode", "livenodes"));
+    int result = admin.startReconfiguration("datanode", "livenodes");
+    Assertions.assertThat(result).isEqualTo(0);
     final List<String> outsForStartReconf = new ArrayList<>();
     final List<String> errsForStartReconf = new ArrayList<>();
     reconfigurationOutErrFormatter("startReconfiguration", "datanode",
         "livenodes", outsForStartReconf, errsForStartReconf);
-    assertEquals(3, outsForStartReconf.size());
-    assertEquals(0, errsForStartReconf.size());
-    assertTrue(outsForStartReconf.get(0).startsWith("Started reconfiguration 
task on node"));
-    assertTrue(outsForStartReconf.get(1).startsWith("Started reconfiguration 
task on node"));
-    assertEquals("Starting of reconfiguration task successful on 2 nodes, 
failed on 0 nodes.",
-        outsForStartReconf.get(2));
+    String started = "Started reconfiguration task on node";
+    String starting =
+        "Starting of reconfiguration task successful on 2 nodes, failed on 0 
nodes.";
+    Assertions.assertThat(outsForStartReconf.size()).isEqualTo(3);
+    Assertions.assertThat(errsForStartReconf.size()).isEqualTo(0);
+    Assertions.assertThat(outsForStartReconf.get(0)).startsWith(started);
+    Assertions.assertThat(outsForStartReconf.get(1)).startsWith(started);
+    Assertions.assertThat(outsForStartReconf.get(2)).startsWith(starting);
 
     Thread.sleep(1000);
     final List<String> outs = new ArrayList<>();
     final List<String> errs = new ArrayList<>();
     awaitReconfigurationFinished("datanode", "livenodes", outs, errs);
-    assertEquals(9, outs.size());
-    assertEquals(0, errs.size());
+    Assertions.assertThat(outs.size()).isEqualTo(9);
+    Assertions.assertThat(errs.size()).isEqualTo(0);
     LOG.info("dfsadmin -status -livenodes output:");
     outs.forEach(s -> LOG.info("{}", s));
-    assertTrue(outs.get(0).startsWith("Reconfiguring status for node"));
-    assertTrue("SUCCESS: Changed property 
dfs.datanode.peer.stats.enabled".equals(outs.get(2))
-        || "SUCCESS: Changed property 
dfs.datanode.peer.stats.enabled".equals(outs.get(1)));
-    assertTrue("\tFrom: \"false\"".equals(outs.get(3)) || "\tFrom: 
\"false\"".equals(outs.get(2)));
-    assertTrue("\tTo: \"true\"".equals(outs.get(4)) || "\tTo: 
\"true\"".equals(outs.get(3)));
-    assertEquals("SUCCESS: Changed property dfs.datanode.peer.stats.enabled", 
outs.get(5));
-    assertEquals("\tFrom: \"false\"", outs.get(6));
-    assertEquals("\tTo: \"true\"", outs.get(7));
-    assertEquals("Retrieval of reconfiguration status successful on 2 nodes, 
failed on 0 nodes.",
-        outs.get(8));
+    Assertions.assertThat(outs.get(0)).startsWith("Reconfiguring status for 
node");
+
+    String success = "SUCCESS: Changed property 
dfs.datanode.peer.stats.enabled";
+    String from = "\tFrom: \"false\"";
+    String to = "\tTo: \"true\"";
+    String retrieval =
+        "Retrieval of reconfiguration status successful on 2 nodes, failed on 
0 nodes.";
+
+    
Assertions.assertThat(Collections.singleton(success)).containsAnyOf(outs.get(1),
 outs.get(2));

Review Comment:
   i think it's better to assert that the list 
   ```
   assertThat(out.sublist(1, 4)).containsExactlyInAnyOrder(from, too, success)
   ```
   
   you could do something similar on sublist(5,9) with a 
`containsExactly(success,from,to,retrieval)` assert



##########
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/tools/TestDFSAdmin.java:
##########
@@ -1238,36 +1240,44 @@ public void testAllDatanodesReconfig()
     when(reconfigurationUtil.parseChangedProperties(any(Configuration.class),
         any(Configuration.class))).thenReturn(changes);
 
-    assertEquals(0, admin.startReconfiguration("datanode", "livenodes"));
+    int result = admin.startReconfiguration("datanode", "livenodes");
+    Assertions.assertThat(result).isEqualTo(0);
     final List<String> outsForStartReconf = new ArrayList<>();
     final List<String> errsForStartReconf = new ArrayList<>();
     reconfigurationOutErrFormatter("startReconfiguration", "datanode",
         "livenodes", outsForStartReconf, errsForStartReconf);
-    assertEquals(3, outsForStartReconf.size());
-    assertEquals(0, errsForStartReconf.size());
-    assertTrue(outsForStartReconf.get(0).startsWith("Started reconfiguration 
task on node"));
-    assertTrue(outsForStartReconf.get(1).startsWith("Started reconfiguration 
task on node"));
-    assertEquals("Starting of reconfiguration task successful on 2 nodes, 
failed on 0 nodes.",
-        outsForStartReconf.get(2));
+    String started = "Started reconfiguration task on node";
+    String starting =
+        "Starting of reconfiguration task successful on 2 nodes, failed on 0 
nodes.";
+    Assertions.assertThat(outsForStartReconf.size()).isEqualTo(3);

Review Comment:
   do an assert that the collection is of a given size, e.g 
   ```
   assertThat(outsForStartReconf).hasSize(3)
   ```
   the generated assert will then include the list of entries



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