[libc-commits] [PATCH] D75487: [libc] [UnitTest] Add Matchers

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/utils/UnitTest/Matchers.h:1
+//===-------------------------- Matchers.h ----------------------*- C++ -*-===//
+//
----------------
I didn't go look what gtest's suggested organization is, but instead of putting all matchers in `Matchers.h`, does it make sense to put the `ErrnoSetterMacther` class in its own header file?


================
Comment at: libc/utils/UnitTest/Matchers.h:16
+extern "C" const char *strerror(int);
+
+template <typename T>
----------------
Why not enclose in `__llvm_libc::testing`? To avoid verbosity?


================
Comment at: libc/utils/UnitTest/Matchers.h:48
+template <typename T = int>
+ErrnoSetterMatcher<T> Succeeds(T ExpectedReturn = 0, int ExpectedErrno = 0) {
+  return ErrnoSetterMatcher<T>(ExpectedReturn, ExpectedErrno);
----------------
These names are generic enough that some kind of specificity would be nice? I am not able to think of an alternate. May be `ErrnoSetterSucceeds` and `ErrnoSetterFails`?


================
Comment at: libc/utils/testutils/StreamWrapper.h:1
+//===------------------------ StreamWrapper.h -------------------*- C++ -*-===//
+//
----------------
I don't necessarily like this, but can't think of an alternate. So, LGTM.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75487/new/

https://reviews.llvm.org/D75487