[GitHub] [hadoop] ashutoshcipher commented on a diff in pull request #5243: MAPREDUCE-7428. Fix failing MapReduce tests due to the JUnit upgrades in WebServicesTestUtils

[GitBox <gi...@apache.org>]

ashutoshcipher commented on code in PR #5243:
URL: https://github.com/apache/hadoop/pull/5243#discussion_r1053272651


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/TestContainerLogsUtils.java:
##########
@@ -75,13 +73,12 @@ public static void createContainerLogFileInRemoteFS(Configuration conf,
     if (fs.exists(rootLogDirPath)) {
       fs.delete(rootLogDirPath, true);
     }
-    assertTrue(fs.mkdirs(rootLogDirPath));
+    fs.mkdirs(rootLogDirPath);
     Path appLogsDir = new Path(rootLogDirPath, appId.toString());
     if (fs.exists(appLogsDir)) {
       fs.delete(appLogsDir, true);
     }
-    assertTrue(fs.mkdirs(appLogsDir));
-
+    fs.mkdirs(appLogsDir);

Review Comment:
   > I thought we are happy because the directory has been removed in the previous line, but rethinking this, it's possible to fail removing the directory and then mkdir fails. Adding back the check in the next commit.
   
   Thanks for adding it back. It makes sense. LGTM



-- 
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: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org