Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc] android atest compat #93852

Merged
merged 7 commits into from
May 30, 2024

Conversation

nickdesaulniers
Copy link
Member

See individual commit messages in GH PR Review.

These changes slighly modify the output of the unittests so that they better
match GTest, so that utilities that parse the expected output from GTest (such
as Android's unit test harness) can read the output from our unit tests.

This allows our unit tests to be run on Android devices.

  • libc: UnitTest: print number of tests being run
  • libc: UnitTest: basic command line flag support

Android's atest unit test harness assumes GTest specific output in order to
know how many unit tests there are in total. Print the test number count before
each run, so that atest can parse this value correctly.

Link: https://android-review.googlesource.com/c/platform/external/llvm-libc/+/3107252
Android's atest unit test harness assumes GTest specific output in order to
parse the results of running the unit tests.

Add very primitive command line parsing to:
- support --gtest_color=no to disable printing terminal colors.
- recognize --gtest_print_time and print the test time in milliseconds.
  - most of our unit tests run on the order of microseconds, so its useful to
    preserve the existing behavior.  But upsteram GTest ONLY prints time tests
    in milliseconds, and Android's atest expects to be able to parse exactly
    that. Atest always passes --gtest_print_time. The word `took` is removed as
    that also differs from upstream GTest, tripping up parsers.
- ignore other --gtest_* flags

Do so so that atest can parse the output correctly.

Link: https://android-review.googlesource.com/c/platform/external/llvm-libc/+/3107252
Link: https://google.github.io/googletest/advanced.html#colored-terminal-output
Link: https://google.github.io/googletest/advanced.html#suppressing-the-elapsed-time
@llvmbot llvmbot added the libc label May 30, 2024
@nickdesaulniers nickdesaulniers changed the title android atest compat [libc] android atest compat May 30, 2024
@llvmbot
Copy link

llvmbot commented May 30, 2024

@llvm/pr-subscribers-libc

Author: Nick Desaulniers (paternity leave) (nickdesaulniers)

Changes

See individual commit messages in GH PR Review.

These changes slighly modify the output of the unittests so that they better
match GTest, so that utilities that parse the expected output from GTest (such
as Android's unit test harness) can read the output from our unit tests.

This allows our unit tests to be run on Android devices.

  • libc: UnitTest: print number of tests being run
  • libc: UnitTest: basic command line flag support

Full diff: https://github.com/llvm/llvm-project/pull/93852.diff

4 Files Affected:

  • (modified) libc/test/UnitTest/LibcTest.cpp (+31-15)
  • (modified) libc/test/UnitTest/LibcTest.h (+11-1)
  • (modified) libc/test/UnitTest/LibcTestMain.cpp (+32-4)
  • (modified) libc/test/utils/UnitTest/testfilter_test.cpp (+16-14)
diff --git a/libc/test/UnitTest/LibcTest.cpp b/libc/test/UnitTest/LibcTest.cpp
index 846ad331e5237..52d1fdb8d5212 100644
--- a/libc/test/UnitTest/LibcTest.cpp
+++ b/libc/test/UnitTest/LibcTest.cpp
@@ -127,19 +127,36 @@ void Test::addTest(Test *T) {
   End = T;
 }
 
-int Test::runTests(const char *TestFilter) {
-  int TestCount = 0;
+int Test::getNumTests() {
+  int N = 0;
+  for (Test *T = Start; T; T = T->Next, ++N);
+  return N;
+}
+
+int Test::runTests(const TestOptions &Options) {
+  const char *green = Options.PrintColor ? "\033[32m" : "";
+  const char *red = Options.PrintColor ? "\033[31m" : "";
+  const char *reset = Options.PrintColor ? "\033[0m" : "";
+
+  int TestCount = getNumTests();
+  if (TestCount) {
+    tlog << green << "[==========] " << reset << "Running " << TestCount
+         << " test";
+    if (TestCount > 1)
+      tlog << "s";
+    tlog << " from 1 test suite.\n";
+  }
+
   int FailCount = 0;
   for (Test *T = Start; T != nullptr; T = T->Next) {
     const char *TestName = T->getName();
-    cpp::string StrTestName(TestName);
-    constexpr auto GREEN = "\033[32m";
-    constexpr auto RED = "\033[31m";
-    constexpr auto RESET = "\033[0m";
-    if ((TestFilter != nullptr) && (StrTestName != TestFilter)) {
+
+    if (Options.TestFilter && cpp::string{TestName} != Options.TestFilter) {
+      --TestCount;
       continue;
     }
-    tlog << GREEN << "[ RUN      ] " << RESET << TestName << '\n';
+
+    tlog << green << "[ RUN      ] " << reset << TestName << '\n';
     [[maybe_unused]] const auto start_time = clock();
     RunContext Ctx;
     T->SetUp();
@@ -149,13 +166,13 @@ int Test::runTests(const char *TestFilter) {
     [[maybe_unused]] const auto end_time = clock();
     switch (Ctx.status()) {
     case RunContext::RunResult::Fail:
-      tlog << RED << "[  FAILED  ] " << RESET << TestName << '\n';
+      tlog << red << "[  FAILED  ] " << reset << TestName << '\n';
       ++FailCount;
       break;
     case RunContext::RunResult::Pass:
-      tlog << GREEN << "[       OK ] " << RESET << TestName;
+      tlog << green << "[       OK ] " << reset << TestName;
 #ifdef LIBC_TEST_USE_CLOCK
-      tlog << " (took ";
+      tlog << " (";
       if (start_time > end_time) {
         tlog << "unknown - try rerunning)\n";
       } else {
@@ -164,7 +181,7 @@ int Test::runTests(const char *TestFilter) {
         const uint64_t duration_us = (duration * 1000 * 1000) / CLOCKS_PER_SEC;
         const uint64_t duration_ns =
             (duration * 1000 * 1000 * 1000) / CLOCKS_PER_SEC;
-        if (duration_ms != 0)
+        if (Options.TimeInMs || duration_ms != 0)
           tlog << duration_ms << " ms)\n";
         else if (duration_us != 0)
           tlog << duration_us << " us)\n";
@@ -176,7 +193,6 @@ int Test::runTests(const char *TestFilter) {
 #endif
       break;
     }
-    ++TestCount;
   }
 
   if (TestCount > 0) {
@@ -185,8 +201,8 @@ int Test::runTests(const char *TestFilter) {
          << '\n';
   } else {
     tlog << "No tests run.\n";
-    if (TestFilter) {
-      tlog << "No matching test for " << TestFilter << '\n';
+    if (Options.TestFilter) {
+      tlog << "No matching test for " << Options.TestFilter << '\n';
     }
   }
 
diff --git a/libc/test/UnitTest/LibcTest.h b/libc/test/UnitTest/LibcTest.h
index bba3c6d743bec..a4bbc56885dc0 100644
--- a/libc/test/UnitTest/LibcTest.h
+++ b/libc/test/UnitTest/LibcTest.h
@@ -100,6 +100,15 @@ bool test(RunContext *Ctx, TestCond Cond, ValType LHS, ValType RHS,
 
 } // namespace internal
 
+struct TestOptions {
+  // A single test name not to run. TODO: does anyone care/use this?
+  const char *TestFilter = nullptr;
+  // Should the test results print color codes to stdout?
+  bool PrintColor = true;
+  // Should the test results print timing only in milliseconds, as GTest does?
+  bool TimeInMs = false;
+};
+
 // NOTE: One should not create instances and call methods on them directly. One
 // should use the macros TEST or TEST_F to write test cases.
 class Test {
@@ -107,13 +116,14 @@ class Test {
   internal::RunContext *Ctx = nullptr;
 
   void setContext(internal::RunContext *C) { Ctx = C; }
+  static int getNumTests();
 
 public:
   virtual ~Test() {}
   virtual void SetUp() {}
   virtual void TearDown() {}
 
-  static int runTests(const char *);
+  static int runTests(const TestOptions &Options);
 
 protected:
   static void addTest(Test *T);
diff --git a/libc/test/UnitTest/LibcTestMain.cpp b/libc/test/UnitTest/LibcTestMain.cpp
index bf1a921c04ca9..26711629f3de5 100644
--- a/libc/test/UnitTest/LibcTestMain.cpp
+++ b/libc/test/UnitTest/LibcTestMain.cpp
@@ -7,16 +7,44 @@
 //===----------------------------------------------------------------------===//
 
 #include "LibcTest.h"
+#include "src/__support/CPP/string_view.h"
 
-static const char *getTestFilter(int argc, char *argv[]) {
-  return argc > 1 ? argv[1] : nullptr;
+using LIBC_NAMESPACE::cpp::string_view;
+using LIBC_NAMESPACE::testing::TestOptions;
+
+namespace {
+
+// A poor-man's getopt_long.
+// Run unit tests with --gtest_color=no to disable printing colors. Other
+// command line flags starting with --gtest_ are ignored. Otherwise, the last
+// command line arg is used as a test filter, if command line args are
+// specified.
+TestOptions parseOptions(int argc, char **argv) {
+  TestOptions Options;
+
+  for (int i = 1; i < argc; ++i) {
+    string_view arg{argv[i]};
+
+    if (arg == "--gtest_color=no")
+      Options.PrintColor = false;
+    else if (arg == "--gtest_print_time")
+      Options.TimeInMs = true;
+    // Ignore other unsupported gtest specific flags.
+    else if (arg.starts_with("--gtest_"))
+      continue;
+    else
+      Options.TestFilter = argv[i];
+  }
+
+  return Options;
 }
 
+} // anonymous namespace
+
 extern "C" int main(int argc, char **argv, char **envp) {
   LIBC_NAMESPACE::testing::argc = argc;
   LIBC_NAMESPACE::testing::argv = argv;
   LIBC_NAMESPACE::testing::envp = envp;
 
-  const char *TestFilter = getTestFilter(argc, argv);
-  return LIBC_NAMESPACE::testing::Test::runTests(TestFilter);
+  return LIBC_NAMESPACE::testing::Test::runTests(parseOptions(argc, argv));
 }
diff --git a/libc/test/utils/UnitTest/testfilter_test.cpp b/libc/test/utils/UnitTest/testfilter_test.cpp
index 567b5e2bde855..d7e68252cf2aa 100644
--- a/libc/test/utils/UnitTest/testfilter_test.cpp
+++ b/libc/test/utils/UnitTest/testfilter_test.cpp
@@ -8,6 +8,8 @@
 
 #include "test/UnitTest/LibcTest.h"
 
+using LIBC_NAMESPACE::testing::TestOptions;
+
 TEST(LlvmLibcTestFilterTest, CorrectFilter) {}
 
 TEST(LlvmLibcTestFilterTest, CorrectFilter2) {}
@@ -17,22 +19,22 @@ TEST(LlvmLibcTestFilterTest, IncorrectFilter) {}
 TEST(LlvmLibcTestFilterTest, NoFilter) {}
 
 TEST(LlvmLibcTestFilterTest, CheckCorrectFilter) {
-  ASSERT_EQ(LIBC_NAMESPACE::testing::Test::runTests(
-                "LlvmLibcTestFilterTest.NoFilter"),
-            0);
-  ASSERT_EQ(LIBC_NAMESPACE::testing::Test::runTests(
-                "LlvmLibcTestFilterTest.IncorrFilter"),
-            1);
-  ASSERT_EQ(LIBC_NAMESPACE::testing::Test::runTests(
-                "LlvmLibcTestFilterTest.CorrectFilter"),
-            0);
-  ASSERT_EQ(LIBC_NAMESPACE::testing::Test::runTests(
-                "LlvmLibcTestFilterTest.CorrectFilter2"),
-            0);
+  TestOptions Options;
+  Options.TestFilter = "LlvmLibcTestFilterTest.NoFilter";
+  ASSERT_EQ(LIBC_NAMESPACE::testing::Test::runTests(Options), 0);
+
+  Options.TestFilter = "LlvmLibcTestFilterTest.IncorrFilter";
+  ASSERT_EQ(LIBC_NAMESPACE::testing::Test::runTests(Options), 1);
+
+  Options.TestFilter = "LlvmLibcTestFilterTest.CorrectFilter";
+  ASSERT_EQ(LIBC_NAMESPACE::testing::Test::runTests(Options), 0);
+
+  Options.TestFilter = "LlvmLibcTestFilterTest.CorrectFilter2";
+  ASSERT_EQ(LIBC_NAMESPACE::testing::Test::runTests(Options), 0);
 }
 
 int main() {
-  LIBC_NAMESPACE::testing::Test::runTests(
-      "LlvmLibcTestFilterTest.CheckCorrectFilter");
+  TestOptions Options{"LlvmLibcTestFilterTest.NoFilter", /*PrintColor=*/true};
+  LIBC_NAMESPACE::testing::Test::runTests(Options);
   return 0;
 }

@nickdesaulniers
Copy link
Member Author

cc @enh-google

Copy link

github-actions bot commented May 30, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a nit

libc/test/UnitTest/LibcTest.h Outdated Show resolved Hide resolved
@nickdesaulniers nickdesaulniers merged commit 6f576d9 into llvm:main May 30, 2024
4 of 5 checks passed
@nickdesaulniers nickdesaulniers deleted the android_atest_compat branch May 30, 2024 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants