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

[Arthur Eubanks via Phabricator via libc-commits]

aeubanks added inline comments.


================
Comment at: libc/test/utils/CMakeLists.txt:10
 add_subdirectory(tools)
+
+
----------------
extraneous empty lines


================
Comment at: libc/test/utils/UnitTest/testfilter_test.cpp:18
+
+TEST(LlvmLibcTestFilterTest, CorrectFilter) { TestFilter = "CorrectFilter"; }
+
----------------
what are these for?


================
Comment at: libc/test/utils/UnitTest/testfilter_test.cpp:20
+
+TEST(LlvmLibcTestFilterTest, InorrectFilter) { TestFilter = "CorrFilter"; }
+
----------------
`IncorrectFilter`


================
Comment at: libc/test/utils/UnitTest/testfilter_test.cpp:32
+  ASSERT_EQ(__llvm_libc::testing::Test::runTests(
+                "LlvmLibcTestFilterTest.CorrectFilter"),
+            0);
----------------
I'd add a test that tests that we run a test that's not the first test in the file, since that was one of the bugs that came up earlier.

e.g. add a `CorrectFilter2` and run that via a filter


================
Comment at: libc/test/utils/UnitTest/testfilter_test.cpp:37
+int main() {
+  __llvm_libc::testing::Test::runTests(
+      "LlvmLibcTestFilterTest.CheckCorrectFilter");
----------------
it's weird to me that we're using `runTests()` to test `runTests()`, but hey I guess if it works...


================
Comment at: libc/utils/UnitTest/LibcTest.cpp:2
+///===-- Implementation of the base class for libc unittests
+///---------------===//
 //
----------------
this shouldn't change


================
Comment at: libc/utils/UnitTest/LibcTest.cpp:187
+    }
+    return 1;
+  }
----------------
moving this to be part of the return statement below would be clearer, it's confusing when we have multiple unbalanced return statements

something like `return FailCount > 0 || TestCount == 0 ? 1 : 0;`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105843