From 6b151a46535ad6f89f89c114ad907cec37afb964 Mon Sep 17 00:00:00 2001 From: Xinhao Yuan <xinhaoyuan@google.com> Date: Tue, 14 Jan 2025 08:57:20 -0800 Subject: [PATCH] Tolerate glob on non-existent path to return a NotFound error. Also add documentation for the API with tests. PiperOrigin-RevId: 715402197 --- common/BUILD | 1 + common/CMakeLists.txt | 2 ++ common/remote_file.h | 3 ++ common/remote_file_oss.cc | 4 ++- common/remote_file_test.cc | 65 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 74 insertions(+), 1 deletion(-) diff --git a/common/BUILD b/common/BUILD index ed3c69a9..8090a6e1 100644 --- a/common/BUILD +++ b/common/BUILD @@ -228,6 +228,7 @@ cc_library( ":remote_file", ":test_util", "@com_google_absl//absl/log:check", + "@com_google_absl//absl/status", "@com_google_absl//absl/time", "@com_google_googletest//:gtest", ], diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index b5a000a1..8279faee 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -176,5 +176,7 @@ fuzztest_cc_test( fuzztest::remote_file fuzztest::test_util absl::check + absl::status + absl::time GTest::gmock_main ) diff --git a/common/remote_file.h b/common/remote_file.h index b4ec099a..08c65d11 100644 --- a/common/remote_file.h +++ b/common/remote_file.h @@ -118,6 +118,9 @@ absl::StatusOr<int64_t> RemoteFileGetSize(std::string_view path); // Finds all files matching `glob` and appends them to `matches`. // +// Returns Ok when matches were found, or NotFound when no matches were found. +// Otherwise returns a non-NotFound error. +// // Note: The OSS implementation of this function fails on Windows, Android, and // Fuchsia. Instead of using this function, consider whether your use case can // be solved in a more specific way, e.g., by listing files in a directory and diff --git a/common/remote_file_oss.cc b/common/remote_file_oss.cc index a75736e6..405c195a 100644 --- a/common/remote_file_oss.cc +++ b/common/remote_file_oss.cc @@ -351,6 +351,7 @@ namespace { #if defined(FUZZTEST_HAS_OSS_GLOB) int HandleGlobError(const char *epath, int eerrno) { + if (eerrno == ENOENT) return 0; LOG(FATAL) << "Error while globbing path: " << VV(epath) << VV(eerrno); return -1; } @@ -379,7 +380,8 @@ absl::Status RemoteGlobMatch(std::string_view glob, ::globfree(&glob_ret); return absl::OkStatus(); #else - LOG(FATAL) << __func__ << "() is not supported on this platform."; + return absl::UnimplementedError( + absl::StrCat(__func__, "() is not supported on this platform")); #endif // defined(FUZZTEST_HAS_OSS_GLOB) } diff --git a/common/remote_file_test.cc b/common/remote_file_test.cc index e5300795..64100ff9 100644 --- a/common/remote_file_test.cc +++ b/common/remote_file_test.cc @@ -24,6 +24,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" #include "absl/log/check.h" +#include "absl/status/status.h" #include "absl/time/clock.h" #include "absl/time/time.h" #include "./common/logging.h" @@ -167,5 +168,69 @@ TEST(RemotePathTouchExistingFile, UpdatesTheLastModifiedTime) { EXPECT_GT(last_modified_time, start_time); } +TEST(RemoteGlobMatch, DoesNotRecurseIntoSubdirectories) { + const fs::path temp_dir = GetTestTempDir(test_info_->name()); + + const std::string file1_path = temp_dir / "file_01"; + CreateFileOrDie(file1_path); + const fs::path dir1_path = temp_dir / "dir_01"; + fs::create_directories(dir1_path); + const std::string file2_path = dir1_path / "file_02"; + CreateFileOrDie(file2_path); + + std::vector<std::string> files; + const absl::Status status = RemoteGlobMatch((temp_dir / "*").string(), files); + if (absl::IsUnimplemented(status)) { + GTEST_SKIP() + << "Skipping RemoteGlobMatch() tests since it is unimplemented: " + << status; + } + ASSERT_TRUE(status.ok()) << status; + EXPECT_THAT(files, UnorderedElementsAre(file1_path, dir1_path)); +} + +TEST(RemoteGlobMatch, ReturnsSinglePathWhenGlobIsExistingPath) { + const fs::path temp_dir = GetTestTempDir(test_info_->name()); + + const std::string file1_path = temp_dir / "file_01"; + CreateFileOrDie(file1_path); + + std::vector<std::string> files; + const absl::Status status = RemoteGlobMatch(temp_dir.string(), files); + if (absl::IsUnimplemented(status)) { + GTEST_SKIP() + << "Skipping RemoteGlobMatch() tests since it is unimplemented: " + << status; + } + ASSERT_TRUE(status.ok()) << status; + EXPECT_THAT(files, UnorderedElementsAre(temp_dir.string())); +} + +TEST(RemoteGlobMatch, ReturnsNotFoundErrorWithEmptyVectorWhenGlobMatchNothing) { + const fs::path temp_dir = GetTestTempDir(test_info_->name()); + std::vector<std::string> files; + const absl::Status status = RemoteGlobMatch((temp_dir / "*").string(), files); + if (absl::IsUnimplemented(status)) { + GTEST_SKIP() + << "Skipping RemoteGlobMatch() tests since it is unimplemented: " + << status; + } + ASSERT_TRUE(absl::IsNotFound(status)) << status; + EXPECT_THAT(files, IsEmpty()); +} + +TEST(RemoteGlobMatch, ReturnsNotFoundErrorWithEmptyVectorWhenPathDoesNotExist) { + std::vector<std::string> files; + const absl::Status status = + RemoteGlobMatch("/this/file/path/does/not/exist", files); + if (absl::IsUnimplemented(status)) { + GTEST_SKIP() + << "Skipping RemoteGlobMatch() tests since it is unimplemented: " + << status; + } + ASSERT_TRUE(absl::IsNotFound(status)) << status; + EXPECT_THAT(files, IsEmpty()); +} + } // namespace } // namespace centipede