Skip to content

Commit

Permalink
refactor logging: refactor logging tests
Browse files Browse the repository at this point in the history
- Remove code duplication between `LoggingTestBase` and `SocketLoggingTest`
- Make  `LoggedText` work correctly by returning the contents of the latest `text=`
- Remove sketchy sychronization via waiting on an `engine::ConditionVariable`
- Split `SocketLoggingTest` into a separate file to make `LoggingTest` independent from the engine and potentially movable to `shared`
  • Loading branch information
Anton3 committed Jan 25, 2023
1 parent e971629 commit 3061fcf
Show file tree
Hide file tree
Showing 8 changed files with 184 additions and 179 deletions.
4 changes: 2 additions & 2 deletions core/include/userver/logging/log.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ LoggerPtr SetDefaultLogger(LoggerPtr);
/// Sets new log level for default logger
void SetDefaultLoggerLevel(Level);

void SetLoggerLevel(LoggerPtr, Level);
void SetLoggerLevel(const LoggerPtr&, Level);

/// Returns log level for default logger
Level GetDefaultLoggerLevel();
Expand All @@ -41,7 +41,7 @@ Level GetLoggerLevel(const LoggerPtr& logger);
void LogFlush();

/// Forces flush of `logger` message queue
void LogFlush(LoggerPtr logger);
void LogFlush(const LoggerPtr& logger);

