HyunWooZZ commented on code in PR #46416:
URL: https://github.com/apache/arrow/pull/46416#discussion_r2093133900


##########
cpp/src/arrow/filesystem/gcsfs_test.cc:
##########
@@ -629,6 +629,29 @@ TEST_F(GcsIntegrationTest, GetFileInfoBucket) {
   ASSERT_RAISES(Invalid, fs->GetFileInfo("gs://" + PreexistingBucketName()));
 }
 
+TEST_F(GcsIntegrationTest, GetFileInfo) {
+  ASSERT_OK_AND_ASSIGN(auto fs, GcsFileSystem::Make(TestGcsOptions()));
+  constexpr auto kTextFileName = "dir/foo/bar.txt";
+  ASSERT_OK_AND_ASSIGN(
+      auto output,
+      fs->OpenOutputStream(PreexistingBucketPath() + kTextFileName, 
/*metadata=*/{}));
+  const auto data = std::string(kLoremIpsum);
+  ASSERT_OK(output->Write(data.data(), data.size()));
+  ASSERT_OK(output->Close());
+
+  // check this is the File.
+  AssertFileInfo(fs.get(), PreexistingBucketPath() + kTextFileName, 
FileType::File);
+
+  // check parent directories are recognized as directories.
+  AssertFileInfo(fs.get(), PreexistingBucketPath() + "dir/", 
FileType::Directory);
+
+  // check not exists.
+  AssertFileInfo(fs.get(), PreexistingBucketPath() + "dir/foo/unexpected_dir/",
+                 FileType::NotFound);
+  AssertFileInfo(fs.get(), PreexistingBucketPath() + "dir/foo/not_bar.txt",
+                 FileType::NotFound);
+}

Review Comment:
   Ok :) I tried to create a mock object MockListObjectsReader, but it was 
quite difficult for me.
   Sorry about that.
   
   > Could you add a comment explaining why we don't have a test for the 
implementation instead of adding a test?
   
   I will add the comment.
   
   To recap our conversation:
   1. I will add a comment in the code explaining why we didn’t test an invalid 
project ID using the storage testbench.
   - The testbench does not enforce any permission checks (ACL/IAM):
   
https://github.com/googleapis/storage-testbench?tab=readme-ov-file#what-is-this-testbench
   
   2. I’ll keep the following test case:  
      - Edit: I think we should skip this test in CI/CD environments.  
        I added code to skip it by default.
   
   ```cpp
   TEST_F(GcsIntegrationTest, GetFileInfoWithoutPermission) {
     if (!std::getenv("ARROW_RUN_REAL_GCS_TESTS")) {
       GTEST_SKIP() << "Skipping test that requires real GCS access. "
                    << "Set ARROW_RUN_REAL_GCS_TESTS=1 to enable.";
     }
     auto options = GcsOptions::Anonymous();
     options.retry_limit_seconds = 15;
     options.project_id = "test-only-invalid-project-id";
   
     ASSERT_OK_AND_ASSIGN(auto fs, GcsFileSystem::Make(options));
   
     AssertFileInfo(fs.get(), PreexistingBucketPath() + "dir/foo/", 
FileType::NotFound);
     AssertFileInfo(fs.get(), PreexistingBucketPath() + "dir/foo/bar.txt",
                    FileType::NotFound);
   }
   ```
   Thank you for your response even night :)
   I hope you have a good weekend!!!
   
   



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to