From 8fe3457dbb4146952b92fb9509bbe4e97dc1f05b Mon Sep 17 00:00:00 2001 From: klementtan Date: Thu, 18 Aug 2022 13:37:25 +0200 Subject: [PATCH] Update LogAcceptCategory() and unit tests with log severity levels Co-authored-by: "Jon Atack " --- src/logging.cpp | 13 +++++++++ src/logging.h | 9 +++---- src/test/i2p_tests.cpp | 4 +++ src/test/logging_tests.cpp | 30 +++++++++++++-------- test/functional/test_framework/test_node.py | 2 ++ 5 files changed, 41 insertions(+), 17 deletions(-) diff --git a/src/logging.cpp b/src/logging.cpp index 38162978204..522b4dc42e7 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -124,6 +124,19 @@ bool BCLog::Logger::WillLogCategory(BCLog::LogFlags category) const return (m_categories.load(std::memory_order_relaxed) & category) != 0; } +bool BCLog::Logger::WillLogCategoryLevel(BCLog::LogFlags category, BCLog::Level level) const +{ + // Log messages at Warning and Error level unconditionally, so that + // important troubleshooting information doesn't get lost. + if (level >= BCLog::Level::Warning) return true; + + if (!WillLogCategory(category)) return false; + + StdLockGuard scoped_lock(m_cs); + const auto it{m_category_log_levels.find(category)}; + return level >= (it == m_category_log_levels.end() ? LogLevel() : it->second); +} + bool BCLog::Logger::DefaultShrinkDebugFile() const { return m_categories == BCLog::NONE; diff --git a/src/logging.h b/src/logging.h index f49a2db326a..76bed39e918 100644 --- a/src/logging.h +++ b/src/logging.h @@ -176,6 +176,8 @@ namespace BCLog { bool DisableCategory(const std::string& str); bool WillLogCategory(LogFlags category) const; + bool WillLogCategoryLevel(LogFlags category, Level level) const; + /** Returns a vector of the log categories in alphabetical order. */ std::vector LogCategoriesList() const; /** Returns a string with the log categories in alphabetical order. */ @@ -194,12 +196,7 @@ BCLog::Logger& LogInstance(); /** Return true if log accepts specified category, at the specified level. */ static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level level) { - // Log messages at Warning and Error level unconditionally, so that - // important troubleshooting information doesn't get lost. - if (level >= BCLog::Level::Warning) { - return true; - } - return LogInstance().WillLogCategory(category); + return LogInstance().WillLogCategoryLevel(category, level); } /** Return true if str parses as a log category and set the flag */ diff --git a/src/test/i2p_tests.cpp b/src/test/i2p_tests.cpp index bd9ba4b8f74..6fb104622d6 100644 --- a/src/test/i2p_tests.cpp +++ b/src/test/i2p_tests.cpp @@ -3,6 +3,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -19,6 +20,8 @@ BOOST_FIXTURE_TEST_SUITE(i2p_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(unlimited_recv) { + const auto prev_log_level{LogInstance().LogLevel()}; + LogInstance().SetLogLevel(BCLog::Level::Debug); auto CreateSockOrig = CreateSock; // Mock CreateSock() to create MockSock. @@ -39,6 +42,7 @@ BOOST_AUTO_TEST_CASE(unlimited_recv) } CreateSock = CreateSockOrig; + LogInstance().SetLogLevel(prev_log_level); } BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index 5a5e3b3f1f0..00c6107c5a0 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -25,6 +26,8 @@ struct LogSetup : public BasicTestingSetup { bool prev_log_timestamps; bool prev_log_threadnames; bool prev_log_sourcelocations; + std::unordered_map prev_category_levels; + BCLog::Level prev_log_level; LogSetup() : prev_log_path{LogInstance().m_file_path}, tmp_log_path{m_args.GetDataDirBase() / "tmp_debug.log"}, @@ -32,14 +35,21 @@ struct LogSetup : public BasicTestingSetup { prev_print_to_file{LogInstance().m_print_to_file}, prev_log_timestamps{LogInstance().m_log_timestamps}, prev_log_threadnames{LogInstance().m_log_threadnames}, - prev_log_sourcelocations{LogInstance().m_log_sourcelocations} + prev_log_sourcelocations{LogInstance().m_log_sourcelocations}, + prev_category_levels{LogInstance().CategoryLevels()}, + prev_log_level{LogInstance().LogLevel()} { LogInstance().m_file_path = tmp_log_path; LogInstance().m_reopen_file = true; LogInstance().m_print_to_file = true; LogInstance().m_log_timestamps = false; LogInstance().m_log_threadnames = false; - LogInstance().m_log_sourcelocations = true; + + // Prevent tests from failing when the line number of the logs changes. + LogInstance().m_log_sourcelocations = false; + + LogInstance().SetLogLevel(BCLog::Level::Debug); + LogInstance().SetCategoryLogLevel({}); } ~LogSetup() @@ -51,6 +61,8 @@ struct LogSetup : public BasicTestingSetup { LogInstance().m_log_timestamps = prev_log_timestamps; LogInstance().m_log_threadnames = prev_log_threadnames; LogInstance().m_log_sourcelocations = prev_log_sourcelocations; + LogInstance().SetLogLevel(prev_log_level); + LogInstance().SetCategoryLogLevel(prev_category_levels); } }; @@ -74,6 +86,7 @@ BOOST_AUTO_TEST_CASE(logging_timer) BOOST_FIXTURE_TEST_CASE(logging_LogPrintf_, LogSetup) { + LogInstance().m_log_sourcelocations = true; LogPrintf_("fn1", "src1", 1, BCLog::LogFlags::NET, BCLog::Level::Debug, "foo1: %s", "bar1\n"); LogPrintf_("fn2", "src2", 2, BCLog::LogFlags::NET, BCLog::Level::None, "foo2: %s", "bar2\n"); LogPrintf_("fn3", "src3", 3, BCLog::LogFlags::NONE, BCLog::Level::Debug, "foo3: %s", "bar3\n"); @@ -94,9 +107,6 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintf_, LogSetup) BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros, LogSetup) { - // Prevent tests from failing when the line number of the following log calls changes. - LogInstance().m_log_sourcelocations = false; - LogPrintf("foo5: %s\n", "bar5"); LogPrint(BCLog::NET, "foo6: %s\n", "bar6"); LogPrintLevel(BCLog::NET, BCLog::Level::Debug, "foo7: %s\n", "bar7"); @@ -123,16 +133,14 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros, LogSetup) BOOST_FIXTURE_TEST_CASE(logging_LogPrintMacros_CategoryName, LogSetup) { - // Prevent tests from failing when the line number of the following log calls changes. - LogInstance().m_log_sourcelocations = false; LogInstance().EnableCategory(BCLog::LogFlags::ALL); - const auto concated_categery_names = LogInstance().LogCategoriesString(); + const auto concatenated_category_names = LogInstance().LogCategoriesString(); std::vector> expected_category_names; - const auto category_names = SplitString(concated_categery_names, ','); + const auto category_names = SplitString(concatenated_category_names, ','); for (const auto& category_name : category_names) { - BCLog::LogFlags category = BCLog::NONE; + BCLog::LogFlags category; const auto trimmed_category_name = TrimString(category_name); - BOOST_TEST(GetLogCategory(category, trimmed_category_name)); + BOOST_REQUIRE(GetLogCategory(category, trimmed_category_name)); expected_category_names.emplace_back(category, trimmed_category_name); } diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 03f6c8adea6..e28d458f69a 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -118,6 +118,8 @@ class TestNode(): self.args.append("-logthreadnames") if self.version_is_at_least(219900): self.args.append("-logsourcelocations") + if self.version_is_at_least(239000): + self.args.append("-loglevel=debug") self.cli = TestNodeCLI(bitcoin_cli, self.datadir) self.use_cli = use_cli