Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 3.0.0-alpha2
    • Component/s: fs/adl
    • Labels:
      None

      Description

      HADOOP-12875 provided the initial implementation of the FileSystem contract tests covering Azure Data Lake. This issue tracks subsequent improvements on those test suites for improved coverage and matching the specified semantics more closely.

      1. HADOOP-13257.001.patch
        61 kB
        Vishwajeet Dusane
      2. HADOOP-13257.002.patch
        61 kB
        Vishwajeet Dusane

        Issue Links

          Activity

          Hide
          cnauroth Chris Nauroth added a comment -

          Vishwajeet Dusane, I've taken the liberty of assigning this to you initially, but please feel free to unassign if that's not right.

          Here is a copy-paste of comments from Steve Loughran on HADOOP-12875.

          Various tests are being skipped with a "BUG" commentary. those are signs of where the FS doesn't behave as expected. It's generally acceptable to skip tests where the feature is unimplemented especially in the quirks of object stores —but not where things are fundamentally at odds with the filesystem specification As an example, the test testRmNonEmptyRootDirNonRecursive() is skipped. That test is verifying a core feature of the FS behaviour. Same for AbstractContractMkdirTest.testNoMkdirOverFile(), where we say "you cannot create a directory where there is a file".
          TestListStatus uses System.out. Replace with logging
          Contract tests must not be using fs.default.fs as source; makes it impossible to have >1 FS in a test module. Look at how hadoop-aws defines a separate FS URI for each FS in options like test.fs.s3n.name and test.fs.s3a.name
          I don't understand why every setup has been overridden with a check for the test being enabled. The AdlStorageContract class should subclass isEnabled() for this
          Add a test for a seek of a long negative number, expect it throw an EOFException. Your seek() bounds checking only checks for -1, which is only a small subset of the possible negative seek ranges. This test should actually go into AbstractContractSeekTest, as we should see what other filesystems get up to.
          Rather than skip some of AbstractContractMkdirTest, these MUST be fixed.
          In TestConfigurationSetting}, all the close() calls must be designed to complete even in the presence of assertion failures. Use try-with-resources.

          Show
          cnauroth Chris Nauroth added a comment - Vishwajeet Dusane , I've taken the liberty of assigning this to you initially, but please feel free to unassign if that's not right. Here is a copy-paste of comments from Steve Loughran on HADOOP-12875 . Various tests are being skipped with a "BUG" commentary. those are signs of where the FS doesn't behave as expected. It's generally acceptable to skip tests where the feature is unimplemented especially in the quirks of object stores —but not where things are fundamentally at odds with the filesystem specification As an example, the test testRmNonEmptyRootDirNonRecursive() is skipped. That test is verifying a core feature of the FS behaviour. Same for AbstractContractMkdirTest.testNoMkdirOverFile(), where we say "you cannot create a directory where there is a file". TestListStatus uses System.out. Replace with logging Contract tests must not be using fs.default.fs as source; makes it impossible to have >1 FS in a test module. Look at how hadoop-aws defines a separate FS URI for each FS in options like test.fs.s3n.name and test.fs.s3a.name I don't understand why every setup has been overridden with a check for the test being enabled. The AdlStorageContract class should subclass isEnabled() for this Add a test for a seek of a long negative number, expect it throw an EOFException. Your seek() bounds checking only checks for -1, which is only a small subset of the possible negative seek ranges. This test should actually go into AbstractContractSeekTest, as we should see what other filesystems get up to. Rather than skip some of AbstractContractMkdirTest, these MUST be fixed. In TestConfigurationSetting}, all the close() calls must be designed to complete even in the presence of assertion failures. Use try-with-resources.
          Hide
          vishwajeet.dusane Vishwajeet Dusane added a comment -

          Chris Douglas - I have updated patch with new live test and update to the existing live suite in v001 patch.

          Show
          vishwajeet.dusane Vishwajeet Dusane added a comment - Chris Douglas - I have updated patch with new live test and update to the existing live suite in v001 patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 18 new or modified test files.
          +1 mvninstall 7m 10s trunk passed
          +1 compile 0m 15s trunk passed
          +1 checkstyle 0m 12s trunk passed
          +1 mvnsite 0m 18s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 0m 22s trunk passed
          +1 javadoc 0m 11s trunk passed
          +1 mvninstall 0m 13s the patch passed
          +1 compile 0m 12s the patch passed
          +1 javac 0m 12s the patch passed
          -0 checkstyle 0m 9s hadoop-tools/hadoop-azure-datalake: The patch generated 8 new + 0 unchanged - 0 fixed = 8 total (was 0)
          +1 mvnsite 0m 15s the patch passed
          +1 mvneclipse 0m 10s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 26s the patch passed
          +1 javadoc 0m 9s the patch passed
          +1 unit 3m 29s hadoop-azure-datalake in the patch passed.
          +1 asflicense 0m 16s The patch does not generate ASF License warnings.
          15m 31s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13257
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12838751/HADOOP-13257.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux d65fa6d75b00 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 79448d4
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11057/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-azure-datalake.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11057/testReport/
          modules C: hadoop-tools/hadoop-azure-datalake U: hadoop-tools/hadoop-azure-datalake
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11057/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 14s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 18 new or modified test files. +1 mvninstall 7m 10s trunk passed +1 compile 0m 15s trunk passed +1 checkstyle 0m 12s trunk passed +1 mvnsite 0m 18s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 0m 22s trunk passed +1 javadoc 0m 11s trunk passed +1 mvninstall 0m 13s the patch passed +1 compile 0m 12s the patch passed +1 javac 0m 12s the patch passed -0 checkstyle 0m 9s hadoop-tools/hadoop-azure-datalake: The patch generated 8 new + 0 unchanged - 0 fixed = 8 total (was 0) +1 mvnsite 0m 15s the patch passed +1 mvneclipse 0m 10s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 26s the patch passed +1 javadoc 0m 9s the patch passed +1 unit 3m 29s hadoop-azure-datalake in the patch passed. +1 asflicense 0m 16s The patch does not generate ASF License warnings. 15m 31s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13257 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12838751/HADOOP-13257.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux d65fa6d75b00 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 79448d4 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/11057/artifact/patchprocess/diff-checkstyle-hadoop-tools_hadoop-azure-datalake.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11057/testReport/ modules C: hadoop-tools/hadoop-azure-datalake U: hadoop-tools/hadoop-azure-datalake Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11057/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks for providing a patch. It seems addresses all Steve's previous comments and looks good to me overall. I find it interesting when I read through fillUnicodes() methods. I also like the idea of using Parameterized tests.

          1. I don't have a Azure subscription;did you finish a successful full test run integrated against the Azure Data Lake back-end with this patch?
          2. In TestAdlSupportedCharsetInPath, is failureReport ever reported? Naming private helper methods assertTrue and assertFalse may be confusing with JUnit methods. We'd choose different names.
          3. In the TestMetadata.java, we can make the parent a static variable as it's used in all test cases.
          4. Many asserEquals() calls should use the pattern assertEquals(expected, actual) or else the failing message will be confusing.
          5. License statement in TestAdlPermissionLive is ill-formatted.
          6. When generating Parameterized.Parameters, can we use loops? They're clearer for covering different cases.
          7. The follow methods can be simplified
            283	  private boolean contains(FileStatus[] statuses, String remotePath) {
            284	    boolean contains = false;
            285	    for (FileStatus status : statuses) {
            286	
            287	      if (status.getPath().toString().equals(remotePath)) {
            288	        contains = true;
            289	      }
            290	    }
            291	
            292	    if (!contains) {
            293	      for (FileStatus status : statuses) {
            294	        LOG.debug("Directory Content");
            295	        LOG.debug(status.getPath().toString());
            296	      }
            297	    }
            298	
            299	    return contains;
            300	  }
            

            as

              private boolean contains(FileStatus[] statuses, String remotePath) {
                for (FileStatus status : statuses) {
                  LOG.debug("Directory Content: {}", status.getPath());
                  if (status.getPath().toString().equals(remotePath)) {
                    return true;
                  }
                }
                return false;
              }
            
          8. Checkstyle warnings are related if you run locally mvn checkstyle:check with this patch (I can't open the Jenkins pre-commit report)
            [ERROR] src/main/java/org/apache/hadoop/fs/adl/AdlFileSystem.java[473] (blocks) NeedBraces: 'if' construct must use '{}'s.
            [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java[28:8] (imports) UnusedImports: Unused import - org.apache.hadoop.fs.Path.
            [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java[29:8] (imports) UnusedImports: Unused import - org.apache.hadoop.fs.permission.FsPermission.
            [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java[30:8] (imports) UnusedImports: Unused import - org.apache.hadoop.security.AccessControlException.
            [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java[31:8] (imports) UnusedImports: Unused import - org.junit.Assert.
            [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java[40:15] (imports) UnusedImports: Unused import - org.apache.hadoop.fs.FileContextTestHelper.exists.
            [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileSystemContractLive.java[25:8] (imports) UnusedImports: Unused import - org.apache.hadoop.security.AccessControlException.
            [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileSystemContractLive.java[28:8] (imports) UnusedImports: Unused import - org.junit.Ignore.
            
          Show
          liuml07 Mingliang Liu added a comment - Thanks for providing a patch. It seems addresses all Steve's previous comments and looks good to me overall. I find it interesting when I read through fillUnicodes() methods. I also like the idea of using Parameterized tests. I don't have a Azure subscription;did you finish a successful full test run integrated against the Azure Data Lake back-end with this patch? In TestAdlSupportedCharsetInPath , is failureReport ever reported? Naming private helper methods assertTrue and assertFalse may be confusing with JUnit methods. We'd choose different names. In the TestMetadata.java , we can make the parent a static variable as it's used in all test cases. Many asserEquals() calls should use the pattern assertEquals(expected, actual) or else the failing message will be confusing. License statement in TestAdlPermissionLive is ill-formatted. When generating Parameterized.Parameters , can we use loops? They're clearer for covering different cases. The follow methods can be simplified 283 private boolean contains(FileStatus[] statuses, String remotePath) { 284 boolean contains = false ; 285 for (FileStatus status : statuses) { 286 287 if (status.getPath().toString().equals(remotePath)) { 288 contains = true ; 289 } 290 } 291 292 if (!contains) { 293 for (FileStatus status : statuses) { 294 LOG.debug( "Directory Content" ); 295 LOG.debug(status.getPath().toString()); 296 } 297 } 298 299 return contains; 300 } as private boolean contains(FileStatus[] statuses, String remotePath) { for (FileStatus status : statuses) { LOG.debug( "Directory Content: {}" , status.getPath()); if (status.getPath().toString().equals(remotePath)) { return true ; } } return false ; } Checkstyle warnings are related if you run locally mvn checkstyle:check with this patch (I can't open the Jenkins pre-commit report) [ERROR] src/main/java/org/apache/hadoop/fs/adl/AdlFileSystem.java[473] (blocks) NeedBraces: ' if ' construct must use '{}'s. [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java[28:8] (imports) UnusedImports: Unused import - org.apache.hadoop.fs.Path. [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java[29:8] (imports) UnusedImports: Unused import - org.apache.hadoop.fs.permission.FsPermission. [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java[30:8] (imports) UnusedImports: Unused import - org.apache.hadoop.security.AccessControlException. [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java[31:8] (imports) UnusedImports: Unused import - org.junit.Assert. [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java[40:15] (imports) UnusedImports: Unused import - org.apache.hadoop.fs.FileContextTestHelper.exists. [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileSystemContractLive.java[25:8] (imports) UnusedImports: Unused import - org.apache.hadoop.security.AccessControlException. [ERROR] src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileSystemContractLive.java[28:8] (imports) UnusedImports: Unused import - org.junit.Ignore.
          Hide
          vishwajeet.dusane Vishwajeet Dusane added a comment -

          Thanks Mingliang Liu for taking some time to review and the feedback.

          1. I don't have a Azure subscription;did you finish a successful full test run integrated against the Azure Data Lake back-end with this patch?

          i do have internal Jenkins setup to execute contract test periodically against Azure Data Lake back-end. All test are passing consistently. This patch do have dependency on HDFS-11132. Steve Loughran has a support for the change, Could Steve Loughran, Chris Douglas, Chris Nauroth or Mingliang Liu please commit HDFS-11132, in case no further comment to address HDFS-11132 to unblock this patch.

          2. In TestAdlSupportedCharsetInPath, is failureReport ever reported? Naming private helper ...

          Good catch, failureReport is no longer needed. I had added it earlier to get collective report after concurrent test execution. So as assertTrue and assertFalse. I will incorporate the comment and update patch.

          3. In the TestMetadata.java, we can make the parent a static variable as it's used in all test cases.

          Yes, I will make parent as member variable to limit its scope to TestMetadata.java.

          6. When generating Parameterized.Parameters, can we use loops? They're clearer for covering different cases.

          Sorry i doubt if i understood the comment, Could you please clarify?

          7. The follow methods can be simplified

          Initially i had the simplified version of the code you proposed. Issue faced, output was flood with logs since TestAdlSupportedCharsetInPath has 470+ test. Hence added check to dump the log only when not found.

          Checkstyle warnings are related if you run ...

          Ohh i missed to see these warning, i noticed +1 from jenkins and ignore to read through the report. Thanks Mingliang Liu for highlighting.

          Show
          vishwajeet.dusane Vishwajeet Dusane added a comment - Thanks Mingliang Liu for taking some time to review and the feedback. 1. I don't have a Azure subscription;did you finish a successful full test run integrated against the Azure Data Lake back-end with this patch? i do have internal Jenkins setup to execute contract test periodically against Azure Data Lake back-end. All test are passing consistently. This patch do have dependency on HDFS-11132 . Steve Loughran has a support for the change, Could Steve Loughran , Chris Douglas , Chris Nauroth or Mingliang Liu please commit HDFS-11132 , in case no further comment to address HDFS-11132 to unblock this patch. 2. In TestAdlSupportedCharsetInPath, is failureReport ever reported? Naming private helper ... Good catch, failureReport is no longer needed. I had added it earlier to get collective report after concurrent test execution. So as assertTrue and assertFalse . I will incorporate the comment and update patch. 3. In the TestMetadata.java, we can make the parent a static variable as it's used in all test cases. Yes, I will make parent as member variable to limit its scope to TestMetadata.java . 6. When generating Parameterized.Parameters, can we use loops? They're clearer for covering different cases. Sorry i doubt if i understood the comment, Could you please clarify? 7. The follow methods can be simplified Initially i had the simplified version of the code you proposed. Issue faced, output was flood with logs since TestAdlSupportedCharsetInPath has 470+ test. Hence added check to dump the log only when not found. Checkstyle warnings are related if you run ... Ohh i missed to see these warning, i noticed +1 from jenkins and ignore to read through the report. Thanks Mingliang Liu for highlighting.
          Hide
          liuml07 Mingliang Liu added a comment -

          Hi Vishwajeet Dusane,

          I just committed the HDFS-11132.

          Thanks,

          Show
          liuml07 Mingliang Liu added a comment - Hi Vishwajeet Dusane , I just committed the HDFS-11132 . Thanks,
          Hide
          liuml07 Mingliang Liu added a comment -

          6. When generating Parameterized.Parameters, can we use loops? They're clearer for covering different cases.
          Sorry i doubt if i understood the comment, Could you please clarify?

          For example, when generating parameters in TestAdlPermissionLive,

          63	  @Parameterized.Parameters(name = "{0}")
          64	  public static Collection adlCreateNonRecursiveTestData()
          65	      throws UnsupportedEncodingException {
          66	    /*
          67	      Test Data
          68	      File/Folder name, User permission, Group permission, Other Permission,
          69	      Parent already exist
          70	      shouldCreateSucceed, expectedExceptionIfFileCreateFails
          71	    */
          72	    return Arrays.asList(new Object[][] {
          73	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL,
          74	            FsAction.ALL)},
          75	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL,
          76	            FsAction.NONE)},
          77	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL,
          78	            FsAction.EXECUTE)},
          79	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL,
          80	            FsAction.READ_EXECUTE)},
          81	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL,
          82	            FsAction.WRITE_EXECUTE)},
          83	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL,
          84	            FsAction.WRITE)},
          85	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL,
          86	            FsAction.READ)},
          87	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL,
          88	            FsAction.READ_WRITE)},
          89	
          90	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL,
          91	            FsAction.ALL)},
          92	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL,
          93	            FsAction.EXECUTE, FsAction.NONE)},
          94	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL,
          95	            FsAction.READ_EXECUTE, FsAction.NONE)},
          96	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL,
          97	            FsAction.WRITE_EXECUTE, FsAction.NONE)},
          98	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL,
          99	            FsAction.WRITE, FsAction.NONE)},
          100	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.READ,
          101	            FsAction.NONE)},
          102	        {new TestData(UUID.randomUUID().toString(), FsAction.ALL,
          103	            FsAction.READ_WRITE, FsAction.NONE)}});
          104	  }
          

          can be some code like (may need to change it):

            public static Collection adlCreateNonRecursiveTestData()
                throws UnsupportedEncodingException {
              final Collection<TestData> datas = new ArrayList<>();
              for (FsAction g : FsAction.values()) {
                for (FsAction o : FsAction.values()) {
                  datas.add(new TestData(UUID.randomUUID().toString(), FsAction.ALL, g, o));
                }
              }
              return datas;
            }
          

          Initially i had the simplified version of the code you proposed. Issue faced, output was flood with logs since TestAdlSupportedCharsetInPath has 470+ test. Hence added check to dump the log only when not found.

          You can still return early.

            private boolean contains(FileStatus[] statuses, String remotePath) {
              for (FileStatus status : statuses) {
                if (status.getPath().toString().equals(remotePath)) {
                  return true;
                }
              }
              for (FileStatus status : statuses) {
                LOG.debug("Directory Content: {}", status.getPath());
              }
              return false;
            }
          

          By the way, if you love lambda, you can use following code:

            private boolean contains(FileStatus[] statuses, String remotePath) {
              for (FileStatus status : statuses) {
                if (status.getPath().toString().equals(remotePath)) {
                  return true;
                }
              }
              Arrays.stream(statuses).forEach(s -> LOG.info(s.getPath().toString()));
              return false;
            }
          
          Show
          liuml07 Mingliang Liu added a comment - 6. When generating Parameterized.Parameters, can we use loops? They're clearer for covering different cases. Sorry i doubt if i understood the comment, Could you please clarify? For example, when generating parameters in TestAdlPermissionLive , 63 @Parameterized.Parameters(name = "{0}" ) 64 public static Collection adlCreateNonRecursiveTestData() 65 throws UnsupportedEncodingException { 66 /* 67 Test Data 68 File/Folder name, User permission, Group permission, Other Permission, 69 Parent already exist 70 shouldCreateSucceed, expectedExceptionIfFileCreateFails 71 */ 72 return Arrays.asList( new Object [][] { 73 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL, 74 FsAction.ALL)}, 75 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL, 76 FsAction.NONE)}, 77 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL, 78 FsAction.EXECUTE)}, 79 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL, 80 FsAction.READ_EXECUTE)}, 81 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL, 82 FsAction.WRITE_EXECUTE)}, 83 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL, 84 FsAction.WRITE)}, 85 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL, 86 FsAction.READ)}, 87 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL, 88 FsAction.READ_WRITE)}, 89 90 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.ALL, 91 FsAction.ALL)}, 92 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, 93 FsAction.EXECUTE, FsAction.NONE)}, 94 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, 95 FsAction.READ_EXECUTE, FsAction.NONE)}, 96 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, 97 FsAction.WRITE_EXECUTE, FsAction.NONE)}, 98 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, 99 FsAction.WRITE, FsAction.NONE)}, 100 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, FsAction.READ, 101 FsAction.NONE)}, 102 { new TestData(UUID.randomUUID().toString(), FsAction.ALL, 103 FsAction.READ_WRITE, FsAction.NONE)}}); 104 } can be some code like (may need to change it): public static Collection adlCreateNonRecursiveTestData() throws UnsupportedEncodingException { final Collection<TestData> datas = new ArrayList<>(); for (FsAction g : FsAction.values()) { for (FsAction o : FsAction.values()) { datas.add( new TestData(UUID.randomUUID().toString(), FsAction.ALL, g, o)); } } return datas; } Initially i had the simplified version of the code you proposed. Issue faced, output was flood with logs since TestAdlSupportedCharsetInPath has 470+ test. Hence added check to dump the log only when not found. You can still return early. private boolean contains(FileStatus[] statuses, String remotePath) { for (FileStatus status : statuses) { if (status.getPath().toString().equals(remotePath)) { return true ; } } for (FileStatus status : statuses) { LOG.debug( "Directory Content: {}" , status.getPath()); } return false ; } By the way, if you love lambda, you can use following code: private boolean contains(FileStatus[] statuses, String remotePath) { for (FileStatus status : statuses) { if (status.getPath().toString().equals(remotePath)) { return true ; } } Arrays.stream(statuses).forEach(s -> LOG.info(s.getPath().toString())); return false ; }
          Hide
          vishwajeet.dusane Vishwajeet Dusane added a comment -

          Incorporated review comments from Mingliang Liu.

          Show
          vishwajeet.dusane Vishwajeet Dusane added a comment - Incorporated review comments from Mingliang Liu .
          Hide
          vishwajeet.dusane Vishwajeet Dusane added a comment -

          Thanks for clarification Mingliang Liu and +1 for the comment on loop through FsAction.values().

          Show
          vishwajeet.dusane Vishwajeet Dusane added a comment - Thanks for clarification Mingliang Liu and +1 for the comment on loop through FsAction.values() .
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 13s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 20 new or modified test files.
          +1 mvninstall 18m 22s trunk passed
          +1 compile 0m 16s trunk passed
          +1 checkstyle 0m 11s trunk passed
          +1 mvnsite 0m 18s trunk passed
          +1 mvneclipse 0m 17s trunk passed
          +1 findbugs 0m 23s trunk passed
          +1 javadoc 0m 11s trunk passed
          +1 mvninstall 0m 13s the patch passed
          +1 compile 0m 12s the patch passed
          +1 javac 0m 12s the patch passed
          +1 checkstyle 0m 9s the patch passed
          +1 mvnsite 0m 14s the patch passed
          +1 mvneclipse 0m 9s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 0m 25s the patch passed
          +1 javadoc 0m 9s the patch passed
          +1 unit 3m 28s hadoop-azure-datalake in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          26m 48s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue HADOOP-13257
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841428/HADOOP-13257.002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 19c50b5490fe 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / c87b3a4
          Default Java 1.8.0_111
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11182/testReport/
          modules C: hadoop-tools/hadoop-azure-datalake U: hadoop-tools/hadoop-azure-datalake
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11182/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          hadoopqa Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 reexec 0m 13s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 20 new or modified test files. +1 mvninstall 18m 22s trunk passed +1 compile 0m 16s trunk passed +1 checkstyle 0m 11s trunk passed +1 mvnsite 0m 18s trunk passed +1 mvneclipse 0m 17s trunk passed +1 findbugs 0m 23s trunk passed +1 javadoc 0m 11s trunk passed +1 mvninstall 0m 13s the patch passed +1 compile 0m 12s the patch passed +1 javac 0m 12s the patch passed +1 checkstyle 0m 9s the patch passed +1 mvnsite 0m 14s the patch passed +1 mvneclipse 0m 9s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 0m 25s the patch passed +1 javadoc 0m 9s the patch passed +1 unit 3m 28s hadoop-azure-datalake in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 26m 48s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue HADOOP-13257 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12841428/HADOOP-13257.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 19c50b5490fe 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18 21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / c87b3a4 Default Java 1.8.0_111 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/11182/testReport/ modules C: hadoop-tools/hadoop-azure-datalake U: hadoop-tools/hadoop-azure-datalake Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/11182/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          liuml07 Mingliang Liu added a comment -

          Thanks for updating the patch; congrats on a clean Jenkins pre-commit run.

          I'll review and/or commit this today.

          Show
          liuml07 Mingliang Liu added a comment - Thanks for updating the patch; congrats on a clean Jenkins pre-commit run. I'll review and/or commit this today.
          Hide
          liuml07 Mingliang Liu added a comment -

          +1

          Committed to trunk branch. Thanks for your contribution, Vishwajeet Dusane.

          Show
          liuml07 Mingliang Liu added a comment - +1 Committed to trunk branch. Thanks for your contribution, Vishwajeet Dusane .
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10933 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10933/)
          HADOOP-13257. Improve Azure Data Lake contract tests. Contributed by (liuml07: rev 4113ec5fa5ca049ebaba039b1faf3911c6a34f7b)

          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractOpenLive.java
          • (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestMetadata.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/TestAdlRead.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileSystemContractLive.java
          • (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlSupportedCharsetInPath.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractRenameLive.java
          • (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlPermissionLive.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/main/java/org/apache/hadoop/fs/adl/AdlFileSystem.java
          • (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractRootDirLive.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlDifferentSizeWritesLive.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractAppendLive.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractMkdirLive.java
          • (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextCreateMkdirLive.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractConcatLive.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractDeleteLive.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractCreateLive.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractSeekLive.java
          • (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractGetFileStatusLive.java
          • (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlInternalCreateNonRecursive.java
          • (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/TestListStatus.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10933 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10933/ ) HADOOP-13257 . Improve Azure Data Lake contract tests. Contributed by (liuml07: rev 4113ec5fa5ca049ebaba039b1faf3911c6a34f7b) (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractOpenLive.java (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestMetadata.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/TestAdlRead.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileSystemContractLive.java (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlSupportedCharsetInPath.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractRenameLive.java (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlPermissionLive.java (edit) hadoop-tools/hadoop-azure-datalake/src/main/java/org/apache/hadoop/fs/adl/AdlFileSystem.java (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextMainOperationsLive.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractRootDirLive.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlDifferentSizeWritesLive.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractAppendLive.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractMkdirLive.java (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlFileContextCreateMkdirLive.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractConcatLive.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractDeleteLive.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractCreateLive.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractSeekLive.java (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlContractGetFileStatusLive.java (add) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/live/TestAdlInternalCreateNonRecursive.java (edit) hadoop-tools/hadoop-azure-datalake/src/test/java/org/apache/hadoop/fs/adl/TestListStatus.java
          Hide
          vishwajeet.dusane Vishwajeet Dusane added a comment -

          Thanks a lot Mingliang Liu for code feedback and pushing this patch through. Also Chris Douglas, Chris Nauroth, Steve Loughran for valuable feedback and support.

          Show
          vishwajeet.dusane Vishwajeet Dusane added a comment - Thanks a lot Mingliang Liu for code feedback and pushing this patch through. Also Chris Douglas , Chris Nauroth , Steve Loughran for valuable feedback and support.

            People

            • Assignee:
              vishwajeet.dusane Vishwajeet Dusane
              Reporter:
              cnauroth Chris Nauroth
            • Votes:
              0 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development