From 017797b7184fa6c3af5a7ddc9063879ce1a90d36 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Thu, 1 Dec 2022 17:56:36 +0000 Subject: [PATCH 01/19] Add fstat to OsSysCalls Signed-off-by: Raven Black --- envoy/api/os_sys_calls.h | 5 +++ source/common/api/posix/os_sys_calls_impl.cc | 5 +++ source/common/api/posix/os_sys_calls_impl.h | 1 + source/common/api/win32/os_sys_calls_impl.cc | 5 +++ source/common/api/win32/os_sys_calls_impl.h | 1 + test/common/api/os_sys_calls_test.cc | 35 ++++++++++++++++++++ test/mocks/api/mocks.h | 1 + 7 files changed, 53 insertions(+) diff --git a/envoy/api/os_sys_calls.h b/envoy/api/os_sys_calls.h index 28a591307443b..6a26a19edc70a 100644 --- a/envoy/api/os_sys_calls.h +++ b/envoy/api/os_sys_calls.h @@ -149,6 +149,11 @@ class OsSysCalls { */ virtual SysCallIntResult stat(const char* pathname, struct stat* buf) PURE; + /** + * @see man 2 fstat + */ + virtual SysCallIntResult fstat(os_fd_t fd, struct stat* buf) PURE; + /** * @see man 2 setsockopt */ diff --git a/source/common/api/posix/os_sys_calls_impl.cc b/source/common/api/posix/os_sys_calls_impl.cc index 3afac06a4c685..a142f949a61e7 100644 --- a/source/common/api/posix/os_sys_calls_impl.cc +++ b/source/common/api/posix/os_sys_calls_impl.cc @@ -194,6 +194,11 @@ SysCallIntResult OsSysCallsImpl::stat(const char* pathname, struct stat* buf) { return {rc, rc != -1 ? 0 : errno}; } +SysCallIntResult OsSysCallsImpl::fstat(os_fd_t fd, struct stat* buf) { + const int rc = ::fstat(fd, buf); + return {rc, rc != -1 ? 0 : errno}; +} + SysCallIntResult OsSysCallsImpl::setsockopt(os_fd_t sockfd, int level, int optname, const void* optval, socklen_t optlen) { const int rc = ::setsockopt(sockfd, level, optname, optval, optlen); diff --git a/source/common/api/posix/os_sys_calls_impl.h b/source/common/api/posix/os_sys_calls_impl.h index 61a79c8aab063..0c4491eab7996 100644 --- a/source/common/api/posix/os_sys_calls_impl.h +++ b/source/common/api/posix/os_sys_calls_impl.h @@ -35,6 +35,7 @@ class OsSysCallsImpl : public OsSysCalls { SysCallPtrResult mmap(void* addr, size_t length, int prot, int flags, int fd, off_t offset) override; SysCallIntResult stat(const char* pathname, struct stat* buf) override; + SysCallIntResult fstat(os_fd_t fd, struct stat* buf) override; SysCallIntResult setsockopt(os_fd_t sockfd, int level, int optname, const void* optval, socklen_t optlen) override; SysCallIntResult getsockopt(os_fd_t sockfd, int level, int optname, void* optval, diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc index 1a630b5d9c6b6..27a61d65f3eb5 100644 --- a/source/common/api/win32/os_sys_calls_impl.cc +++ b/source/common/api/win32/os_sys_calls_impl.cc @@ -226,6 +226,11 @@ SysCallIntResult OsSysCallsImpl::stat(const char* pathname, struct stat* buf) { return {rc, rc != -1 ? 0 : errno}; } +SysCallIntResult OsSysCallsImpl::fstat(os_fd_t fd, struct stat* buf) { + const int rc = ::fstat(fd, buf); + return {rc, rc != -1 ? 0 : errno}; +} + SysCallIntResult OsSysCallsImpl::setsockopt(os_fd_t sockfd, int level, int optname, const void* optval, socklen_t optlen) { const int rc = ::setsockopt(sockfd, level, optname, static_cast(optval), optlen); diff --git a/source/common/api/win32/os_sys_calls_impl.h b/source/common/api/win32/os_sys_calls_impl.h index 44bc4993709f8..2afa41dd2ce3b 100644 --- a/source/common/api/win32/os_sys_calls_impl.h +++ b/source/common/api/win32/os_sys_calls_impl.h @@ -36,6 +36,7 @@ class OsSysCallsImpl : public OsSysCalls { SysCallPtrResult mmap(void* addr, size_t length, int prot, int flags, int fd, off_t offset) override; SysCallIntResult stat(const char* pathname, struct stat* buf) override; + SysCallIntResult fstat(os_fd_t fd, struct stat* buf) override; SysCallIntResult setsockopt(os_fd_t sockfd, int level, int optname, const void* optval, socklen_t optlen) override; SysCallIntResult getsockopt(os_fd_t sockfd, int level, int optname, void* optval, diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index 5748b257ac791..8a14079c07a8f 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -1,9 +1,44 @@ #include "source/common/api/os_sys_calls_impl.h" +#include "test/test_common/environment.h" + #include "gtest/gtest.h" namespace Envoy { +TEST(OsSyscallsTest, OpenPwriteFstatCloseStatUnlink) { + auto& os_syscalls = Api::OsSysCallsSingleton::get(); + std::string path{TestEnvironment::temporaryPath("envoy_test")}; + TestEnvironment::createPath(path); + std::string file_path = path + "/file"; + absl::string_view file_contents = "12345"; + Api::SysCallIntResult open_result = os_syscalls.open(file_path.c_str(), O_CREAT | O_RDWR); + EXPECT_NE(open_result.return_value_, -1); + EXPECT_EQ(open_result.errno_, 0); + os_fd_t fd = open_result.return_value_; + Api::SysCallSizeResult write_result = + os_syscalls.pwrite(fd, file_contents.begin(), file_contents.size(), 0); + EXPECT_EQ(write_result.return_value_, file_contents.size()); + EXPECT_EQ(write_result.errno_, 0); + struct stat fstat_value; + Api::SysCallIntResult fstat_result = os_syscalls.fstat(fd, &fstat_value); + EXPECT_EQ(fstat_result.return_value_, 0); + EXPECT_EQ(fstat_result.errno_, 0); + EXPECT_EQ(fstat_value.st_size, file_contents.size()); + Api::SysCallIntResult close_result = os_syscalls.close(fd); + EXPECT_EQ(close_result.return_value_, 0); + EXPECT_EQ(close_result.errno_, 0); + struct stat stat_value; + Api::SysCallIntResult stat_result = os_syscalls.stat(file_path.c_str(), &stat_value); + EXPECT_EQ(stat_result.return_value_, 0); + EXPECT_EQ(stat_result.errno_, 0); + EXPECT_EQ(stat_value.st_size, file_contents.size()); + Api::SysCallIntResult unlink_result = os_syscalls.unlink(file_path.c_str()); + EXPECT_EQ(unlink_result.return_value_, 0); + EXPECT_EQ(unlink_result.errno_, 0); + TestEnvironment::removePath(path); +} + TEST(OsSyscallsTest, SetAlternateGetifaddrs) { auto& os_syscalls = Api::OsSysCallsSingleton::get(); const bool pre_alternate_support = os_syscalls.supportsGetifaddrs(); diff --git a/test/mocks/api/mocks.h b/test/mocks/api/mocks.h index 7d136a0784237..95f8bf8a8a179 100644 --- a/test/mocks/api/mocks.h +++ b/test/mocks/api/mocks.h @@ -94,6 +94,7 @@ class MockOsSysCalls : public OsSysCallsImpl { MOCK_METHOD(SysCallPtrResult, mmap, (void* addr, size_t length, int prot, int flags, int fd, off_t offset)); MOCK_METHOD(SysCallIntResult, stat, (const char* name, struct stat* stat)); + MOCK_METHOD(SysCallIntResult, fstat, (os_fd_t fd, struct stat* stat)); MOCK_METHOD(SysCallIntResult, chmod, (const std::string& name, mode_t mode)); MOCK_METHOD(int, setsockopt_, (os_fd_t sockfd, int level, int optname, const void* optval, socklen_t optlen)); From 9e8d1b4228931515204b3ecc1bb6196220c14d34 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Thu, 1 Dec 2022 19:14:23 +0000 Subject: [PATCH 02/19] Add test comments for readability Signed-off-by: Raven Black --- test/common/api/os_sys_calls_test.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index 8a14079c07a8f..5b375ddac4338 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -6,33 +6,40 @@ namespace Envoy { +// Test happy path for open, pwrite, fstat, close, stat and unlink. TEST(OsSyscallsTest, OpenPwriteFstatCloseStatUnlink) { auto& os_syscalls = Api::OsSysCallsSingleton::get(); std::string path{TestEnvironment::temporaryPath("envoy_test")}; TestEnvironment::createPath(path); std::string file_path = path + "/file"; absl::string_view file_contents = "12345"; + // Test open Api::SysCallIntResult open_result = os_syscalls.open(file_path.c_str(), O_CREAT | O_RDWR); EXPECT_NE(open_result.return_value_, -1); EXPECT_EQ(open_result.errno_, 0); os_fd_t fd = open_result.return_value_; + // Test write Api::SysCallSizeResult write_result = os_syscalls.pwrite(fd, file_contents.begin(), file_contents.size(), 0); EXPECT_EQ(write_result.return_value_, file_contents.size()); EXPECT_EQ(write_result.errno_, 0); + // Test fstat struct stat fstat_value; Api::SysCallIntResult fstat_result = os_syscalls.fstat(fd, &fstat_value); EXPECT_EQ(fstat_result.return_value_, 0); EXPECT_EQ(fstat_result.errno_, 0); EXPECT_EQ(fstat_value.st_size, file_contents.size()); + // Test close Api::SysCallIntResult close_result = os_syscalls.close(fd); EXPECT_EQ(close_result.return_value_, 0); EXPECT_EQ(close_result.errno_, 0); + // Test stat struct stat stat_value; Api::SysCallIntResult stat_result = os_syscalls.stat(file_path.c_str(), &stat_value); EXPECT_EQ(stat_result.return_value_, 0); EXPECT_EQ(stat_result.errno_, 0); EXPECT_EQ(stat_value.st_size, file_contents.size()); + // Test unlink Api::SysCallIntResult unlink_result = os_syscalls.unlink(file_path.c_str()); EXPECT_EQ(unlink_result.return_value_, 0); EXPECT_EQ(unlink_result.errno_, 0); From ec3358fceb6e79d80a3be5407e78b1f749c9a45a Mon Sep 17 00:00:00 2001 From: Raven Black Date: Thu, 1 Dec 2022 21:53:27 +0000 Subject: [PATCH 03/19] Satisfy the spelling demon Signed-off-by: Raven Black --- test/common/api/os_sys_calls_test.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index 5b375ddac4338..c1725f03a1b24 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -6,40 +6,40 @@ namespace Envoy { -// Test happy path for open, pwrite, fstat, close, stat and unlink. +// Test happy path for `open`, `pwrite`, `fstat`, `close`, `stat` and `unlink`. TEST(OsSyscallsTest, OpenPwriteFstatCloseStatUnlink) { auto& os_syscalls = Api::OsSysCallsSingleton::get(); std::string path{TestEnvironment::temporaryPath("envoy_test")}; TestEnvironment::createPath(path); std::string file_path = path + "/file"; absl::string_view file_contents = "12345"; - // Test open + // Test `open` Api::SysCallIntResult open_result = os_syscalls.open(file_path.c_str(), O_CREAT | O_RDWR); EXPECT_NE(open_result.return_value_, -1); EXPECT_EQ(open_result.errno_, 0); os_fd_t fd = open_result.return_value_; - // Test write + // Test `write` Api::SysCallSizeResult write_result = os_syscalls.pwrite(fd, file_contents.begin(), file_contents.size(), 0); EXPECT_EQ(write_result.return_value_, file_contents.size()); EXPECT_EQ(write_result.errno_, 0); - // Test fstat + // Test `fstat` struct stat fstat_value; Api::SysCallIntResult fstat_result = os_syscalls.fstat(fd, &fstat_value); EXPECT_EQ(fstat_result.return_value_, 0); EXPECT_EQ(fstat_result.errno_, 0); EXPECT_EQ(fstat_value.st_size, file_contents.size()); - // Test close + // Test `close` Api::SysCallIntResult close_result = os_syscalls.close(fd); EXPECT_EQ(close_result.return_value_, 0); EXPECT_EQ(close_result.errno_, 0); - // Test stat + // Test `stat` struct stat stat_value; Api::SysCallIntResult stat_result = os_syscalls.stat(file_path.c_str(), &stat_value); EXPECT_EQ(stat_result.return_value_, 0); EXPECT_EQ(stat_result.errno_, 0); EXPECT_EQ(stat_value.st_size, file_contents.size()); - // Test unlink + // Test `unlink` Api::SysCallIntResult unlink_result = os_syscalls.unlink(file_path.c_str()); EXPECT_EQ(unlink_result.return_value_, 0); EXPECT_EQ(unlink_result.errno_, 0); From 9bd5d0aa9d8ddaec5a3e37df6232b4e7e47e39a3 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Fri, 2 Dec 2022 16:12:20 +0000 Subject: [PATCH 04/19] Include fcntl.h for O_CREATE etc on windows Signed-off-by: Raven Black --- test/common/api/os_sys_calls_test.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index c1725f03a1b24..475cb134ca78e 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -1,3 +1,5 @@ +#include + #include "source/common/api/os_sys_calls_impl.h" #include "test/test_common/environment.h" From 13d21378f65fe11c4dcefc352be3669cf9e8e943 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Fri, 2 Dec 2022 21:02:19 +0000 Subject: [PATCH 05/19] Implement for windows, and test-cover pread and pwrite Signed-off-by: Raven Black --- source/common/api/win32/os_sys_calls_impl.cc | 22 ++++++++++++++++++-- test/common/api/os_sys_calls_test.cc | 13 +++++++++--- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc index 27a61d65f3eb5..2eabc5114cc1b 100644 --- a/source/common/api/win32/os_sys_calls_impl.cc +++ b/source/common/api/win32/os_sys_calls_impl.cc @@ -144,12 +144,30 @@ SysCallSizeResult OsSysCallsImpl::readv(os_fd_t fd, const iovec* iov, int num_io SysCallSizeResult OsSysCallsImpl::pwrite(os_fd_t fd, const void* buffer, size_t length, off_t offset) const { - PANIC("not implemented"); + OVERLAPPED overlapped; + overlapped.Offset = offset; + overlapped.OffsetHigh = offset & 0xffffffff00000000; + overlapped.hEvent = 0; + DWORD length_written; + bool success = ::WriteFile(fd, buffer, length, &length_written, overlapped); + if (!success) { + return {-1, ::GetLastError()}; + } + return {length_written, 0}; } SysCallSizeResult OsSysCallsImpl::pread(os_fd_t fd, void* buffer, size_t length, off_t offset) const { - PANIC("not implemented"); + OVERLAPPED overlapped; + overlapped.Offset = offset; + overlapped.OffsetHigh = offset & 0xffffffff00000000; + overlapped.hEvent = 0; + DWORD length_read; + bool success = ::ReadFile(fd, buffer, length, &length_read, overlapped); + if (!success) { + return {-1, ::GetLastError()}; + } + return {length_read, 0}; } SysCallSizeResult OsSysCallsImpl::recv(os_fd_t socket, void* buffer, size_t length, int flags) { diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index 475cb134ca78e..61e4663d838f3 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -8,8 +8,8 @@ namespace Envoy { -// Test happy path for `open`, `pwrite`, `fstat`, `close`, `stat` and `unlink`. -TEST(OsSyscallsTest, OpenPwriteFstatCloseStatUnlink) { +// Test happy path for `open`, `pwrite`, `pread`, `fstat`, `close`, `stat` and `unlink`. +TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) { auto& os_syscalls = Api::OsSysCallsSingleton::get(); std::string path{TestEnvironment::temporaryPath("envoy_test")}; TestEnvironment::createPath(path); @@ -20,11 +20,18 @@ TEST(OsSyscallsTest, OpenPwriteFstatCloseStatUnlink) { EXPECT_NE(open_result.return_value_, -1); EXPECT_EQ(open_result.errno_, 0); os_fd_t fd = open_result.return_value_; - // Test `write` + // Test `pwrite` Api::SysCallSizeResult write_result = os_syscalls.pwrite(fd, file_contents.begin(), file_contents.size(), 0); EXPECT_EQ(write_result.return_value_, file_contents.size()); EXPECT_EQ(write_result.errno_, 0); + // Test `pread` + char read_buffer[5]; + Api::SysCallSizeResult read_result = os_syscalls.pread(fd, read_buffer, sizeof(read_buffer), 0); + EXPECT_EQ(read_result.return_value_, sizeof(read_buffer)); + EXPECT_EQ(read_result.errno_, 0); + absl::string_view read_buffer_view{read_buffer, sizeof(read_buffer)}; + EXPECT_EQ(file_contents, read_buffer_view); // Test `fstat` struct stat fstat_value; Api::SysCallIntResult fstat_result = os_syscalls.fstat(fd, &fstat_value); From 660c8db9d3dec8d1f1158ac446d09aaa6946d85e Mon Sep 17 00:00:00 2001 From: Raven Black Date: Fri, 2 Dec 2022 22:33:08 +0000 Subject: [PATCH 06/19] no pread and pwrite for Windows after all; make the test not use them Signed-off-by: Raven Black --- source/common/api/win32/os_sys_calls_impl.cc | 22 ++------------------ test/common/api/os_sys_calls_test.cc | 5 +++++ 2 files changed, 7 insertions(+), 20 deletions(-) diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc index 2eabc5114cc1b..27a61d65f3eb5 100644 --- a/source/common/api/win32/os_sys_calls_impl.cc +++ b/source/common/api/win32/os_sys_calls_impl.cc @@ -144,30 +144,12 @@ SysCallSizeResult OsSysCallsImpl::readv(os_fd_t fd, const iovec* iov, int num_io SysCallSizeResult OsSysCallsImpl::pwrite(os_fd_t fd, const void* buffer, size_t length, off_t offset) const { - OVERLAPPED overlapped; - overlapped.Offset = offset; - overlapped.OffsetHigh = offset & 0xffffffff00000000; - overlapped.hEvent = 0; - DWORD length_written; - bool success = ::WriteFile(fd, buffer, length, &length_written, overlapped); - if (!success) { - return {-1, ::GetLastError()}; - } - return {length_written, 0}; + PANIC("not implemented"); } SysCallSizeResult OsSysCallsImpl::pread(os_fd_t fd, void* buffer, size_t length, off_t offset) const { - OVERLAPPED overlapped; - overlapped.Offset = offset; - overlapped.OffsetHigh = offset & 0xffffffff00000000; - overlapped.hEvent = 0; - DWORD length_read; - bool success = ::ReadFile(fd, buffer, length, &length_read, overlapped); - if (!success) { - return {-1, ::GetLastError()}; - } - return {length_read, 0}; + PANIC("not implemented"); } SysCallSizeResult OsSysCallsImpl::recv(os_fd_t socket, void* buffer, size_t length, int flags) { diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index 61e4663d838f3..1c004eba57312 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -20,6 +20,10 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) { EXPECT_NE(open_result.return_value_, -1); EXPECT_EQ(open_result.errno_, 0); os_fd_t fd = open_result.return_value_; +#ifdef WIN32 + // pwrite and pread are not supported. + ::_write(fd, file_contents.begin(), 5); +#else // Test `pwrite` Api::SysCallSizeResult write_result = os_syscalls.pwrite(fd, file_contents.begin(), file_contents.size(), 0); @@ -32,6 +36,7 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) { EXPECT_EQ(read_result.errno_, 0); absl::string_view read_buffer_view{read_buffer, sizeof(read_buffer)}; EXPECT_EQ(file_contents, read_buffer_view); +#endif // Test `fstat` struct stat fstat_value; Api::SysCallIntResult fstat_result = os_syscalls.fstat(fd, &fstat_value); From 46afa094bb21727cd9ab62546684d96249770889 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Fri, 2 Dec 2022 22:38:28 +0000 Subject: [PATCH 07/19] Catch failure of write in the windows test path Signed-off-by: Raven Black --- test/common/api/os_sys_calls_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index 1c004eba57312..6824255a3ba6a 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -21,8 +21,8 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) { EXPECT_EQ(open_result.errno_, 0); os_fd_t fd = open_result.return_value_; #ifdef WIN32 - // pwrite and pread are not supported. - ::_write(fd, file_contents.begin(), 5); + // pwrite and pread are not supported. Just write some bytes so we can still test stat. + EXPECT_EQ(file_contents.size(), ::_write(fd, file_contents.begin(), file_contents.size())); #else // Test `pwrite` Api::SysCallSizeResult write_result = From 64a6ab84906f1a73e1b96605278cf7643d2303eb Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 5 Dec 2022 15:11:01 +0000 Subject: [PATCH 08/19] spell function names with backticks Signed-off-by: Raven Black --- test/common/api/os_sys_calls_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index 6824255a3ba6a..152d92f00e5a0 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -21,7 +21,7 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) { EXPECT_EQ(open_result.errno_, 0); os_fd_t fd = open_result.return_value_; #ifdef WIN32 - // pwrite and pread are not supported. Just write some bytes so we can still test stat. + // `pwrite` and `pread` are not supported. Just write some bytes so we can still test stat. EXPECT_EQ(file_contents.size(), ::_write(fd, file_contents.begin(), file_contents.size())); #else // Test `pwrite` From 4ece43014888dc33a14e4438c97773e904fbb5a7 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 5 Dec 2022 16:31:33 +0000 Subject: [PATCH 09/19] Explain fcntl.h, and make it only included on Win32 Signed-off-by: Raven Black --- test/common/api/os_sys_calls_test.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index 152d92f00e5a0..94cb2e85e17e9 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -1,4 +1,6 @@ -#include +#ifdef WIN32 +#include // Needed for `O_CREAT` and `O_RDWR` on Win32. +#endif #include "source/common/api/os_sys_calls_impl.h" From 545841522d1fedea4617cd284b27d48a7898aaf9 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 5 Dec 2022 16:34:35 +0000 Subject: [PATCH 10/19] check_format insists on adding this newline Signed-off-by: Raven Black --- test/common/api/os_sys_calls_test.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index 94cb2e85e17e9..9d571421b9ae1 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -1,5 +1,6 @@ #ifdef WIN32 #include // Needed for `O_CREAT` and `O_RDWR` on Win32. + #endif #include "source/common/api/os_sys_calls_impl.h" From 6dfddbb6ac5524e3aa09fef6aa43d22b855cddc3 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 5 Dec 2022 18:22:24 +0000 Subject: [PATCH 11/19] Make Win32 OsSysCalls::close work on file descriptors, not just sockets Signed-off-by: Raven Black --- source/common/api/win32/os_sys_calls_impl.cc | 8 +++++++- test/common/api/os_sys_calls_test.cc | 1 + 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc index 27a61d65f3eb5..6e44267106cf0 100644 --- a/source/common/api/win32/os_sys_calls_impl.cc +++ b/source/common/api/win32/os_sys_calls_impl.cc @@ -115,7 +115,13 @@ SysCallIntResult OsSysCallsImpl::ioctl(os_fd_t sockfd, unsigned long control_cod SysCallIntResult OsSysCallsImpl::close(os_fd_t fd) { const int rc = ::closesocket(fd); - return {rc, rc != -1 ? 0 : ::WSAGetLastError()}; + int socket_err = ::WSAGetLastError(); + if (rc == -1) { + // If closesocket failed, maybe the descriptor was a file. + rc = ::close(fd); + } + // Assume the descriptor was a socket, and return the error from that one, if both failed. + return {rc, rc != -1 ? 0 : socket_err}; } SysCallSizeResult OsSysCallsImpl::writev(os_fd_t fd, const iovec* iov, int num_iov) { diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index 9d571421b9ae1..d15c9ebcac51d 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -46,6 +46,7 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) { EXPECT_EQ(fstat_result.return_value_, 0); EXPECT_EQ(fstat_result.errno_, 0); EXPECT_EQ(fstat_value.st_size, file_contents.size()); +#ifndef WIN32 // Test `close` Api::SysCallIntResult close_result = os_syscalls.close(fd); EXPECT_EQ(close_result.return_value_, 0); From 8a8b5c96c089ae6948fab122b483fb6c48714d85 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 5 Dec 2022 19:59:52 +0000 Subject: [PATCH 12/19] Backtick the spelling demon again Signed-off-by: Raven Black --- source/common/api/win32/os_sys_calls_impl.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc index 6e44267106cf0..df80b87d61fd6 100644 --- a/source/common/api/win32/os_sys_calls_impl.cc +++ b/source/common/api/win32/os_sys_calls_impl.cc @@ -117,7 +117,7 @@ SysCallIntResult OsSysCallsImpl::close(os_fd_t fd) { const int rc = ::closesocket(fd); int socket_err = ::WSAGetLastError(); if (rc == -1) { - // If closesocket failed, maybe the descriptor was a file. + // If `closesocket` failed, maybe the descriptor was a file. rc = ::close(fd); } // Assume the descriptor was a socket, and return the error from that one, if both failed. From 37d6ca02fdfdea4dda6f031c6c9a67407ede585b Mon Sep 17 00:00:00 2001 From: Raven Black Date: Tue, 6 Dec 2022 15:15:53 +0000 Subject: [PATCH 13/19] Remove accidental ifndef Signed-off-by: Raven Black --- test/common/api/os_sys_calls_test.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index d15c9ebcac51d..9d571421b9ae1 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -46,7 +46,6 @@ TEST(OsSyscallsTest, OpenPwritePreadFstatCloseStatUnlink) { EXPECT_EQ(fstat_result.return_value_, 0); EXPECT_EQ(fstat_result.errno_, 0); EXPECT_EQ(fstat_value.st_size, file_contents.size()); -#ifndef WIN32 // Test `close` Api::SysCallIntResult close_result = os_syscalls.close(fd); EXPECT_EQ(close_result.return_value_, 0); From bdbf2ea2edceb65e0542094e7892c64e695e73cd Mon Sep 17 00:00:00 2001 From: Raven Black Date: Tue, 6 Dec 2022 21:18:14 +0000 Subject: [PATCH 14/19] Return code no longer const Signed-off-by: Raven Black --- source/common/api/win32/os_sys_calls_impl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc index df80b87d61fd6..69502f12cff28 100644 --- a/source/common/api/win32/os_sys_calls_impl.cc +++ b/source/common/api/win32/os_sys_calls_impl.cc @@ -114,8 +114,8 @@ SysCallIntResult OsSysCallsImpl::ioctl(os_fd_t sockfd, unsigned long control_cod } SysCallIntResult OsSysCallsImpl::close(os_fd_t fd) { - const int rc = ::closesocket(fd); - int socket_err = ::WSAGetLastError(); + int rc = ::closesocket(fd); + const int socket_err = ::WSAGetLastError(); if (rc == -1) { // If `closesocket` failed, maybe the descriptor was a file. rc = ::close(fd); From 46fc40b9b1d25d43abdf0d44af9b287956c535af Mon Sep 17 00:00:00 2001 From: Raven Black Date: Wed, 7 Dec 2022 15:30:50 +0000 Subject: [PATCH 15/19] Add stat for AsyncFileHandle Signed-off-by: Raven Black --- .../async_file_context_thread_pool.cc | 21 +++++++++++ .../async_file_context_thread_pool.h | 2 ++ .../common/async_files/async_file_handle.h | 4 +++ .../async_file_handle_thread_pool_test.cc | 36 +++++++++++++++++-- test/extensions/common/async_files/mocks.cc | 5 +++ test/extensions/common/async_files/mocks.h | 2 ++ test/test_common/status_utility.h | 9 ++--- 7 files changed, 70 insertions(+), 9 deletions(-) diff --git a/source/extensions/common/async_files/async_file_context_thread_pool.cc b/source/extensions/common/async_files/async_file_context_thread_pool.cc index b25e06ca7d0e8..43a07ef051519 100644 --- a/source/extensions/common/async_files/async_file_context_thread_pool.cc +++ b/source/extensions/common/async_files/async_file_context_thread_pool.cc @@ -37,6 +37,22 @@ template class AsyncFileActionThreadPool : public AsyncFileActionWi AsyncFileHandle handle_; }; +class ActionStat : public AsyncFileActionThreadPool> { +public: + ActionStat(AsyncFileHandle handle, std::function)> on_complete) + : AsyncFileActionThreadPool>(handle, on_complete) {} + + absl::StatusOr executeImpl() override { + ASSERT(fileDescriptor() != -1); + struct stat stat_result; + auto result = posix().fstat(fileDescriptor(), &stat_result); + if (result.return_value_ != 0) { + return statusAfterFileError(result); + } + return stat_result; + } +}; + class ActionCreateHardLink : public AsyncFileActionThreadPool { public: ActionCreateHardLink(AsyncFileHandle handle, absl::string_view filename, @@ -171,6 +187,11 @@ class ActionDuplicateFile : public AsyncFileActionThreadPool +AsyncFileContextThreadPool::stat(std::function)> on_complete) { + return checkFileAndEnqueue(std::make_shared(handle(), std::move(on_complete))); +} + absl::StatusOr AsyncFileContextThreadPool::createHardLink(absl::string_view filename, std::function on_complete) { diff --git a/source/extensions/common/async_files/async_file_context_thread_pool.h b/source/extensions/common/async_files/async_file_context_thread_pool.h index 79bdb876f1c28..8c14d75be8212 100644 --- a/source/extensions/common/async_files/async_file_context_thread_pool.h +++ b/source/extensions/common/async_files/async_file_context_thread_pool.h @@ -21,6 +21,8 @@ class AsyncFileContextThreadPool final : public AsyncFileContextBase { public: explicit AsyncFileContextThreadPool(AsyncFileManager& manager, int fd); + absl::StatusOr + stat(std::function)> on_complete) override; absl::StatusOr createHardLink(absl::string_view filename, std::function on_complete) override; diff --git a/source/extensions/common/async_files/async_file_handle.h b/source/extensions/common/async_files/async_file_handle.h index 3d6534733b4e8..a8e54a4166060 100644 --- a/source/extensions/common/async_files/async_file_handle.h +++ b/source/extensions/common/async_files/async_file_handle.h @@ -17,6 +17,10 @@ namespace AsyncFiles { // Instantiated from an AsyncFileManager. class AsyncFileContext : public std::enable_shared_from_this { public: + // Gets a stat struct for the file. + virtual absl::StatusOr + stat(std::function)> on_complete) PURE; + // Action to hard link the file that is currently open. Typically for use in tandem with // createAnonymousFile to turn that file into a named file after finishing writing its contents. // diff --git a/test/extensions/common/async_files/async_file_handle_thread_pool_test.cc b/test/extensions/common/async_files/async_file_handle_thread_pool_test.cc index 8707ce92afb45..538462fdfa148 100644 --- a/test/extensions/common/async_files/async_file_handle_thread_pool_test.cc +++ b/test/extensions/common/async_files/async_file_handle_thread_pool_test.cc @@ -26,6 +26,7 @@ namespace Common { namespace AsyncFiles { using StatusHelpers::IsOkAndHolds; +using StatusHelpers::StatusIs; using ::testing::_; using ::testing::Eq; using ::testing::Return; @@ -206,7 +207,7 @@ class TestTmpFile { posix.close(fd_); posix.unlink(template_); } - std::string name() { return std::string(template_); } + std::string name() { return {template_}; } private: int fd_; @@ -420,6 +421,36 @@ TEST_F(AsyncFileHandleWithMockPosixTest, CancellingFailedCreateHardLinkInProgres close(handle); } +TEST_F(AsyncFileHandleWithMockPosixTest, StatSuccessReturnsPopulatedStatStruct) { + auto handle = createAnonymousFile(); + struct stat expected_stat = {}; + expected_stat.st_size = 9876; + EXPECT_CALL(mock_posix_file_operations_, fstat(_, _)).WillOnce([&](int, struct stat* buffer) { + *buffer = expected_stat; + return Api::SysCallIntResult{0, 0}; + }); + std::promise> fstat_status_promise; + EXPECT_OK(handle->stat([&](absl::StatusOr status) { + fstat_status_promise.set_value(std::move(status)); + })); + auto fstat_status = fstat_status_promise.get_future().get(); + EXPECT_THAT(fstat_status, IsOkAndHolds(testing::Field(&stat::st_size, expected_stat.st_size))); + close(handle); +} + +TEST_F(AsyncFileHandleWithMockPosixTest, StatFailureReportsError) { + auto handle = createAnonymousFile(); + EXPECT_CALL(mock_posix_file_operations_, fstat(_, _)) + .WillOnce(Return(Api::SysCallIntResult{-1, EBADF})); + std::promise> fstat_status_promise; + EXPECT_OK(handle->stat([&](absl::StatusOr status) { + fstat_status_promise.set_value(std::move(status)); + })); + auto fstat_status = fstat_status_promise.get_future().get(); + EXPECT_THAT(fstat_status, StatusIs(absl::StatusCode::kFailedPrecondition)); + close(handle); +} + TEST_F(AsyncFileHandleWithMockPosixTest, CloseFailureReportsError) { auto handle = createAnonymousFile(); EXPECT_CALL(mock_posix_file_operations_, close(1)) @@ -438,8 +469,7 @@ TEST_F(AsyncFileHandleWithMockPosixTest, DuplicateFailureReportsError) { EXPECT_OK(handle->duplicate( [&](absl::StatusOr status) { dup_status_promise.set_value(status); })); auto dup_status = dup_status_promise.get_future().get(); - EXPECT_EQ(absl::StatusCode::kFailedPrecondition, dup_status.status().code()) - << dup_status.status(); + EXPECT_THAT(dup_status, StatusIs(absl::StatusCode::kFailedPrecondition)); close(handle); } diff --git a/test/extensions/common/async_files/mocks.cc b/test/extensions/common/async_files/mocks.cc index 8a782eb663eda..68ffa093c5936 100644 --- a/test/extensions/common/async_files/mocks.cc +++ b/test/extensions/common/async_files/mocks.cc @@ -11,6 +11,11 @@ using ::testing::_; MockAsyncFileContext::MockAsyncFileContext(std::shared_ptr manager) : manager_(manager) { + ON_CALL(*this, stat(_)) + .WillByDefault([this](std::function)> on_complete) { + return manager_->enqueue( + std::shared_ptr(new TypedMockAsyncFileAction(on_complete))); + }); ON_CALL(*this, createHardLink(_, _)) .WillByDefault([this](absl::string_view, std::function on_complete) { return manager_->enqueue( diff --git a/test/extensions/common/async_files/mocks.h b/test/extensions/common/async_files/mocks.h index cb2dda1aef9f1..f96260710a881 100644 --- a/test/extensions/common/async_files/mocks.h +++ b/test/extensions/common/async_files/mocks.h @@ -20,6 +20,8 @@ class MockAsyncFileContext : public Extensions::Common::AsyncFiles::AsyncFileCon // appropriate MockAsyncFileAction on the MockAsyncFileManager (if one was provided). // These can be consumed by calling MockAsyncFileManager::nextActionCompletes // with the desired parameter to the on_complete callback. + MOCK_METHOD(absl::StatusOr, stat, + (std::function)> on_complete)); MOCK_METHOD(absl::StatusOr, createHardLink, (absl::string_view filename, std::function on_complete)); MOCK_METHOD(absl::Status, close, (std::function on_complete)); diff --git a/test/test_common/status_utility.h b/test/test_common/status_utility.h index 890965ee03389..bd0ea6b02d5d6 100644 --- a/test/test_common/status_utility.h +++ b/test/test_common/status_utility.h @@ -7,22 +7,19 @@ namespace Envoy { namespace StatusHelpers { -// Check that a StatusOr is OK and has a value equal to its argument. +// Check that a StatusOr is OK and has a value equal to or matching its argument. // // For example: // // StatusOr status(3); // EXPECT_THAT(status, IsOkAndHolds(3)); +// EXPECT_THAT(status, IsOkAndHolds(Gt(2))); MATCHER_P(IsOkAndHolds, expected, "") { if (!arg.ok()) { *result_listener << "which has unexpected status: " << arg.status(); return false; } - if (*arg != expected) { - *result_listener << "which has wrong value: " << *arg; - return false; - } - return true; + return ::testing::ExplainMatchResult(expected, *arg, result_listener); } // Check that a StatusOr as a status code equal to its argument. From a3ab997d5e2aa36e49450804c5c61c433adc776e Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 12 Dec 2022 18:05:40 +0000 Subject: [PATCH 16/19] Add stat by filename Signed-off-by: Raven Black --- .../common/async_files/async_file_manager.h | 8 ++++++ .../async_file_manager_thread_pool.cc | 26 +++++++++++++++++++ .../async_file_manager_thread_pool.h | 2 ++ .../async_file_manager_thread_pool_test.cc | 24 +++++++++++++---- test/test_common/status_utility.h | 6 ++++- test/test_common/status_utility_test.cc | 10 ++++++- 6 files changed, 69 insertions(+), 7 deletions(-) diff --git a/source/extensions/common/async_files/async_file_manager.h b/source/extensions/common/async_files/async_file_manager.h index 1eb0a6a57a69c..d515d49c63b56 100644 --- a/source/extensions/common/async_files/async_file_manager.h +++ b/source/extensions/common/async_files/async_file_manager.h @@ -49,6 +49,14 @@ class AsyncFileManager { openExistingFile(absl::string_view filename, Mode mode, std::function)> on_complete) PURE; + // Action to stat a file. + // on_complete receives a stat structure on success, or an error on failure. + // + // Returns a cancellation function, which aborts the operation + // unless the callback has already been called. + virtual CancelFunction stat(absl::string_view filename, + std::function)> on_complete) PURE; + // Action to delete a named file. // on_complete receives OK on success, or an error on failure. // diff --git a/source/extensions/common/async_files/async_file_manager_thread_pool.cc b/source/extensions/common/async_files/async_file_manager_thread_pool.cc index 8126d8361dcfc..1dfd7b84e4c3e 100644 --- a/source/extensions/common/async_files/async_file_manager_thread_pool.cc +++ b/source/extensions/common/async_files/async_file_manager_thread_pool.cc @@ -214,6 +214,26 @@ class ActionOpenExistingFile : public ActionWithFileResult { const AsyncFileManager::Mode mode_; }; +class ActionStat : public AsyncFileActionWithResult> { +public: + ActionStat(Api::OsSysCalls& posix, absl::string_view filename, + std::function)> on_complete) + : AsyncFileActionWithResult(on_complete), posix_(posix), filename_(filename) {} + + absl::StatusOr executeImpl() override { + struct stat ret; + Api::SysCallIntResult stat_result = posix_.stat(filename_.c_str(), &ret); + if (stat_result.return_value_ == -1) { + return statusAfterFileError(stat_result); + } + return ret; + } + +private: + Api::OsSysCalls& posix_; + const std::string filename_; +}; + class ActionUnlink : public AsyncFileActionWithResult { public: ActionUnlink(Api::OsSysCalls& posix, absl::string_view filename, @@ -246,6 +266,12 @@ CancelFunction AsyncFileManagerThreadPool::openExistingFile( return enqueue(std::make_shared(*this, filename, mode, on_complete)); } +CancelFunction +AsyncFileManagerThreadPool::stat(absl::string_view filename, + std::function)> on_complete) { + return enqueue(std::make_shared(posix(), filename, on_complete)); +} + CancelFunction AsyncFileManagerThreadPool::unlink(absl::string_view filename, std::function on_complete) { return enqueue(std::make_shared(posix(), filename, on_complete)); diff --git a/source/extensions/common/async_files/async_file_manager_thread_pool.h b/source/extensions/common/async_files/async_file_manager_thread_pool.h index 0b2ee24a81ea7..2a82202384337 100644 --- a/source/extensions/common/async_files/async_file_manager_thread_pool.h +++ b/source/extensions/common/async_files/async_file_manager_thread_pool.h @@ -36,6 +36,8 @@ class AsyncFileManagerThreadPool : public AsyncFileManager, CancelFunction openExistingFile(absl::string_view filename, Mode mode, std::function)> on_complete) override; + CancelFunction stat(absl::string_view filename, + std::function)> on_complete) override; CancelFunction unlink(absl::string_view filename, std::function on_complete) override; std::string describe() const override; diff --git a/test/extensions/common/async_files/async_file_manager_thread_pool_test.cc b/test/extensions/common/async_files/async_file_manager_thread_pool_test.cc index c7d7d8b74ba19..3ece36519063f 100644 --- a/test/extensions/common/async_files/async_file_manager_thread_pool_test.cc +++ b/test/extensions/common/async_files/async_file_manager_thread_pool_test.cc @@ -25,6 +25,8 @@ namespace Extensions { namespace Common { namespace AsyncFiles { +using StatusHelpers::HasStatusCode; + enum class BlockerState { Start, BlockingDuringExecution, @@ -268,7 +270,7 @@ TEST_F(AsyncFileManagerSingleThreadTest, CreateAnonymousFileWorks) { EXPECT_OK(status); } -TEST_F(AsyncFileManagerSingleThreadTest, OpenExistingFileAndUnlinkWork) { +TEST_F(AsyncFileManagerSingleThreadTest, OpenExistingFileStatAndUnlinkWork) { char filename[1024]; snprintf(filename, sizeof(filename), "%s/async_file.XXXXXX", tmpdir_.c_str()); Api::OsSysCalls& posix = Api::OsSysCallsSingleton().get(); @@ -282,6 +284,11 @@ TEST_F(AsyncFileManagerSingleThreadTest, OpenExistingFileAndUnlinkWork) { EXPECT_OK(handle->close(close_blocker.callback())); absl::Status status = close_blocker.getResult(); EXPECT_OK(status); + WaitForResult> stat_blocker; + manager_->stat(filename, stat_blocker.callback()); + absl::StatusOr stat_result = stat_blocker.getResult(); + EXPECT_OK(stat_result); + EXPECT_EQ(0, stat_result.value().st_size); WaitForResult unlink_blocker; manager_->unlink(filename, unlink_blocker.callback()); status = unlink_blocker.getResult(); @@ -295,14 +302,21 @@ TEST_F(AsyncFileManagerSingleThreadTest, OpenExistingFileFailsForNonexistent) { manager_->openExistingFile(absl::StrCat(tmpdir_, "/nonexistent_file"), AsyncFileManager::Mode::ReadWrite, handle_blocker.callback()); absl::Status status = handle_blocker.getResult().status(); - EXPECT_EQ(absl::StatusCode::kNotFound, status.code()) << status; + EXPECT_THAT(status, HasStatusCode(absl::StatusCode::kNotFound)); +} + +TEST_F(AsyncFileManagerSingleThreadTest, StatFailsForNonexistent) { + WaitForResult> stat_blocker; + manager_->stat(absl::StrCat(tmpdir_, "/nonexistent_file"), stat_blocker.callback()); + absl::StatusOr result = stat_blocker.getResult(); + EXPECT_THAT(result, HasStatusCode(absl::StatusCode::kNotFound)); } TEST_F(AsyncFileManagerSingleThreadTest, UnlinkFailsForNonexistent) { - WaitForResult> handle_blocker; + WaitForResult handle_blocker; manager_->unlink(absl::StrCat(tmpdir_, "/nonexistent_file"), handle_blocker.callback()); - absl::Status status = handle_blocker.getResult().status(); - EXPECT_EQ(absl::StatusCode::kNotFound, status.code()) << status; + absl::Status status = handle_blocker.getResult(); + EXPECT_THAT(status, HasStatusCode(absl::StatusCode::kNotFound)); } } // namespace AsyncFiles diff --git a/test/test_common/status_utility.h b/test/test_common/status_utility.h index bd0ea6b02d5d6..8f4717fce331f 100644 --- a/test/test_common/status_utility.h +++ b/test/test_common/status_utility.h @@ -19,7 +19,11 @@ MATCHER_P(IsOkAndHolds, expected, "") { *result_listener << "which has unexpected status: " << arg.status(); return false; } - return ::testing::ExplainMatchResult(expected, *arg, result_listener); + if (!::testing::Matches(expected)(*arg)) { + *result_listener << "which has wrong value: " << ::testing::PrintToString(*arg); + return false; + } + return true; } // Check that a StatusOr as a status code equal to its argument. diff --git a/test/test_common/status_utility_test.cc b/test/test_common/status_utility_test.cc index 92617500fb8e7..cf4f122e3f9e9 100644 --- a/test/test_common/status_utility_test.cc +++ b/test/test_common/status_utility_test.cc @@ -1,4 +1,5 @@ #include "test/test_common/status_utility.h" +#include namespace Envoy { namespace StatusHelpers { @@ -126,7 +127,14 @@ TEST(StatusUtilityTest, IsOkAndHoldsSuccess) { TEST(StatusUtilityTest, IsOkAndHoldsFailureByValue) { ::testing::StringMatchResultListener listener; - ::testing::ExplainMatchResult(IsOkAndHolds(5), absl::StatusOr{6}, &listener); + EXPECT_FALSE(::testing::ExplainMatchResult(IsOkAndHolds(5), absl::StatusOr{6}, &listener)); + EXPECT_EQ("which has wrong value: 6", listener.str()); +} + +TEST(StatusUtilityTest, IsOkAndHoldsFailureByValueMatcher) { + ::testing::StringMatchResultListener listener; + EXPECT_FALSE(::testing::ExplainMatchResult(IsOkAndHolds(::testing::Lt(4)), absl::StatusOr{6}, + &listener)); EXPECT_EQ("which has wrong value: 6", listener.str()); } From 4fba965d407cb5017fefc5e088374b6a86ab85d4 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 12 Dec 2022 20:01:38 +0000 Subject: [PATCH 17/19] header order Signed-off-by: Raven Black --- test/test_common/status_utility_test.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test_common/status_utility_test.cc b/test/test_common/status_utility_test.cc index cf4f122e3f9e9..e81cd70cb36cd 100644 --- a/test/test_common/status_utility_test.cc +++ b/test/test_common/status_utility_test.cc @@ -1,6 +1,7 @@ -#include "test/test_common/status_utility.h" #include +#include "test/test_common/status_utility.h" + namespace Envoy { namespace StatusHelpers { From 6c5bc8a6e47e1e3fc9feaa578d6458f3d17e5179 Mon Sep 17 00:00:00 2001 From: Raven Black Date: Mon, 12 Dec 2022 20:43:52 +0000 Subject: [PATCH 18/19] Add stat to mock manager Signed-off-by: Raven Black --- test/extensions/common/async_files/mocks.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/extensions/common/async_files/mocks.h b/test/extensions/common/async_files/mocks.h index f96260710a881..80280f900a85e 100644 --- a/test/extensions/common/async_files/mocks.h +++ b/test/extensions/common/async_files/mocks.h @@ -63,6 +63,9 @@ class MockAsyncFileManager : public Extensions::Common::AsyncFiles::AsyncFileMan MOCK_METHOD(CancelFunction, openExistingFile, (absl::string_view filename, Mode mode, std::function)> on_complete)); + MOCK_METHOD(CancelFunction, stat, + (absl::string_view filename, + std::function)> on_complete)); MOCK_METHOD(CancelFunction, unlink, (absl::string_view filename, std::function on_complete)); MOCK_METHOD(std::string, describe, (), (const)); From d3bf6eb5756f447450efc0422fd2733f8e45822b Mon Sep 17 00:00:00 2001 From: Raven Black Date: Tue, 13 Dec 2022 15:47:27 +0000 Subject: [PATCH 19/19] Mock manager stat should enqueue Signed-off-by: Raven Black --- test/extensions/common/async_files/mocks.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/extensions/common/async_files/mocks.cc b/test/extensions/common/async_files/mocks.cc index 68ffa093c5936..347f61af0aba2 100644 --- a/test/extensions/common/async_files/mocks.cc +++ b/test/extensions/common/async_files/mocks.cc @@ -52,6 +52,12 @@ MockAsyncFileManager::MockAsyncFileManager() { queue_.push_back(std::dynamic_pointer_cast(action)); return [this]() { mockCancel(); }; }); + ON_CALL(*this, stat(_, _)) + .WillByDefault( + [this](absl::string_view, std::function)> on_complete) { + return enqueue( + std::shared_ptr(new TypedMockAsyncFileAction(on_complete))); + }); ON_CALL(*this, createAnonymousFile(_, _)) .WillByDefault([this](absl::string_view, std::function)> on_complete) {