[libc-commits] [libc] [libc] android atest compat (PR #93852)
Nick Desaulniers via libc-commits
libc-commits at lists.llvm.org
Thu May 30 11:22:56 PDT 2024
https://github.com/nickdesaulniers updated https://github.com/llvm/llvm-project/pull/93852
>From 2488e241f11e7971fa2fe88f60825a7ff498aaf5 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Wed, 29 May 2024 14:28:13 -0700
Subject: [PATCH 1/5] libc: UnitTest: print number of tests being run
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
---
libc/test/UnitTest/LibcTest.cpp | 29 ++++++++++++++++++++++-------
libc/test/UnitTest/LibcTest.h | 1 +
2 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/libc/test/UnitTest/LibcTest.cpp b/libc/test/UnitTest/LibcTest.cpp
index 846ad331e5237..04c7ad6f8f098 100644
--- a/libc/test/UnitTest/LibcTest.cpp
+++ b/libc/test/UnitTest/LibcTest.cpp
@@ -127,18 +127,34 @@ void Test::addTest(Test *T) {
End = T;
}
+int Test::getNumTests() {
+ int N = 0;
+ for (Test *T = Start; T; T = T->Next, ++N);
+ return N;
+}
+
int Test::runTests(const char *TestFilter) {
- int TestCount = 0;
+ constexpr auto GREEN = "\033[32m";
+ constexpr auto RED = "\033[31m";
+ constexpr auto RESET = "\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';
[[maybe_unused]] const auto start_time = clock();
RunContext Ctx;
@@ -176,7 +192,6 @@ int Test::runTests(const char *TestFilter) {
#endif
break;
}
- ++TestCount;
}
if (TestCount > 0) {
diff --git a/libc/test/UnitTest/LibcTest.h b/libc/test/UnitTest/LibcTest.h
index bba3c6d743bec..3cc3a49229d66 100644
--- a/libc/test/UnitTest/LibcTest.h
+++ b/libc/test/UnitTest/LibcTest.h
@@ -107,6 +107,7 @@ class Test {
internal::RunContext *Ctx = nullptr;
void setContext(internal::RunContext *C) { Ctx = C; }
+ static int getNumTests();
public:
virtual ~Test() {}
>From 3c0af50d73d46225bb8fa471b5e3b76bfaedb68a Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Wed, 29 May 2024 16:00:01 -0700
Subject: [PATCH 2/5] libc: UnitTest: basic command line flag support
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
---
libc/test/UnitTest/LibcTest.cpp | 25 +++++++-------
libc/test/UnitTest/LibcTest.h | 11 +++++-
libc/test/UnitTest/LibcTestMain.cpp | 36 +++++++++++++++++---
libc/test/utils/UnitTest/testfilter_test.cpp | 30 ++++++++--------
4 files changed, 71 insertions(+), 31 deletions(-)
diff --git a/libc/test/UnitTest/LibcTest.cpp b/libc/test/UnitTest/LibcTest.cpp
index 04c7ad6f8f098..52d1fdb8d5212 100644
--- a/libc/test/UnitTest/LibcTest.cpp
+++ b/libc/test/UnitTest/LibcTest.cpp
@@ -133,14 +133,15 @@ int Test::getNumTests() {
return N;
}
-int Test::runTests(const char *TestFilter) {
- constexpr auto GREEN = "\033[32m";
- constexpr auto RED = "\033[31m";
- constexpr auto RESET = "\033[0m";
+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";
+ tlog << green << "[==========] " << reset << "Running " << TestCount
+ << " test";
if (TestCount > 1)
tlog << "s";
tlog << " from 1 test suite.\n";
@@ -155,7 +156,7 @@ int Test::runTests(const char *TestFilter) {
continue;
}
- tlog << GREEN << "[ RUN ] " << RESET << TestName << '\n';
+ tlog << green << "[ RUN ] " << reset << TestName << '\n';
[[maybe_unused]] const auto start_time = clock();
RunContext Ctx;
T->SetUp();
@@ -165,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 {
@@ -180,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";
@@ -200,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 3cc3a49229d66..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 {
@@ -114,7 +123,7 @@ class 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;
}
>From 556d3bf0a5a4a365aec06f10afe8cb86d713519e Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Thu, 30 May 2024 10:55:30 -0700
Subject: [PATCH 3/5] reformat
---
libc/test/UnitTest/LibcTest.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/libc/test/UnitTest/LibcTest.cpp b/libc/test/UnitTest/LibcTest.cpp
index 52d1fdb8d5212..50f12d88a843e 100644
--- a/libc/test/UnitTest/LibcTest.cpp
+++ b/libc/test/UnitTest/LibcTest.cpp
@@ -129,7 +129,8 @@ void Test::addTest(Test *T) {
int Test::getNumTests() {
int N = 0;
- for (Test *T = Start; T; T = T->Next, ++N);
+ for (Test *T = Start; T; T = T->Next, ++N)
+ ;
return N;
}
>From 73d63302e4b2164e76d512f4c9fd8f4d7ebed36a Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Thu, 30 May 2024 10:58:30 -0700
Subject: [PATCH 4/5] update comment about --gtest_print_time
---
libc/test/UnitTest/LibcTestMain.cpp | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/libc/test/UnitTest/LibcTestMain.cpp b/libc/test/UnitTest/LibcTestMain.cpp
index 26711629f3de5..94536e9716468 100644
--- a/libc/test/UnitTest/LibcTestMain.cpp
+++ b/libc/test/UnitTest/LibcTestMain.cpp
@@ -15,10 +15,12 @@ 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.
+// Run unit tests with --gtest_color=no to disable printing colors, or
+// --gtest_print_time to print timings in milliseconds only (as GTest does, so
+// external tools such as Android's atest may expect that format to parse the
+// output). 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;
>From 1eb494d8751dce1167c65ad92a73ce2218de5979 Mon Sep 17 00:00:00 2001
From: Nick Desaulniers <ndesaulniers at google.com>
Date: Thu, 30 May 2024 11:22:41 -0700
Subject: [PATCH 5/5] fix comment about test filter
---
libc/test/UnitTest/LibcTest.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/libc/test/UnitTest/LibcTest.h b/libc/test/UnitTest/LibcTest.h
index a4bbc56885dc0..63ddcee62ef0b 100644
--- a/libc/test/UnitTest/LibcTest.h
+++ b/libc/test/UnitTest/LibcTest.h
@@ -101,7 +101,7 @@ 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?
+ // If set, then just this one test from the suite will be runt.
const char *TestFilter = nullptr;
// Should the test results print color codes to stdout?
bool PrintColor = true;
More information about the libc-commits
mailing list