[ 
https://issues.apache.org/jira/browse/HADOOP-13257?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15710143#comment-15710143
 ] 

Mingliang Liu commented on HADOOP-13257:
----------------------------------------

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
{code}
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       }
{code}
as
{code}
  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;
  }
{code}
# Checkstyle warnings are related if you run locally {{mvn checkstyle:check}} 
with this patch (I can't open the Jenkins pre-commit report)
{code}
[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.
{code}

> Improve Azure Data Lake contract tests.
> ---------------------------------------
>
>                 Key: HADOOP-13257
>                 URL: https://issues.apache.org/jira/browse/HADOOP-13257
>             Project: Hadoop Common
>          Issue Type: Sub-task
>            Reporter: Chris Nauroth
>            Assignee: Vishwajeet Dusane
>         Attachments: HADOOP-13257.001.patch
>
>
> 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.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

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

Reply via email to