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


##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/TestNewCombinerGrouping.java:
##########
@@ -105,7 +106,8 @@ public int compare(Text o1, Text o2) {
 
   @Test
   public void testCombiner() throws Exception {
-    if (!new File(TEST_ROOT_DIR).mkdirs()) {
+    File testDir = new File(TEST_ROOT_DIR);

Review Comment:
   best to do the fully delete here, so mkdirs always works, even if test was 
stopped partway through



##########
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/src/test/java/org/apache/hadoop/mapreduce/TestNewCombinerGrouping.java:
##########
@@ -174,6 +176,9 @@ public void testCombiner() throws Exception {
     } else {
       Assert.fail("Job failed");
     }
+    if (testDir.exists()) {

Review Comment:
   this doesn't get invoked if the job failed, and as there's no cleanup before 
L109, no recovery.



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