logging: fully convert the ERROR level to ERR
authorAdam Simpkins <simpkins@fb.com>
Thu, 22 Jun 2017 00:23:21 +0000 (17:23 -0700)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Thu, 22 Jun 2017 00:35:56 +0000 (17:35 -0700)
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

folly/experimental/logging/GlogStyleFormatter.cpp
folly/experimental/logging/LogCategory.cpp
folly/experimental/logging/LogLevel.h
folly/experimental/logging/LoggerDB.cpp
folly/experimental/logging/test/GlogFormatterTest.cpp
folly/experimental/logging/test/LogCategoryTest.cpp
folly/experimental/logging/test/LogLevelTest.cpp
folly/experimental/logging/test/LogMessageTest.cpp
folly/experimental/logging/test/LoggerDBTest.cpp
folly/experimental/logging/test/LoggerTest.cpp
folly/experimental/logging/test/PrintfTest.cpp

index 44b6574..2307fb6 100644 (file)
@@ -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";
index ff06ba5..c794c74 100644 (file)
@@ -28,8 +28,8 @@
 namespace folly {
 
 LogCategory::LogCategory(LoggerDB* db)
-    : effectiveLevel_{LogLevel::ERROR},
-      level_{static_cast<uint32_t>(LogLevel::ERROR)},
+    : effectiveLevel_{LogLevel::ERR},
+      level_{static_cast<uint32_t>(LogLevel::ERR)},
       parent_{nullptr},
       name_{},
       db_{db} {}
index 0c751b1..7ce01e6 100644 (file)
@@ -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,
 
index bd92a63..79a1c06 100644 (file)
@@ -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<LogCategory>(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() {}
index 9f0ad77..067fcb1 100644 (file)
@@ -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(
index 39b091e..2509960 100644 (file)
@@ -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);
   }
index d580f61..dc9ec69 100644 (file)
@@ -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"));
index 8d48956..3bd83f7 100644 (file)
@@ -26,7 +26,7 @@ using namespace folly;
     SCOPED_TRACE(                                                             \
         "input string: \"" + folly::backslashify<std::string>(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());                              \
index d7fbf54..ee8e4ff 100644 (file)
@@ -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(
index 0afedd9..a2b285c 100644 (file)
@@ -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: "
index 02f04bc..39b6819 100644 (file)
@@ -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: "