[libc-commits] [libc] [RFC][libc] shut up passing tests (PR #117253)

Nick Desaulniers via libc-commits libc-commits at lists.llvm.org
Wed Dec 4 09:02:55 PST 2024


https://github.com/nickdesaulniers updated https://github.com/llvm/llvm-project/pull/117253

>From af5b3bc9aaa03f83e8451a940d0d8ea3c78c6b39 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Thu, 21 Nov 2024 14:38:08 -0800
Subject: [PATCH] [RFC][libc] shut up passing tests

When a test fails, you have to scroll quite a bit to find which test failed.
This gets worst over time as we add more tests.

gtest has the option --gtest_brief=1 to silence passing tests from the output.
Add this to our pseudo-gtest framework.

With this change, only failing tests are printed. Example:

  $ ninja libc-unit-tests
  [1206/1357] Running unit test libc.test.src.string.memchr_test.__unit__
FAILED: libc/test/src/string/CMakeFiles/libc.test.src.string.memchr_test.__unit__ /android0/llvm-project/build/libc/test/src/string/CMakeFiles/libc.test.src.string.memchr_test.__unit__
cd /android0/llvm-project/build/libc/test/src/string && /android0/llvm-project/build/libc/test/src/string/libc.test.src.string.memchr_test.__unit__.__build__ --gtest_brief=${BRIEF:-1}
/android0/llvm-project/libc/test/src/string/memchr_test.cpp:24: FAILURE
      Expected: call_memchr(src, 'b', size)
      Which is: bc
To be equal to: "b"
      Which is: b
[  FAILED  ] LlvmLibcMemChrTest.FindsCharacterAfterNullTerminator

There are currently two tests that do print output which can probably also get
cleaned up (they are kind of like expected failures):
- stack_chk_guard_test
- LlvmLibcTestFilterTest.IncorrFilter

To get the old behavior, one can do:

  $ BRIEF=0 ninja libc-unit-tests

Longer term, I'd like to move to use of llvm-lit, as is used throughout most of
the rest of llvm.

Link: https://google.github.io/googletest/advanced.html#suppressing-test-passes
---
 libc/cmake/modules/LLVMLibCTestRules.cmake   |  6 +++---
 libc/test/UnitTest/LibcTest.cpp              | 20 ++++++++++++++------
 libc/test/UnitTest/LibcTest.h                |  2 ++
 libc/test/UnitTest/LibcTestMain.cpp          |  2 ++
 libc/test/utils/UnitTest/testfilter_test.cpp |  3 +++
 5 files changed, 24 insertions(+), 9 deletions(-)

diff --git a/libc/cmake/modules/LLVMLibCTestRules.cmake b/libc/cmake/modules/LLVMLibCTestRules.cmake
index 36f871920c3c33..3312139b4954b2 100644
--- a/libc/cmake/modules/LLVMLibCTestRules.cmake
+++ b/libc/cmake/modules/LLVMLibCTestRules.cmake
@@ -235,7 +235,7 @@ function(create_libc_unittest fq_target_name)
   if(NOT LIBC_UNITTEST_NO_RUN_POSTBUILD)
     add_custom_target(
       ${fq_target_name}
-      COMMAND ${fq_build_target_name}
+      COMMAND ${fq_build_target_name} "--gtest_brief=\$\${BRIEF:-1}"
       COMMENT "Running unit test ${fq_target_name}"
     )
   endif()
@@ -526,7 +526,7 @@ function(add_integration_test test_name)
       $<TARGET_FILE:${fq_build_target_name}> ${INTEGRATION_TEST_ARGS})
   add_custom_target(
     ${fq_target_name}
-    COMMAND ${test_cmd}
+    COMMAND ${test_cmd} "--gtest_brief=\$\${BRIEF:-1}"
     COMMAND_EXPAND_LISTS
     COMMENT "Running integration test ${fq_target_name}"
   )
