[libc-commits] [libc] [libc][NFC] Extend ErrnoSetterMatcher to test expected inequalities. (PR #67153)

Siva Chandra via libc-commits libc-commits at lists.llvm.org
Fri Sep 22 07:57:57 PDT 2023


https://github.com/sivachandra created https://github.com/llvm/llvm-project/pull/67153

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.


>From 519cfb0ebc9d0f274b2f7895ba4d0321ef21677d Mon Sep 17 00:00:00 2001
From: Siva Chandra Reddy <sivachandra at google.com>
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 1d786c7e462b159..e0d22f04431fe43 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 <typename T> 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<int>(cmp)]; }
+};
+
 template <typename T> class ErrnoSetterMatcher : public Matcher<T> {
-  T ExpectedReturn;
-  T ActualReturn;
-  int ExpectedErrno;
-  int ActualErrno;
+  Comparator<T> return_cmp;
+  Comparator<int> 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 <typename T> class ErrnoSetterMatcher : public Matcher<T> {
   }
 
 public:
-  ErrnoSetterMatcher(T ExpectedReturn, int ExpectedErrno)
-      : ExpectedReturn(ExpectedReturn), ExpectedErrno(ExpectedErrno) {}
+  ErrnoSetterMatcher(Comparator<T> rcmp) : return_cmp(rcmp) {}
+  ErrnoSetterMatcher(Comparator<T> rcmp, Comparator<int> ecmp)
+      : return_cmp(rcmp), errno_cmp(ecmp) {}
+
+  ErrnoSetterMatcher<T> with_errno(Comparator<int> 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<T>) {
-        tlog << "Expected return value to be: "
-             << str(fputil::FPBits<T>(ExpectedReturn)) << '\n';
-        tlog << "                    But got: "
-             << str(fputil::FPBits<T>(ActualReturn)) << '\n';
+        tlog << "Expected return value to be " << return_cmp.str() << ": "
+             << str(fputil::FPBits<T>(return_cmp.expected)) << '\n'
+             << "                    But got: "
+             << str(fputil::FPBits<T>(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 <typename T> class ErrnoSetterMatcher : public Matcher<T> {
 
 namespace ErrnoSetterMatcher {
 
+template <typename T> internal::Comparator<T> LT(T val) {
+  return internal::Comparator<T>{internal::CompareAction::LT, val};
+}
+
+template <typename T> internal::Comparator<T> LE(T val) {
+  return internal::Comparator<T>{internal::CompareAction::LE, val};
+}
+
+template <typename T> internal::Comparator<T> GT(T val) {
+  return internal::Comparator<T>{internal::CompareAction::GT, val};
+}
+
+template <typename T> internal::Comparator<T> GE(T val) {
+  return internal::Comparator<T>{internal::CompareAction::GE, val};
+}
+
+template <typename T> internal::Comparator<T> EQ(T val) {
+  return internal::Comparator<T>{internal::CompareAction::EQ, val};
+}
+
+template <typename T> internal::Comparator<T> NE(T val) {
+  return internal::Comparator<T>{internal::CompareAction::NE, val};
+}
+
 template <typename RetT = int>
 static internal::ErrnoSetterMatcher<RetT> Succeeds(RetT ExpectedReturn = 0,
                                                    int ExpectedErrno = 0) {
-  return {ExpectedReturn, ExpectedErrno};
+  return internal::ErrnoSetterMatcher<RetT>(EQ(ExpectedReturn),
+                                            EQ(ExpectedErrno));
 }
 
 template <typename RetT = int>
 static internal::ErrnoSetterMatcher<RetT> Fails(int ExpectedErrno,
                                                 RetT ExpectedReturn = -1) {
-  return {ExpectedReturn, ExpectedErrno};
+  return internal::ErrnoSetterMatcher<RetT>(EQ(ExpectedReturn),
+                                            EQ(ExpectedErrno));
+}
+
+template <typename RetT>
+static internal::ErrnoSetterMatcher<RetT>
+returns(internal::Comparator<RetT> cmp) {
+  return internal::ErrnoSetterMatcher<RetT>(cmp);
 }
 
 } // namespace ErrnoSetterMatcher
diff --git a/libc/test/src/stdio/fileop_test.cpp b/libc/test/src/stdio/fileop_test.cpp
index 989de2963cf9cc6..68a749a6c93e73f 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 <stdio.h>
 
+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);



More information about the libc-commits mailing list