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