namespace impl {

Expand Down
20 changes: 12 additions & 8 deletions core/src/logging/dynamic_debug_test.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
#include <userver/utest/utest.hpp>

#include <gmock/gmock.h>

#include <logging/dynamic_debug.hpp>
#include <logging/logging_test.hpp>
#include <userver/logging/log.hpp>
Expand All @@ -12,11 +14,12 @@ TEST_F(LoggingTest, DynamicDebugBasic) {
LOG_INFO() << "before";

const std::string location = USERVER_FILEPATH;
logging::AddDynamicDebugLog(location, 17);
logging::AddDynamicDebugLog(location, 10001);

#line 10001
LOG_INFO() << "123";

logging::RemoveDynamicDebugLog(location, 17);
logging::RemoveDynamicDebugLog(location, 10001);

LOG_INFO() << "after";

Expand All @@ -42,10 +45,10 @@ TEST_F(LoggingTest, DynamicDebugAnyLine) {

LOG_INFO() << "after";

EXPECT_FALSE(LoggedTextContains("before"));
EXPECT_TRUE(LoggedTextContains("123"));
EXPECT_TRUE(LoggedTextContains("456"));
EXPECT_FALSE(LoggedTextContains("after"));
EXPECT_THAT(GetStreamString(), testing::Not(testing::HasSubstr("before")));
EXPECT_THAT(GetStreamString(), testing::HasSubstr("123"));
EXPECT_THAT(GetStreamString(), testing::HasSubstr("456"));
EXPECT_THAT(GetStreamString(), testing::Not(testing::HasSubstr("after")));

logging::SetDefaultLoggerLevel(logging::Level::kInfo);
}
Expand All @@ -56,10 +59,11 @@ TEST_F(LoggingTest, DynamicDebugAnyLineRemove) {
LOG_INFO() << "before";

const std::string location = USERVER_FILEPATH;
logging::AddDynamicDebugLog(location, 63);
logging::AddDynamicDebugLog(location, 64);
logging::AddDynamicDebugLog(location, 20001);
logging::AddDynamicDebugLog(location, 20002);
logging::RemoveDynamicDebugLog(location, logging::kAnyLine);

#line 20001
LOG_INFO() << "123";
LOG_INFO() << "456";

Expand Down
9 changes: 7 additions & 2 deletions core/src/logging/log.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,12 @@ void SetDefaultLoggerLevel(Level level) {
UpdateLogLevelCache();
}

void SetLoggerLevel(LoggerPtr logger, Level level) { logger->SetLevel(level); }
void SetLoggerLevel(const LoggerPtr& logger, Level level) {
logger->SetLevel(level);
if (logger == DefaultLoggerOptional()) {
UpdateLogLevelCache();
}
}

Level GetDefaultLoggerLevel() {
return static_cast<Level>(DefaultLogger()->GetLevel());
Expand All @@ -79,7 +84,7 @@ Level GetLoggerLevel(const LoggerPtr& logger) { return logger->GetLevel(); }

void LogFlush() { DefaultLogger()->Flush(); }

void LogFlush(LoggerPtr logger) { logger->Flush(); }
void LogFlush(const LoggerPtr& logger) { logger->Flush(); }

namespace impl {

Expand Down
10 changes: 6 additions & 4 deletions core/src/logging/log_message_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <unordered_map>
#include <unordered_set>

#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <boost/filesystem/path.hpp>

Expand Down Expand Up @@ -74,7 +75,8 @@ TEST_F(LoggingTest, TskvEncodeKeyWithDot) {
logging::LogExtra le;
le.Extend("http.port.ipv4", "4040");
LOG_CRITICAL() << "line 1\nline 2" << le;
EXPECT_EQ(LoggedText(), "line 1\\nline 2\thttp_port_ipv4=4040");
EXPECT_THAT(GetStreamString(),
testing::HasSubstr("line 1\\nline 2\thttp_port_ipv4=4040"));
}

TEST_F(LoggingTest, FloatingPoint) {
Expand Down Expand Up @@ -139,9 +141,9 @@ TEST_F(LoggingTest, TracefulExceptionDebug) {

LOG_CRITICAL() << utils::TracefulException("traceful exception");

EXPECT_TRUE(LoggedTextContains("traceful exception"))
EXPECT_THAT(GetStreamString(), testing::HasSubstr("traceful exception"))
<< "traceful exception is missing its message";
EXPECT_TRUE(LoggedTextContains("\tstacktrace="))
EXPECT_THAT(GetStreamString(), testing::HasSubstr("\tstacktrace="))
<< "traceful exception is missing its trace";
}

Expand Down Expand Up @@ -169,7 +171,7 @@ TEST_F(LoggingTest, AttachedException) {
<< "missing plain exception message";
EXPECT_TRUE(LoggedTextContains("plain exception with additional info"))
<< "traceful exception message malformed";
EXPECT_TRUE(LoggedTextContains("\tstacktrace="))
EXPECT_THAT(GetStreamString(), testing::HasSubstr("\tstacktrace="))
<< "traceful exception missing its trace";
}

Expand Down
25 changes: 4 additions & 21 deletions core/src/logging/log_test.cpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
#include <gtest/gtest.h>

#include <logging/logging_test.hpp>
#include <userver/engine/condition_variable.hpp>
#include <userver/engine/mutex.hpp>
#include <logging/socket_logging_test.hpp>
#include <userver/logging/log.hpp>
#include <userver/logging/log_helper_fwd.hpp>
#include <userver/utest/utest.hpp>
#include <userver/utils/async.hpp>

Expand Down Expand Up @@ -73,30 +70,16 @@ TEST_F(LoggingTest, DocsData) {
LOG_ERROR() << "This is unbelievable, fix me, please!";
/// [Sample logging usage]

bool flag = true;
const bool flag = true;
/// [Example set custom logging usage]
logging::Level level = flag ? logging::Level::kDebug : logging::Level::kInfo;
const auto level = flag ? logging::Level::kDebug : logging::Level::kInfo;
LOG(level) << "some text";
/// [Example set custom logging usage]
}

UTEST_F(SocketLoggingTest, Test) {
engine::Mutex mt;
engine::ConditionVariable cv;
auto task = utils::Async("server task", &SocketLoggingTest::Server, this,
std::ref(cv));

std::unique_lock lock(mt);
if (!cv.Wait(lock, [&]() { return IsStarts(); })) {
FAIL();
}

LOG_ERROR() << "test";
if (!cv.Wait(lock, [&]() { return IsRead(); })) {
FAIL();
}

EXPECT_EQ("test", LoggedText());
EXPECT_EQ("test", NextLoggedText());
}

USERVER_NAMESPACE_END
Loading

0 comments on commit 3061fcf

Please sign in to comment.