[jira] [Work logged] (HDFS-16351) add path exception information in FSNamesystem

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/HDFS-16351?focusedWorklogId=690447&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-690447 ]

ASF GitHub Bot logged work on HDFS-16351:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 04/Dec/21 05:49
            Start Date: 04/Dec/21 05:49
    Worklog Time Spent: 10m 
      Work Description: ayushtkn commented on a change in pull request #3713:
URL: https://github.com/apache/hadoop/pull/3713#discussion_r762390099



##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestFSNamesystem.java
##########
@@ -120,6 +123,23 @@ public void testStartupSafemode() throws IOException {
       + "isInSafeMode still returned false",  fsn.isInSafeMode());
   }
 
+  @Test
+  public void testCheckAccess() throws IOException {
+    Configuration conf = new Configuration();
+    FSImage fsImage = Mockito.mock(FSImage.class);

Review comment:
       Remove this, one test is enough, we need not to mock and try

##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
##########
@@ -30,6 +30,7 @@
 import java.util.Map;
 import java.util.Random;
 
+import org.apache.hadoop.test.GenericTestUtils;

Review comment:
       import order seems wrong,
   the import should be in org.apache.hadoop. block with the others.

##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
##########
@@ -260,6 +261,33 @@ private void createAndCheckPermission(OpType op, Path name, short umask,
     checkPermission(name, expectedPermission, delete);
   }
 
+  @Test
+  public void testFSNamesystemCheckAccess() throws Exception {
+    Path testValidDir = new Path("/test1");
+    Path testValidFile = new Path("/test1/file1");
+    Path testInvalidPath = new Path("/test2");
+    fs = FileSystem.get(conf);
+
+    fs.mkdirs(testValidDir);
+    fs.create(testValidFile);
+
+    fs.access(testValidDir, FsAction.READ);
+    fs.access(testValidFile, FsAction.READ);
+
+    assertTrue(fs.exists(testValidDir));
+    assertTrue(fs.exists(testValidFile));
+
+    try {
+      fs.access(testInvalidPath, FsAction.READ);
+      fail("Failed to get expected FileNotFoundException");
+    } catch (FileNotFoundException e) {
+      GenericTestUtils.assertExceptionContains(
+              "Path not found: " + testInvalidPath, e);
+    } finally {
+      fs.delete(testValidDir, true);
+    }
+  }
+

Review comment:
       This is like testing normal fs.access also which isn't required, we just changed the exception, we can test that only.
   Can use LambdaTestUtils for that rather than the present try-catch. Something like this should do:
   ```
     @Test
     public void testFSNamesystemCheckAccess() throws Exception {
       Path testInvalidPath = new Path("/test2");
       fs = FileSystem.get(conf);
   
       LambdaTestUtils.intercept(FileNotFoundException.class,
           "Path not found: " + testInvalidPath,
           () -> fs.access(testInvalidPath, FsAction.READ));
     }
   ```

##########
File path: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSPermission.java
##########
@@ -289,7 +317,7 @@ public void testImmutableFsPermission() throws IOException {
     fs.setPermission(new Path("/"),
         FsPermission.createImmutable((short)0777));
   }
-  
+

Review comment:
       unrelated, revert




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


Issue Time Tracking
-------------------

    Worklog Id:     (was: 690447)
    Time Spent: 2h 10m  (was: 2h)

> add path exception information in FSNamesystem
> ----------------------------------------------
>
>                 Key: HDFS-16351
>                 URL: https://issues.apache.org/jira/browse/HDFS-16351
>             Project: Hadoop HDFS
>          Issue Type: Improvement
>          Components: hdfs
>    Affects Versions: 3.3.1
>            Reporter: guophilipse
>            Priority: Minor
>              Labels: pull-request-available
>          Time Spent: 2h 10m
>  Remaining Estimate: 0h
>
> add path information in exception message to make message more clear in FSNamesystem



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

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