From: Adam Simpkins Date: Thu, 22 Jun 2017 00:23:21 +0000 (-0700) Subject: logging: fully convert the ERROR level to ERR X-Git-Tag: v2017.06.26.00~19 X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=53f2f752563152c9b10cfe36030aa871927f795a logging: fully convert the ERROR level to ERR Summary: Switch all code in the logging library from using `ERROR` to `ERR`, and remove the `ERROR` LogLevel entirely, even if it is not already defined as a macro. Previously the code kept `ERROR` available as a LogLevel name if it had not already been defined as a macro (which is done by common Windows header files). However, this made for inconsistent behavior, and made it easy to write code that would not be portable to Windows. This diff fully drops the `ERROR` name for consistency across platforms. Reviewed By: wez Differential Revision: D5288600 fbshipit-source-id: 8d2d52e955959c278345fc9c2086c7cacf9660f9 --- diff --git a/folly/experimental/logging/GlogStyleFormatter.cpp b/folly/experimental/logging/GlogStyleFormatter.cpp index 44b65747..2307fb68 100644 --- a/folly/experimental/logging/GlogStyleFormatter.cpp +++ b/folly/experimental/logging/GlogStyleFormatter.cpp @@ -29,7 +29,7 @@ StringPiece getGlogLevelName(LogLevel level) { return "VERBOSE"; } else if (level < LogLevel::WARN) { return "INFO"; - } else if (level < LogLevel::ERROR) { + } else if (level < LogLevel::ERR) { return "WARNING"; } else if (level < LogLevel::CRITICAL) { return "ERROR"; diff --git a/folly/experimental/logging/LogCategory.cpp b/folly/experimental/logging/LogCategory.cpp index ff06ba52..c794c748 100644 --- a/folly/experimental/logging/LogCategory.cpp +++ b/folly/experimental/logging/LogCategory.cpp @@ -28,8 +28,8 @@ namespace folly { LogCategory::LogCategory(LoggerDB* db) - : effectiveLevel_{LogLevel::ERROR}, - level_{static_cast(LogLevel::ERROR)}, + : effectiveLevel_{LogLevel::ERR}, + level_{static_cast(LogLevel::ERR)}, parent_{nullptr}, name_{}, db_{db} {} diff --git a/folly/experimental/logging/LogLevel.h b/folly/experimental/logging/LogLevel.h index 0c751b15..7ce01e66 100644 --- a/folly/experimental/logging/LogLevel.h +++ b/folly/experimental/logging/LogLevel.h @@ -62,9 +62,6 @@ enum class LogLevel : uint32_t { // other log libraries that also use ERROR as their log level name (e.g., // glog). ERR = 4000, -#ifndef ERROR - ERROR = 4000, -#endif CRITICAL = 5000, diff --git a/folly/experimental/logging/LoggerDB.cpp b/folly/experimental/logging/LoggerDB.cpp index bd92a637..79a1c068 100644 --- a/folly/experimental/logging/LoggerDB.cpp +++ b/folly/experimental/logging/LoggerDB.cpp @@ -64,14 +64,14 @@ LoggerDB* LoggerDB::get() { } LoggerDB::LoggerDB() { - // Create the root log category, and set the level to ERROR by default + // Create the root log category, and set the level to ERR by default auto rootUptr = std::make_unique(this); LogCategory* root = rootUptr.get(); auto ret = loggersByName_.wlock()->emplace(root->getName(), std::move(rootUptr)); DCHECK(ret.second); - root->setLevelLocked(LogLevel::ERROR, false); + root->setLevelLocked(LogLevel::ERR, false); } LoggerDB::LoggerDB(TestConstructorArg) : LoggerDB() {} diff --git a/folly/experimental/logging/test/GlogFormatterTest.cpp b/folly/experimental/logging/test/GlogFormatterTest.cpp index 9f0ad77d..067fcb10 100644 --- a/folly/experimental/logging/test/GlogFormatterTest.cpp +++ b/folly/experimental/logging/test/GlogFormatterTest.cpp @@ -149,7 +149,7 @@ TEST(GlogFormatter, unprintableChars) { tid); EXPECT_EQ( expected, - formatMsg(LogLevel::ERROR, "foo\abar\x1btest", "escapes.cpp", 97)); + formatMsg(LogLevel::ERR, "foo\abar\x1btest", "escapes.cpp", 97)); expected = folly::sformat( "I0417 13:45:56.123456 {:5d} escapes.cpp:98] foo\\\\bar\"test\n", tid); EXPECT_EQ( diff --git a/folly/experimental/logging/test/LogCategoryTest.cpp b/folly/experimental/logging/test/LogCategoryTest.cpp index 39b091ee..2509960b 100644 --- a/folly/experimental/logging/test/LogCategoryTest.cpp +++ b/folly/experimental/logging/test/LogCategoryTest.cpp @@ -31,11 +31,11 @@ TEST(LogCategory, effectiveLevel) { Logger foo2{&db, "..foo.."}; EXPECT_EQ(foo.getCategory(), foo2.getCategory()); - EXPECT_EQ(LogLevel::ERROR, db.getCategory("")->getLevel()); - EXPECT_EQ(LogLevel::ERROR, db.getCategory("")->getEffectiveLevel()); + EXPECT_EQ(LogLevel::ERR, db.getCategory("")->getLevel()); + EXPECT_EQ(LogLevel::ERR, db.getCategory("")->getEffectiveLevel()); EXPECT_EQ(LogLevel::MAX_LEVEL, db.getCategory("foo.bar")->getLevel()); - EXPECT_EQ(LogLevel::ERROR, db.getCategory("foo.bar")->getEffectiveLevel()); + EXPECT_EQ(LogLevel::ERR, db.getCategory("foo.bar")->getEffectiveLevel()); db.setLevel(".foo", LogLevel::WARN); EXPECT_EQ(LogLevel::MAX_LEVEL, db.getCategory("foo.bar")->getLevel()); @@ -58,7 +58,7 @@ TEST(LogCategory, effectiveLevel) { EXPECT_EQ(LogLevel::CRITICAL, noinherit->getEffectiveLevel()); // Modify the root logger's level - db.setLevel(".", LogLevel::ERROR); + db.setLevel(".", LogLevel::ERR); EXPECT_EQ(LogLevel::MAX_LEVEL, db.getCategory("foo.test.1234")->getLevel()); EXPECT_EQ( LogLevel::WARN, db.getCategory("foo.test.1234")->getEffectiveLevel()); @@ -73,8 +73,7 @@ TEST(LogCategory, effectiveLevel) { db.getCategory("foo.test.noinherit")->getEffectiveLevel()); EXPECT_EQ(LogLevel::MAX_LEVEL, db.getCategory("bar.foo.test")->getLevel()); - EXPECT_EQ( - LogLevel::ERROR, db.getCategory("bar.foo.test")->getEffectiveLevel()); + EXPECT_EQ(LogLevel::ERR, db.getCategory("bar.foo.test")->getEffectiveLevel()); } void testNumHandlers(size_t numHandlers) { @@ -139,7 +138,7 @@ void testNumHandlers(size_t numHandlers) { // Log a message to a sibling of foobar Logger siblingLogger{&db, "foo.sibling"}; - FB_LOG(siblingLogger, ERROR, "oh noes"); + FB_LOG(siblingLogger, ERR, "oh noes"); for (const auto& handler : handlers) { auto& messages = handler->getMessages(); EXPECT_EQ(2, messages.size()); @@ -148,7 +147,7 @@ void testNumHandlers(size_t numHandlers) { auto& messages = rootHandler->getMessages(); ASSERT_EQ(3, messages.size()); EXPECT_EQ("oh noes", messages[2].first.getMessage()); - EXPECT_EQ(LogLevel::ERROR, messages[2].first.getLevel()); + EXPECT_EQ(LogLevel::ERR, messages[2].first.getLevel()); EXPECT_EQ(siblingLogger.getCategory(), messages[2].first.getCategory()); EXPECT_EQ(rootCategory, messages[2].second); } diff --git a/folly/experimental/logging/test/LogLevelTest.cpp b/folly/experimental/logging/test/LogLevelTest.cpp index d580f61c..dc9ec69d 100644 --- a/folly/experimental/logging/test/LogLevelTest.cpp +++ b/folly/experimental/logging/test/LogLevelTest.cpp @@ -46,6 +46,7 @@ TEST(LogLevel, fromString) { EXPECT_EQ(LogLevel::ERR, stringToLogLevel("err")); EXPECT_EQ(LogLevel::ERR, stringToLogLevel("eRr")); EXPECT_EQ(LogLevel::ERR, stringToLogLevel("error")); + EXPECT_EQ(LogLevel::ERR, stringToLogLevel("ERR")); EXPECT_EQ(LogLevel::ERR, stringToLogLevel("ERROR")); EXPECT_EQ(LogLevel::CRITICAL, stringToLogLevel("critical")); diff --git a/folly/experimental/logging/test/LogMessageTest.cpp b/folly/experimental/logging/test/LogMessageTest.cpp index 8d489568..3bd83f79 100644 --- a/folly/experimental/logging/test/LogMessageTest.cpp +++ b/folly/experimental/logging/test/LogMessageTest.cpp @@ -26,7 +26,7 @@ using namespace folly; SCOPED_TRACE( \ "input string: \"" + folly::backslashify(value) + "\""); \ LogMessage checkMsg{ \ - category, LogLevel::ERROR, __FILE__, __LINE__, std::string{value}}; \ + category, LogLevel::ERR, __FILE__, __LINE__, std::string{value}}; \ EXPECT_EQ(expected, checkMsg.getMessage()); \ EXPECT_EQ(hasNewlines, checkMsg.containsNewlines()); \ EXPECT_EQ(__FILE__, checkMsg.getFileName()); \ diff --git a/folly/experimental/logging/test/LoggerDBTest.cpp b/folly/experimental/logging/test/LoggerDBTest.cpp index d7fbf543..ee8e4ff6 100644 --- a/folly/experimental/logging/test/LoggerDBTest.cpp +++ b/folly/experimental/logging/test/LoggerDBTest.cpp @@ -80,9 +80,9 @@ TEST(LoggerDB, processConfigString) { EXPECT_EQ(LogLevel::DBG5, db.getCategory("foo.bar")->getLevel()); EXPECT_EQ(LogLevel::DBG5, db.getCategory("foo.bar")->getEffectiveLevel()); EXPECT_EQ(LogLevel::MAX_LEVEL, db.getCategory("foo")->getLevel()); - EXPECT_EQ(LogLevel::ERROR, db.getCategory("foo")->getEffectiveLevel()); - EXPECT_EQ(LogLevel::ERROR, db.getCategory("")->getLevel()); - EXPECT_EQ(LogLevel::ERROR, db.getCategory("")->getEffectiveLevel()); + EXPECT_EQ(LogLevel::ERR, db.getCategory("foo")->getEffectiveLevel()); + EXPECT_EQ(LogLevel::ERR, db.getCategory("")->getLevel()); + EXPECT_EQ(LogLevel::ERR, db.getCategory("")->getEffectiveLevel()); EXPECT_EQ(LogLevel::MAX_LEVEL, db.getCategory("foo.bar.test")->getLevel()); EXPECT_EQ( diff --git a/folly/experimental/logging/test/LoggerTest.cpp b/folly/experimental/logging/test/LoggerTest.cpp index 0afedd9b..a2b285c4 100644 --- a/folly/experimental/logging/test/LoggerTest.cpp +++ b/folly/experimental/logging/test/LoggerTest.cpp @@ -67,14 +67,14 @@ TEST_F(LoggerTest, subCategory) { // Log from a sub-category. Logger subLogger{&db_, "test.foo.bar"}; auto expectedLine = __LINE__ + 1; - FB_LOG(subLogger, ERROR, "sub-category\nlog message"); + FB_LOG(subLogger, ERR, "sub-category\nlog message"); auto& messages = handler_->getMessages(); ASSERT_EQ(1, messages.size()); EXPECT_EQ("sub-category\nlog message", messages[0].first.getMessage()); EXPECT_EQ("LoggerTest.cpp", pathBasename(messages[0].first.getFileName())); EXPECT_EQ(expectedLine, messages[0].first.getLineNumber()); - EXPECT_EQ(LogLevel::ERROR, messages[0].first.getLevel()); + EXPECT_EQ(LogLevel::ERR, messages[0].first.getLevel()); EXPECT_TRUE(messages[0].first.containsNewlines()); EXPECT_EQ(subLogger.getCategory(), messages[0].first.getCategory()); EXPECT_EQ(logger_.getCategory(), messages[0].second); @@ -294,13 +294,13 @@ TEST_F(LoggerTest, logMacros) { Logger footest{&db_, "test.foo.test"}; Logger footest1234{&db_, "test.foo.test.1234"}; Logger other{&db_, "test.other"}; - db_.setLevel("test", LogLevel::ERROR); + db_.setLevel("test", LogLevel::ERR); db_.setLevel("test.foo", LogLevel::DBG2); db_.setLevel("test.foo.test", LogLevel::DBG7); auto& messages = handler_->getMessages(); - // test.other's effective level should be ERROR, so a warning + // test.other's effective level should be ERR, so a warning // message to it should be discarded FB_LOG(other, WARN, "this should be discarded"); ASSERT_EQ(0, messages.size()); @@ -332,7 +332,7 @@ TEST_F(LoggerTest, logMacros) { messages.clear(); // Bad format arguments should not throw - FB_LOGF(footest1234, ERROR, "whoops: {}, {}", getValue()); + FB_LOGF(footest1234, ERR, "whoops: {}, {}", getValue()); ASSERT_EQ(1, messages.size()); EXPECT_EQ( "error formatting log message: " diff --git a/folly/experimental/logging/test/PrintfTest.cpp b/folly/experimental/logging/test/PrintfTest.cpp index 02f04bce..39b6819a 100644 --- a/folly/experimental/logging/test/PrintfTest.cpp +++ b/folly/experimental/logging/test/PrintfTest.cpp @@ -34,13 +34,13 @@ TEST(PrintfTest, printfStyleMacros) { Logger footest{&db, "test.foo.test"}; Logger footest1234{&db, "test.foo.test.1234"}; Logger other{&db, "test.other"}; - db.setLevel("test", LogLevel::ERROR); + db.setLevel("test", LogLevel::ERR); db.setLevel("test.foo", LogLevel::DBG2); db.setLevel("test.foo.test", LogLevel::DBG7); auto& messages = handler->getMessages(); - // test.other's effective level should be ERROR, so a warning + // test.other's effective level should be ERR, so a warning // message to it should be discarded FB_LOGC(other, WARN, "this should be discarded: %d", 5); ASSERT_EQ(0, messages.size()); @@ -91,7 +91,7 @@ TEST(PrintfTest, printfStyleMacros) { messages.clear(); // Errors attempting to format the message should not throw - FB_LOGC(footest1234, ERROR, "width overflow: %999999999999999999999d", 5); + FB_LOGC(footest1234, ERR, "width overflow: %999999999999999999999d", 5); ASSERT_EQ(1, messages.size()); EXPECT_EQ( "error formatting printf-style log message: "