-
Notifications
You must be signed in to change notification settings - Fork 9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HADOOP-19446. ABFS: [FnsOverBlob][Tests] Add Tests For Negative Scenarios Identified for Delete Operation #7376
base: trunk
Are you sure you want to change the base?
Conversation
🎊 +1 overall
This message was automatically generated. |
Test Results============================================================
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added some thoughts.
Assertions.assertThat(testClient.deleteIdempotencyCheckOp(op) | ||
.getResult() | ||
.getStatusCode()) | ||
.describedAs( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix indentation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
@@ -286,6 +304,7 @@ public void testDeleteIdempotencyTriggerHttp404() throws Exception { | |||
doReturn(idempotencyRetOp).when(mockClient).deleteIdempotencyCheckOp(any()); | |||
TracingContext tracingContext = getTestTracingContext(fs, false); | |||
doReturn(tracingContext).when(idempotencyRetOp).createNewTracingContext(any()); | |||
//To-discuss: If DFS service type, this if case can be removed? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this about?
fs.create(p1); | ||
fs.create(p2); | ||
AbfsBlobClient client = (AbfsBlobClient) fs.getAbfsClient(); | ||
client.deleteBlobPath(p, null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is this delete on a file in implicit dir?
fs.create() will create explicit dir right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like discussed- using azcopy commands for creating implicit files now
Assertions.assertThat(!fs.exists(p1)) | ||
.describedAs("FileStatus of the deleted directory should not exist.") | ||
.isTrue(); | ||
Assertions.assertThat(!fs.exists(p1)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant assert
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
|
||
Path p = new Path("/nonExistingPath"); | ||
intercept(FileNotFoundException.class, | ||
() -> assertDeleted(fs, p, true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we simply call fs.delete() here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Taken
fs.delete(testFile, false); | ||
|
||
intercept(FileNotFoundException.class, | ||
() -> assertDeleted(fs, testFile, true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above.
@@ -359,6 +494,11 @@ public void testDeleteImplicitDir() throws Exception { | |||
AbfsBlobClient client = (AbfsBlobClient) fs.getAbfsClient(); | |||
client.deleteBlobPath(new Path("/testDir/dir1"), | |||
null, getTestTracingContext(fs, true)); | |||
|
|||
//Deleting non-empty dir with recursion set as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this needed?
Also the test name says deleting implicit dir but it seems like it is deleting explicit dir only.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is for the case with implicit path with children and recursion false
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
5f4494e
to
9fd8db0
Compare
🎊 +1 overall
This message was automatically generated. |
Description of PR
JIRA: https://issues.apache.org/jira/browse/HADOOP-19446
We have identified a few scenarios worth adding integration or mocked behavior tests for while implementing FNS Support over Blob Endpoint. This PR adds tests for the negative scenarios identified for the delete operation particularly.
Test results added in the comment below.