@@ -719,7 +719,7 @@ function(add_libc_hermetic test_name)
 
   add_custom_command(
     OUTPUT ${fq_target_name}-cmd
-    COMMAND ${test_cmd}
+    COMMAND ${test_cmd} "--gtest_brief=\$\${BRIEF:-1}"
     COMMAND_EXPAND_LISTS
     COMMENT "Running hermetic test ${fq_target_name}"
     ${LIBC_HERMETIC_TEST_JOB_POOL}
diff --git a/libc/test/UnitTest/LibcTest.cpp b/libc/test/UnitTest/LibcTest.cpp
index afb1368f009050..727028a8b9c1f1 100644
--- a/libc/test/UnitTest/LibcTest.cpp
+++ b/libc/test/UnitTest/LibcTest.cpp
@@ -140,7 +140,7 @@ int Test::runTests(const TestOptions &Options) {
   const char *reset = Options.PrintColor ? "\033[0m" : "";
 
   int TestCount = getNumTests();
-  if (TestCount) {
+  if (TestCount && !Options.Brief) {
     tlog << green << "[==========] " << reset << "Running " << TestCount
          << " test";
     if (TestCount > 1)
@@ -157,7 +157,8 @@ int Test::runTests(const TestOptions &Options) {
       continue;
     }
 
-    tlog << green << "[ RUN      ] " << reset << TestName << '\n';
+    if (!Options.Brief)
+      tlog << green << "[ RUN      ] " << reset << TestName << '\n';
     [[maybe_unused]] const uint64_t start_time = clock();
     RunContext Ctx;
     T->SetUp();
@@ -171,7 +172,12 @@ int Test::runTests(const TestOptions &Options) {
       ++FailCount;
       break;
     case RunContext::RunResult::Pass:
-      tlog << green << "[       OK ] " << reset << TestName;
+      if (!Options.Brief)
+        tlog << green << "[       OK ] " << reset << TestName;
+
+      if (Options.Brief)
+        break;
+
 #ifdef LIBC_TEST_USE_CLOCK
       tlog << " (";
       if (start_time > end_time) {
@@ -197,9 +203,11 @@ int Test::runTests(const TestOptions &Options) {
   }
 
   if (TestCount > 0) {
-    tlog << "Ran " << TestCount << " tests. "
-         << " PASS: " << TestCount - FailCount << ' ' << " FAIL: " << FailCount
-         << '\n';
+    if (!Options.Brief) {
+      tlog << "Ran " << TestCount << " tests. "
+           << " PASS: " << TestCount - FailCount << ' '
+           << " FAIL: " << FailCount << '\n';
+    }
   } else {
     tlog << "No tests run.\n";
     if (Options.TestFilter) {
diff --git a/libc/test/UnitTest/LibcTest.h b/libc/test/UnitTest/LibcTest.h
index b4e3819ea958de..a175a4615f6f81 100644
--- a/libc/test/UnitTest/LibcTest.h
+++ b/libc/test/UnitTest/LibcTest.h
@@ -108,6 +108,8 @@ struct TestOptions {
   bool PrintColor = true;
   // Should the test results print timing only in milliseconds, as GTest does?
   bool TimeInMs = false;
+  // Should passing tests be suppressed?
+  bool Brief = false;
 };
 
 // NOTE: One should not create instances and call methods on them directly. One
diff --git a/libc/test/UnitTest/LibcTestMain.cpp b/libc/test/UnitTest/LibcTestMain.cpp
index c348d5ef1aa1b1..3ed728d70fca68 100644
--- a/libc/test/UnitTest/LibcTestMain.cpp
+++ b/libc/test/UnitTest/LibcTestMain.cpp
@@ -31,6 +31,8 @@ TestOptions parseOptions(int argc, char **argv) {
       Options.PrintColor = false;
     else if (arg == "--gtest_print_time")
       Options.TimeInMs = true;
+    else if (arg == "--gtest_brief=1")
+      Options.Brief = true;
     // Ignore other unsupported gtest specific flags.
     else if (arg.starts_with("--gtest_"))
       continue;
diff --git a/libc/test/utils/UnitTest/testfilter_test.cpp b/libc/test/utils/UnitTest/testfilter_test.cpp
index d7e68252cf2aae..d3513dc4169bcf 100644
--- a/libc/test/utils/UnitTest/testfilter_test.cpp
+++ b/libc/test/utils/UnitTest/testfilter_test.cpp
@@ -20,6 +20,9 @@ TEST(LlvmLibcTestFilterTest, NoFilter) {}
 
 TEST(LlvmLibcTestFilterTest, CheckCorrectFilter) {
   TestOptions Options;
+
+  Options.Brief = true;
+
   Options.TestFilter = "LlvmLibcTestFilterTest.NoFilter";
   ASSERT_EQ(LIBC_NAMESPACE::testing::Test::runTests(Options), 0);
 



More information about the libc-commits mailing list