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

Siva Chandra via Phabricator via libc-commits libc-commits at lists.llvm.org
Wed Mar 4 11:15:42 PST 2020

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.



More information about the libc-commits mailing list