av8or1 commented on code in PR #39840:
URL: https://github.com/apache/arrow/pull/39840#discussion_r1473128443
##########
cpp/src/arrow/filesystem/azurefs_test.cc:
##########
@@ -1382,6 +1386,32 @@ TEST_F(TestAzuriteFileSystem,
DeleteDirContentsFailureNonexistent) {
this->TestDeleteDirContentsFailureNonexistent();
}
+TEST_F(AzuriteFileSystemTest, DeleteFileSuccessHaveFile) {
+void CreateFile(FileSystem* fs, const std::string& path, const std::string&
data) {
+ const auto file_name = PreexistingData::RandomFileName() +
+ PreexistingData::RandomFileNameExtension();
+ ASSERT_OK(CreateFile(fs_.get(), file_name, "abc"));
+ arrow::fs::AssertFileInfo(fs_.get(), file_name, FileType::File);
+ ASSERT_OK(fs_->DeleteFile(file_name));
+ arrow::fs::AssertFileInfo(fs_.get(), file_name, FileType::NotFound);
+}
Review Comment:
Not certain that I understand the question. The idea behind this test was
to create a file to delete, then delete it. Thus the pseudo-code was:
1) Create a filename
2) Create the file and verify that the file creation was successful
3) Verify that the file type was a file (could be removed if considered
redundant)
4) Delete the file and verify that the delete attempt did not assert
5) Verify that the file is not found (because it has been deleted)
As for the functions RandomFileName() and RandomFileNameExtension(), I
created those to clean the code a bit and placed them with the RandomChars()
method. This is just a choice, to abstract the implementation details into a
function. It doesn't have to be that way. The content of those two methods
could be placed inline at lines 1391 and 1392 and these two methods deleted if
that is the preferred manner of going about it. Open to
suggestion/guidance/feedback.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]