[libc-commits] [PATCH] D105843: [libc] Add option to run specific tests

Arthur Eubanks via Phabricator via libc-commits libc-commits at lists.llvm.org
Tue Jul 13 16:26:10 PDT 2021


aeubanks added a comment.

this is looking much better

I think you need to clang-format again



================
Comment at: libc/utils/UnitTest/LibcTest.cpp:183
+  else {
+    std::cout << "No tests run." << '\n';
+    if(TestFilter)  {
----------------
"No tests run.\n"


================
Comment at: libc/utils/UnitTest/LibcTest.cpp:186
+      std::cout << "No matching test for " << TestFilter << '\n';
+      return 1;
+    }
----------------
I'm tempted to always fail if we didn't run any tests since that seems like it should never happen, any thoughts?


================
Comment at: libc/utils/UnitTest/LibcTest.cpp:363
 
+const char * setTestFilter(int len,char *input[])  {
+  return len > 1 ? input[1] : NULL;
----------------
this is more of a `get` than a `set`


================
Comment at: libc/utils/UnitTest/LibcTest.cpp:363
 
+const char * setTestFilter(int len,char *input[])  {
+  return len > 1 ? input[1] : NULL;
----------------
aeubanks wrote:
> this is more of a `get` than a `set`
I think keeping `argc`/`argv` as the variable names is clearer, that way it's obvious these are directly accessing the command line flags


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105843



More information about the libc-commits mailing list