Details

      Description

      This JIRA to add test cases with CLI

      1. HDFS-6298.patch
        22 kB
        Yi Liu
      2. HDFS-6298.3.patch
        19 kB
        Yi Liu
      3. HDFS-6298.2.patch
        17 kB
        Yi Liu
      4. HDFS-6298.1.patch
        18 kB
        Yi Liu

        Activity

        Hide
        Uma Maheswara Rao G added a comment -

        I have just committed this to branch!

        Show
        Uma Maheswara Rao G added a comment - I have just committed this to branch!
        Hide
        Uma Maheswara Rao G added a comment -

        Thanks Yi for the patch update.
        +1 Patch looks good to me.

        Show
        Uma Maheswara Rao G added a comment - Thanks Yi for the patch update. +1 Patch looks good to me.
        Hide
        Yi Liu added a comment -

        Thanks Uma. OK, Let add the test cases of dir/file permissions for xattr in HDFS-6314.

        The new patch include 3 new end to end tests:

        setfattr : Add an xattr of trusted namespace
        setfattr : Add an xattr of system namespace
        setfattr : Add an xattr of security namespace

        Show
        Yi Liu added a comment - Thanks Uma. OK, Let add the test cases of dir/file permissions for xattr in HDFS-6314 . The new patch include 3 new end to end tests: setfattr : Add an xattr of trusted namespace setfattr : Add an xattr of system namespace setfattr : Add an xattr of security namespace
        Hide
        Uma Maheswara Rao G added a comment -

        Thanks a lot Yi for the update on patch. Actually my intention on test cases is for dir/file permissions based test cases. Ex: If a user does not have permission for a file, then he should not be able to set the xattrs for that file.
        Also I suggest to add tests for each namespace specified by user API. We allow only 2 namespaces from user API.

        Show
        Uma Maheswara Rao G added a comment - Thanks a lot Yi for the update on patch. Actually my intention on test cases is for dir/file permissions based test cases. Ex: If a user does not have permission for a file, then he should not be able to set the xattrs for that file. Also I suggest to add tests for each namespace specified by user API. We allow only 2 namespaces from user API.
        Hide
        Yi Liu added a comment -

        New end to end test is added:
        setfattr : Add an xattr which has wrong prefix

        Show
        Yi Liu added a comment - New end to end test is added: setfattr : Add an xattr which has wrong prefix
        Hide
        Yi Liu added a comment -

        Thanks Uma for review. The new patch includes update for your comments.

        Show
        Yi Liu added a comment - Thanks Uma for review. The new patch includes update for your comments.
        Hide
        Uma Maheswara Rao G added a comment -

        Overall patch looks good. Along with the Andrew comments, I have few comments here.

        assertTrue("Not a HDFS: "+fs.getUri(),
        +               fs instanceof DistributedFileSystem);
        

        format is off here.

        XAttrCommands.java and TestXAttrCommands.java should go into common JIRA as they are from common project. Lets file separate JIRS there and fix.

        From design doc:

         * For xattr of "user" namespace, its access permissions are 
         * defined by the file or directory permission bits. 
        

        I would like to see the permission related test cases to cover here.

        Show
        Uma Maheswara Rao G added a comment - Overall patch looks good. Along with the Andrew comments, I have few comments here. assertTrue( "Not a HDFS: " +fs.getUri(), + fs instanceof DistributedFileSystem); format is off here. XAttrCommands.java and TestXAttrCommands.java should go into common JIRA as they are from common project. Lets file separate JIRS there and fix. From design doc: * For xattr of "user" namespace, its access permissions are * defined by the file or directory permission bits. I would like to see the permission related test cases to cover here.
        Hide
        Yi Liu added a comment -

        Update end to end tests:

        1: setfattr : Add an xattr
        2: setfattr : Add an xattr, and encode is text
        3: setfattr : Add an xattr, and encode is hex
        4: setfattr : Add an xattr, and encode is base64
        5: setfattr : Add multiple xattrs
        6: setfattr : Remove an xattr
        7: setfattr : Remove an xattr which doesn't exist
        8: getfattr : Get an xattr
        9: getfattr : Get an xattr which doesn't exist
        10: getfattr : Get an xattr, and encode is text
        11: getfattr : Get an xattr, and encode is hex
        12: getfattr : Get an xattr, and encode is base64
        13: getfattr : Get an xattr, and encode is invalid
        14: getfattr -R : recursive

        Show
        Yi Liu added a comment - Update end to end tests: 1: setfattr : Add an xattr 2: setfattr : Add an xattr, and encode is text 3: setfattr : Add an xattr, and encode is hex 4: setfattr : Add an xattr, and encode is base64 5: setfattr : Add multiple xattrs 6: setfattr : Remove an xattr 7: setfattr : Remove an xattr which doesn't exist 8: getfattr : Get an xattr 9: getfattr : Get an xattr which doesn't exist 10: getfattr : Get an xattr, and encode is text 11: getfattr : Get an xattr, and encode is hex 12: getfattr : Get an xattr, and encode is base64 13: getfattr : Get an xattr, and encode is invalid 14: getfattr -R : recursive
        Hide
        Yi Liu added a comment -

        Rebase the code, the new patch includes update for comments.

        Show
        Yi Liu added a comment - Rebase the code, the new patch includes update for comments.
        Hide
        Yi Liu added a comment -

        Andrew, thanks a lot for review and comments.

        Let's restore System.err in an @After clause in TestXAttrCommands. The System.err stuff could be done in a @BeforeClass and @AfterClass if you want. We should reset errContent in an @After too.

        OK. Let's restore system.err and reset errContent in an @After clause.

        Higher-level comment, is TestXAttrCommands even necessary when we have the XML infrastructure? It seems like we could fold those into the XML test instead.

        I see your meaning. TestXAttrCommands is a more general test for fs shell input, doesn't rely on any file system. I'm inclined to keep them, could we?

        Maybe add tests for removing/getting a non-existent xattr, help commands?

        OK. Let's add them.

        Show
        Yi Liu added a comment - Andrew, thanks a lot for review and comments. Let's restore System.err in an @After clause in TestXAttrCommands. The System.err stuff could be done in a @BeforeClass and @AfterClass if you want. We should reset errContent in an @After too. OK. Let's restore system.err and reset errContent in an @After clause. Higher-level comment, is TestXAttrCommands even necessary when we have the XML infrastructure? It seems like we could fold those into the XML test instead. I see your meaning. TestXAttrCommands is a more general test for fs shell input, doesn't rely on any file system. I'm inclined to keep them, could we? Maybe add tests for removing/getting a non-existent xattr, help commands? OK. Let's add them.
        Hide
        Andrew Wang added a comment -

        Hi Yi, thanks for the patch. A few review comments:

        • Let's restore System.err in an @After clause in TestXAttrCommands. The System.err stuff could be done in a @BeforeClass and @AfterClass if you want. We should reset errContent in an @After too.
        • Higher-level comment, is TestXAttrCommands even necessary when we have the XML infrastructure? It seems like we could fold those into the XML test instead.
        • Maybe add tests for removing/getting a non-existent xattr, help commands?

        +1 once addressed.

        Show
        Andrew Wang added a comment - Hi Yi, thanks for the patch. A few review comments: Let's restore System.err in an @After clause in TestXAttrCommands. The System.err stuff could be done in a @BeforeClass and @AfterClass if you want. We should reset errContent in an @After too. Higher-level comment, is TestXAttrCommands even necessary when we have the XML infrastructure? It seems like we could fold those into the XML test instead. Maybe add tests for removing/getting a non-existent xattr, help commands? +1 once addressed.
        Hide
        Yi Liu added a comment -

        This test case can be used to verify the whole functionality.

        Show
        Yi Liu added a comment - This test case can be used to verify the whole functionality.
        Hide
        Yi Liu added a comment -

        1: setfattr : Add an xattr
        2: setfattr : Add an xattr, and encode is text
        3: setfattr : Add an xattr, and encode is hex
        4: setfattr : Add an xattr, and encode is base64
        5: setfattr : Add multiple xattrs
        6: setfattr : Remove an xattr
        7: getfattr : Get an xattr
        8: getfattr : Get an xattr, and encode is text
        9: getfattr : Get an xattr, and encode is hex
        10: getfattr : Get an xattr, and encode is base64
        11: getfattr : Get an xattr, and encode is invalid
        12: getfattr -R : recursive

        Show
        Yi Liu added a comment - 1: setfattr : Add an xattr 2: setfattr : Add an xattr, and encode is text 3: setfattr : Add an xattr, and encode is hex 4: setfattr : Add an xattr, and encode is base64 5: setfattr : Add multiple xattrs 6: setfattr : Remove an xattr 7: getfattr : Get an xattr 8: getfattr : Get an xattr, and encode is text 9: getfattr : Get an xattr, and encode is hex 10: getfattr : Get an xattr, and encode is base64 11: getfattr : Get an xattr, and encode is invalid 12: getfattr -R : recursive

          People

          • Assignee:
            Yi Liu
            Reporter:
            Uma Maheswara Rao G
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development