From 519cfb0ebc9d0f274b2f7895ba4d0321ef21677d Mon Sep 17 00:00:00 2001 From: Siva Chandra Reddy Date: Fri, 22 Sep 2023 08:35:26 +0000 Subject: [PATCH] [libc][NFC] Extend ErrnoSetterMatcher to test expected inequalities. Before this change, ErrnoSetterMatcher only allowed testing for equality of the expected return and errno values. This change extends it to allow testing for expected inequalities of the return and errno values. The test libc.test.src.stdio.fileop_test has been updated to use the ErrnoSetterMatcher with tests for inequalities. --- libc/test/UnitTest/ErrnoSetterMatcher.h | 117 +++++++++++++++++++----- libc/test/src/stdio/fileop_test.cpp | 43 +++++---- 2 files changed, 118 insertions(+), 42 deletions(-) diff --git a/libc/test/UnitTest/ErrnoSetterMatcher.h b/libc/test/UnitTest/ErrnoSetterMatcher.h index 1d786c7e462b1..e0d22f04431fe 100644 --- a/libc/test/UnitTest/ErrnoSetterMatcher.h +++ b/libc/test/UnitTest/ErrnoSetterMatcher.h @@ -21,11 +21,42 @@ namespace testing { namespace internal { +enum class CompareAction { EQ = 0, GE, GT, LE, LT, NE }; + +constexpr const char *CompareMessage[] = { + "equal to", "greater than or equal to", + "greater than", "less than or equal to", + "less than", "not equal to"}; + +template struct Comparator { + CompareAction cmp; + T expected; + bool compare(T actual) { + switch (cmp) { + case CompareAction::EQ: + return actual == expected; + case CompareAction::NE: + return actual != expected; + case CompareAction::GE: + return actual >= expected; + case CompareAction::GT: + return actual > expected; + case CompareAction::LE: + return actual <= expected; + case CompareAction::LT: + return actual < expected; + } + __builtin_unreachable(); + } + + const char *str() { return CompareMessage[static_cast(cmp)]; } +}; + template class ErrnoSetterMatcher : public Matcher { - T ExpectedReturn; - T ActualReturn; - int ExpectedErrno; - int ActualErrno; + Comparator return_cmp; + Comparator errno_cmp; + T actual_return; + int actual_errno; // Even though this is a errno matcher primarily, it has to cater to platforms // which do not have an errno. This predicate checks if errno matching is to @@ -39,38 +70,46 @@ template class ErrnoSetterMatcher : public Matcher { } public: - ErrnoSetterMatcher(T ExpectedReturn, int ExpectedErrno) - : ExpectedReturn(ExpectedReturn), ExpectedErrno(ExpectedErrno) {} + ErrnoSetterMatcher(Comparator rcmp) : return_cmp(rcmp) {} + ErrnoSetterMatcher(Comparator rcmp, Comparator ecmp) + : return_cmp(rcmp), errno_cmp(ecmp) {} + + ErrnoSetterMatcher with_errno(Comparator ecmp) { + errno_cmp = ecmp; + return *this; + } void explainError() override { - if (ActualReturn != ExpectedReturn) { + if (!return_cmp.compare(actual_return)) { if constexpr (cpp::is_floating_point_v) { - tlog << "Expected return value to be: " - << str(fputil::FPBits(ExpectedReturn)) << '\n'; - tlog << " But got: " - << str(fputil::FPBits(ActualReturn)) << '\n'; + tlog << "Expected return value to be " << return_cmp.str() << ": " + << str(fputil::FPBits(return_cmp.expected)) << '\n' + << " But got: " + << str(fputil::FPBits(actual_return)) << '\n'; } else { - tlog << "Expected return value to be " << ExpectedReturn << " but got " - << ActualReturn << ".\n"; + tlog << "Expected return value to be " << return_cmp.str() << " " + << return_cmp.expected << " but got " << actual_return << ".\n"; } } if constexpr (!ignore_errno()) { - if (ActualErrno != ExpectedErrno) { - tlog << "Expected errno to be \"" << get_error_string(ExpectedErrno) - << "\" but got \"" << get_error_string(ActualErrno) << "\".\n"; + if (!errno_cmp.compare(actual_errno)) { + tlog << "Expected errno to be " << errno_cmp.str() << " \"" + << get_error_string(errno_cmp.expected) << "\" but got \"" + << get_error_string(actual_errno) << "\".\n"; } } } - bool match(T Got) { - ActualReturn = Got; - ActualErrno = libc_errno; + bool match(T got) { + actual_return = got; + actual_errno = libc_errno; libc_errno = 0; if constexpr (ignore_errno()) - return Got == ExpectedReturn; + return return_cmp.compare(actual_return); else - return Got == ExpectedReturn && ActualErrno == ExpectedErrno; + return return_cmp.compare(actual_return) && + errno_cmp.compare(actual_errno); } }; @@ -78,16 +117,48 @@ template class ErrnoSetterMatcher : public Matcher { namespace ErrnoSetterMatcher { +template internal::Comparator LT(T val) { + return internal::Comparator{internal::CompareAction::LT, val}; +} + +template internal::Comparator LE(T val) { + return internal::Comparator{internal::CompareAction::LE, val}; +} + +template internal::Comparator GT(T val) { + return internal::Comparator{internal::CompareAction::GT, val}; +} + +template internal::Comparator GE(T val) { + return internal::Comparator{internal::CompareAction::GE, val}; +} + +template internal::Comparator EQ(T val) { + return internal::Comparator{internal::CompareAction::EQ, val}; +} + +template internal::Comparator NE(T val) { + return internal::Comparator{internal::CompareAction::NE, val}; +} + template static internal::ErrnoSetterMatcher Succeeds(RetT ExpectedReturn = 0, int ExpectedErrno = 0) { - return {ExpectedReturn, ExpectedErrno}; + return internal::ErrnoSetterMatcher(EQ(ExpectedReturn), + EQ(ExpectedErrno)); } template static internal::ErrnoSetterMatcher Fails(int ExpectedErrno, RetT ExpectedReturn = -1) { - return {ExpectedReturn, ExpectedErrno}; + return internal::ErrnoSetterMatcher(EQ(ExpectedReturn), + EQ(ExpectedErrno)); +} + +template +static internal::ErrnoSetterMatcher +returns(internal::Comparator cmp) { + return internal::ErrnoSetterMatcher(cmp); } } // namespace ErrnoSetterMatcher diff --git a/libc/test/src/stdio/fileop_test.cpp b/libc/test/src/stdio/fileop_test.cpp index 989de2963cf9c..68a749a6c93e7 100644 --- a/libc/test/src/stdio/fileop_test.cpp +++ b/libc/test/src/stdio/fileop_test.cpp @@ -16,11 +16,16 @@ #include "src/stdio/fread.h" #include "src/stdio/fseek.h" #include "src/stdio/fwrite.h" +#include "test/UnitTest/ErrnoSetterMatcher.h" #include "test/UnitTest/Test.h" #include "src/errno/libc_errno.h" #include +using __llvm_libc::testing::ErrnoSetterMatcher::EQ; +using __llvm_libc::testing::ErrnoSetterMatcher::NE; +using __llvm_libc::testing::ErrnoSetterMatcher::returns; + TEST(LlvmLibcFILETest, SimpleFileOperations) { constexpr char FILENAME[] = "testdata/simple_operations.test"; ::FILE *file = __llvm_libc::fopen(FILENAME, "w"); @@ -31,9 +36,9 @@ TEST(LlvmLibcFILETest, SimpleFileOperations) { // This is not a readable file. char read_data[sizeof(CONTENT)]; - ASSERT_EQ(__llvm_libc::fread(read_data, 1, sizeof(CONTENT), file), size_t(0)); + ASSERT_THAT(__llvm_libc::fread(read_data, 1, sizeof(CONTENT), file), + returns(EQ(size_t(0))).with_errno(NE(0))); ASSERT_NE(__llvm_libc::ferror(file), 0); - EXPECT_NE(libc_errno, 0); libc_errno = 0; __llvm_libc::clearerr(file); @@ -62,25 +67,25 @@ TEST(LlvmLibcFILETest, SimpleFileOperations) { ASSERT_NE(__llvm_libc::feof(file), 0); // Should be an error to write. - ASSERT_EQ(size_t(0), __llvm_libc::fwrite(CONTENT, 1, sizeof(CONTENT), file)); + ASSERT_THAT(__llvm_libc::fwrite(CONTENT, 1, sizeof(CONTENT), file), + returns(EQ(size_t(0))).with_errno(NE(0))); ASSERT_NE(__llvm_libc::ferror(file), 0); - ASSERT_NE(libc_errno, 0); libc_errno = 0; __llvm_libc::clearerr(file); // Should be an error to puts. - ASSERT_EQ(EOF, __llvm_libc::fputs(CONTENT, file)); + ASSERT_THAT(__llvm_libc::fputs(CONTENT, file), + returns(EQ(EOF)).with_errno(NE(0))); ASSERT_NE(__llvm_libc::ferror(file), 0); - ASSERT_NE(libc_errno, 0); libc_errno = 0; __llvm_libc::clearerr(file); ASSERT_EQ(__llvm_libc::ferror(file), 0); libc_errno = 0; - ASSERT_EQ(__llvm_libc::fwrite("nothing", 1, 1, file), size_t(0)); - ASSERT_NE(libc_errno, 0); + ASSERT_THAT(__llvm_libc::fwrite("nothing", 1, 1, file), + returns(EQ(0)).with_errno(NE(0))); libc_errno = 0; ASSERT_EQ(__llvm_libc::fclose(file), 0); @@ -97,8 +102,8 @@ TEST(LlvmLibcFILETest, SimpleFileOperations) { // This is not a readable file. libc_errno = 0; - ASSERT_EQ(__llvm_libc::fread(data, 1, 1, file), size_t(0)); - ASSERT_NE(libc_errno, 0); + ASSERT_THAT(__llvm_libc::fread(data, 1, 1, file), + returns(EQ(0)).with_errno(NE(0))); libc_errno = 0; ASSERT_EQ(0, __llvm_libc::fclose(file)); @@ -162,22 +167,22 @@ TEST(LlvmLibcFILETest, FOpenFWriteSizeGreaterThanOne) { FILE *file = __llvm_libc::fopen(FILENAME, "w"); ASSERT_FALSE(file == nullptr); ASSERT_EQ(size_t(0), __llvm_libc::fwrite(WRITE_DATA, 0, 1, file)); - ASSERT_EQ(WRITE_NMEMB, __llvm_libc::fwrite(WRITE_DATA, sizeof(MyStruct), - WRITE_NMEMB, file)); - EXPECT_EQ(libc_errno, 0); + ASSERT_THAT( + __llvm_libc::fwrite(WRITE_DATA, sizeof(MyStruct), WRITE_NMEMB, file), + returns(EQ(WRITE_NMEMB)).with_errno(EQ(0))); ASSERT_EQ(__llvm_libc::fclose(file), 0); file = __llvm_libc::fopen(FILENAME, "r"); ASSERT_FALSE(file == nullptr); MyStruct read_data[WRITE_NMEMB]; ASSERT_EQ(size_t(0), __llvm_libc::fread(read_data, 0, 1, file)); - ASSERT_EQ(WRITE_NMEMB, - __llvm_libc::fread(read_data, sizeof(MyStruct), WRITE_NMEMB, file)); - EXPECT_EQ(libc_errno, 0); + ASSERT_THAT( + __llvm_libc::fread(read_data, sizeof(MyStruct), WRITE_NMEMB, file), + returns(EQ(WRITE_NMEMB)).with_errno(EQ(0))); // Trying to read more should fetch nothing. - ASSERT_EQ(size_t(0), - __llvm_libc::fread(read_data, sizeof(MyStruct), WRITE_NMEMB, file)); - EXPECT_EQ(libc_errno, 0); + ASSERT_THAT( + __llvm_libc::fread(read_data, sizeof(MyStruct), WRITE_NMEMB, file), + returns(EQ(0)).with_errno(EQ(0))); EXPECT_NE(__llvm_libc::feof(file), 0); EXPECT_EQ(__llvm_libc::ferror(file), 0); ASSERT_EQ(__llvm_libc::fclose(file), 0);