From 23438deabba66b55ef9d780d75791eca2565b5af Mon Sep 17 00:00:00 2001 From: Evan Shrubsole Date: Fri, 29 Nov 2024 10:23:27 +0000 Subject: [PATCH] Allow enums that have AbslStringify to be logged as text When an enum has AbslStringify, we log the text coming from stringifying it, not the numeric value. Drive by changes, 1. Changed the tests to use string matchers rather than std::string::find. 2. Fixed test includes. 3. Fix spelling. Bug: webrtc:381502973 Change-Id: I6bab0afda1b20d72c02629e80ff2ac567650183a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/369861 Auto-Submit: Evan Shrubsole Commit-Queue: Evan Shrubsole Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#43471} --- rtc_base/logging.h | 1 + rtc_base/logging_unittest.cc | 85 +++++++++++++++++++++++------------- 2 files changed, 55 insertions(+), 31 deletions(-) diff --git a/rtc_base/logging.h b/rtc_base/logging.h index 6d5a4bebcf..8131102944 100644 --- a/rtc_base/logging.h +++ b/rtc_base/logging.h @@ -318,6 +318,7 @@ inline Val MakeVal( // The enum class types are not implicitly convertible to arithmetic types. template ::value && + !absl::HasAbslStringify::value && !std::is_arithmetic::value>* = nullptr> inline decltype(MakeVal(std::declval>())) MakeVal( T x) { diff --git a/rtc_base/logging_unittest.cc b/rtc_base/logging_unittest.cc index 1186e4fb5a..d50ade0d16 100644 --- a/rtc_base/logging_unittest.cc +++ b/rtc_base/logging_unittest.cc @@ -12,14 +12,11 @@ #if RTC_LOG_ENABLED() -#include - -#include +#include #include "absl/strings/string_view.h" #include "rtc_base/arraysize.h" #include "rtc_base/checks.h" -#include "rtc_base/event.h" #include "rtc_base/platform_thread.h" #include "rtc_base/time_utils.h" #include "test/gmock.h" @@ -97,8 +94,8 @@ TEST(LogTest, SingleStream) { RTC_LOG(LS_INFO) << "INFO"; RTC_LOG(LS_VERBOSE) << "VERBOSE"; - EXPECT_NE(std::string::npos, str.find("INFO")); - EXPECT_EQ(std::string::npos, str.find("VERBOSE")); + EXPECT_THAT(str, HasSubstr("INFO")); + EXPECT_THAT(str, Not(HasSubstr("VERBOSE"))); int i = 1; long l = 2l; @@ -114,32 +111,32 @@ TEST(LogTest, SingleStream) { const char* null_string = nullptr; void* p = reinterpret_cast(0xabcd); - // Log all suported types(except doubles/floats) as a sanity-check. + // Log all supported types(except doubles/floats) as a sanity-check. RTC_LOG(LS_INFO) << "|" << i << "|" << l << "|" << ll << "|" << u << "|" << ul << "|" << ull << "|" << s1.c_str() << "|" << s2 << "|" << absl::string_view(s3) << "|" << p << "|" << null_string << "|"; // Signed integers - EXPECT_NE(std::string::npos, str.find("|1|")); - EXPECT_NE(std::string::npos, str.find("|2|")); - EXPECT_NE(std::string::npos, str.find("|3|")); + EXPECT_THAT(str, HasSubstr("|1|")); + EXPECT_THAT(str, HasSubstr("|2|")); + EXPECT_THAT(str, HasSubstr("|3|")); // Unsigned integers - EXPECT_NE(std::string::npos, str.find("|4|")); - EXPECT_NE(std::string::npos, str.find("|5|")); - EXPECT_NE(std::string::npos, str.find("|6|")); + EXPECT_THAT(str, HasSubstr("|4|")); + EXPECT_THAT(str, HasSubstr("|5|")); + EXPECT_THAT(str, HasSubstr("|6|")); // Strings - EXPECT_NE(std::string::npos, str.find("|char*|")); - EXPECT_NE(std::string::npos, str.find("|std::string|")); - EXPECT_NE(std::string::npos, str.find("|absl::stringview|")); + EXPECT_THAT(str, HasSubstr("|char*|")); + EXPECT_THAT(str, HasSubstr("|std::string|")); + EXPECT_THAT(str, HasSubstr("|absl::stringview|")); // void* - EXPECT_NE(std::string::npos, str.find("|abcd|")); + EXPECT_THAT(str, HasSubstr("|abcd|")); // null char* - EXPECT_NE(std::string::npos, str.find("|(null)|")); + EXPECT_THAT(str, HasSubstr("|(null)|")); LogMessage::RemoveLogToStream(&stream); EXPECT_EQ(LS_NONE, LogMessage::GetLogToStream(&stream)); @@ -152,7 +149,7 @@ TEST(LogTest, LogIfLogIfConditionIsTrue) { LogMessage::AddLogToStream(&stream, LS_INFO); RTC_LOG_IF(LS_INFO, true) << "Hello"; - EXPECT_NE(std::string::npos, str.find("Hello")); + EXPECT_THAT(str, HasSubstr("Hello")); LogMessage::RemoveLogToStream(&stream); } @@ -163,7 +160,7 @@ TEST(LogTest, LogIfDontLogIfConditionIsFalse) { LogMessage::AddLogToStream(&stream, LS_INFO); RTC_LOG_IF(LS_INFO, false) << "Hello"; - EXPECT_EQ(std::string::npos, str.find("Hello")); + EXPECT_THAT(str, Not(HasSubstr("Hello"))); LogMessage::RemoveLogToStream(&stream); } @@ -174,8 +171,8 @@ TEST(LogTest, LogIfFLogIfConditionIsTrue) { LogMessage::AddLogToStream(&stream, LS_INFO); RTC_LOG_IF_F(LS_INFO, true) << "Hello"; - EXPECT_NE(std::string::npos, str.find(__FUNCTION__)); - EXPECT_NE(std::string::npos, str.find("Hello")); + EXPECT_THAT(str, HasSubstr(__FUNCTION__)); + EXPECT_THAT(str, HasSubstr("Hello")); LogMessage::RemoveLogToStream(&stream); } @@ -186,8 +183,8 @@ TEST(LogTest, LogIfFDontLogIfConditionIsFalse) { LogMessage::AddLogToStream(&stream, LS_INFO); RTC_LOG_IF_F(LS_INFO, false) << "Not"; - EXPECT_EQ(std::string::npos, str.find(__FUNCTION__)); - EXPECT_EQ(std::string::npos, str.find("Not")); + EXPECT_THAT(str, Not(HasSubstr(__FUNCTION__))); + EXPECT_THAT(str, Not(HasSubstr("Not"))); LogMessage::RemoveLogToStream(&stream); } @@ -291,9 +288,9 @@ TEST(LogTest, CheckFilePathParsed) { #if defined(WEBRTC_ANDROID) EXPECT_NE(nullptr, strstr(tag, "myfile.cc")); - EXPECT_NE(std::string::npos, str.find("100")); + EXPECT_THAT(str, HasSubstr("100")); #else - EXPECT_NE(std::string::npos, str.find("(myfile.cc:100)")); + EXPECT_THAT(str, HasSubstr("(myfile.cc:100)")); #endif LogMessage::RemoveLogToStream(&stream); } @@ -306,8 +303,8 @@ TEST(LogTest, CheckTagAddedToStringInDefaultOnLogMessageAndroid) { EXPECT_EQ(LS_INFO, LogMessage::GetLogToStream(&stream)); RTC_LOG_TAG(LS_INFO, "my_tag") << "INFO"; - EXPECT_NE(std::string::npos, str.find("INFO")); - EXPECT_NE(std::string::npos, str.find("my_tag")); + EXPECT_THAT(str, HasSubstr("INFO")); + EXPECT_THAT(str, HasSubstr("my_tag")); } #endif @@ -348,10 +345,10 @@ TEST(LogTest, EnumsAreSupported) { LogSinkImpl stream(&str); LogMessage::AddLogToStream(&stream, LS_INFO); RTC_LOG(LS_INFO) << "[" << TestEnum::kValue0 << "]"; - EXPECT_NE(std::string::npos, str.find("[0]")); - EXPECT_EQ(std::string::npos, str.find("[1]")); + EXPECT_THAT(str, HasSubstr("[0]")); + EXPECT_THAT(str, Not(HasSubstr("[1]"))); RTC_LOG(LS_INFO) << "[" << TestEnum::kValue1 << "]"; - EXPECT_NE(std::string::npos, str.find("[1]")); + EXPECT_THAT(str, HasSubstr("[1]")); LogMessage::RemoveLogToStream(&stream); } @@ -389,5 +386,31 @@ TEST(LogTest, UseAbslStringForCustomTypes) { LogMessage::RemoveLogToStream(&stream); } +enum class TestEnumStringify { kValue0 = 0, kValue1 = 1 }; + +template +void AbslStringify(Sink& sink, TestEnumStringify value) { + switch (value) { + case TestEnumStringify::kValue0: + sink.Append("kValue0"); + break; + case TestEnumStringify::kValue1: + sink.Append("kValue1"); + break; + } +} + +TEST(LogTest, EnumSupportsAbslStringify) { + std::string str; + LogSinkImpl stream(&str); + LogMessage::AddLogToStream(&stream, LS_INFO); + RTC_LOG(LS_INFO) << "[" << TestEnumStringify::kValue0 << "]"; + EXPECT_THAT(str, HasSubstr("[kValue0]")); + EXPECT_THAT(str, Not(HasSubstr("[kValue1]"))); + RTC_LOG(LS_INFO) << "[" << TestEnumStringify::kValue1 << "]"; + EXPECT_THAT(str, HasSubstr("[kValue1]")); + LogMessage::RemoveLogToStream(&stream); +} + } // namespace rtc #endif // RTC_LOG_ENABLED()