[
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: [email protected]
For additional commands, e-mail: [email protected]
