[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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75487/new/
https://reviews.llvm.org/D75487
More information about the libc-commits
mailing list