From c06cbcc4ba15f685810796d56a3a3cf24f7761c8 Mon Sep 17 00:00:00 2001 From: NoRePercussions Date: Wed, 8 Feb 2023 19:06:05 -0500 Subject: [PATCH 1/4] Fix: #8 Make LoggerTests work cross-platform The previous tests used a technically write-only stream in memory, which causes the tests to fail on some systems. Instead, we can use an `fmemopen` stream to ensure it is always readable. --- test/loggerTests.cpp | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test/loggerTests.cpp b/test/loggerTests.cpp index 66811e6dc..d76e82c36 100644 --- a/test/loggerTests.cpp +++ b/test/loggerTests.cpp @@ -12,7 +12,10 @@ using namespace okapi; class LoggerTest : public ::testing::Test { protected: virtual void SetUp() { - logFile = open_memstream(&logBuffer, &logSize); + logBuffer = new char[logSize]; + // For testing, we always write, then rewind and read + // so it is not an issue to open in write vs append + logFile = fmemopen(logBuffer, logSize, "w+"); } virtual void TearDown() { @@ -20,7 +23,6 @@ class LoggerTest : public ::testing::Test { if (logger) { logger->close(); } - free(logBuffer); } void logData(const std::shared_ptr &) const { @@ -32,7 +34,7 @@ class LoggerTest : public ::testing::Test { FILE *logFile; char *logBuffer; - size_t logSize; + size_t logSize = 10000; std::shared_ptr logger; }; @@ -41,6 +43,8 @@ TEST_F(LoggerTest, OffLevel) { std::make_unique(0_ms), logFile, Logger::LogLevel::off); logData(logger); + fputs("EMPTY_FILE", logFile); + rewind(logFile); char *line = nullptr; size_t len; @@ -60,6 +64,7 @@ TEST_F(LoggerTest, ErrorLevel) { std::make_unique(0_ms), logFile, Logger::LogLevel::error); logData(logger); + rewind(logFile); char *line = nullptr; size_t len; @@ -78,6 +83,7 @@ TEST_F(LoggerTest, WarningLevel) { std::make_unique(0_ms), logFile, Logger::LogLevel::warn); logData(logger); + rewind(logFile); char *line = nullptr; size_t len; @@ -100,6 +106,7 @@ TEST_F(LoggerTest, InfoLevel) { std::make_unique(0_ms), logFile, Logger::LogLevel::info); logData(logger); + rewind(logFile); char *line = nullptr; size_t len; @@ -126,6 +133,7 @@ TEST_F(LoggerTest, DebugLevel) { std::make_unique(0_ms), logFile, Logger::LogLevel::debug); logData(logger); + rewind(logFile); char *line = nullptr; size_t len; @@ -152,6 +160,7 @@ TEST_F(LoggerTest, DebugLevel) { } TEST_F(LoggerTest, TestLazyLogging) { + rewind(logFile); logger = std::make_shared( std::make_unique(0_ms), logFile, Logger::LogLevel::info); From 271a9da128c8e1e9dd545f498cf79aec97cb61ea Mon Sep 17 00:00:00 2001 From: NoRePercussions Date: Wed, 8 Feb 2023 19:34:24 -0500 Subject: [PATCH 2/4] Fix: Add checks that logger files don't have more contents If each logger test writes more levels than requested as-is, the tests would not catch it due to the logging order. This way, we can assert that it does not write more. --- test/loggerTests.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/test/loggerTests.cpp b/test/loggerTests.cpp index d76e82c36..9fb71218b 100644 --- a/test/loggerTests.cpp +++ b/test/loggerTests.cpp @@ -43,16 +43,13 @@ TEST_F(LoggerTest, OffLevel) { std::make_unique(0_ms), logFile, Logger::LogLevel::off); logData(logger); - fputs("EMPTY_FILE", logFile); rewind(logFile); char *line = nullptr; size_t len; - fputs("EMPTY_FILE", logFile); - - getline(&line, &len, logFile); - EXPECT_STREQ(line, "EMPTY_FILE"); + // Check that we are done reading + EXPECT_EQ(getline(&line, &len, logFile), EOF); if (line) { free(line); @@ -73,6 +70,8 @@ TEST_F(LoggerTest, ErrorLevel) { std::string expected = "0 (" + CrossplatformThread::getName() + ") ERROR: MSG\n"; EXPECT_STREQ(line, expected.c_str()); + EXPECT_EQ(getline(&line, &len, logFile), EOF); + if (line) { free(line); } @@ -96,6 +95,8 @@ TEST_F(LoggerTest, WarningLevel) { expected = "0 (" + CrossplatformThread::getName() + ") WARN: MSG\n"; EXPECT_STREQ(line, expected.c_str()); + EXPECT_EQ(getline(&line, &len, logFile), EOF); + if (line) { free(line); } @@ -123,6 +124,8 @@ TEST_F(LoggerTest, InfoLevel) { expected = "0 (" + CrossplatformThread::getName() + ") INFO: MSG\n"; EXPECT_STREQ(line, expected.c_str()); + EXPECT_EQ(getline(&line, &len, logFile), EOF); + if (line) { free(line); } @@ -154,6 +157,8 @@ TEST_F(LoggerTest, DebugLevel) { expected = "0 (" + CrossplatformThread::getName() + ") DEBUG: MSG\n"; EXPECT_STREQ(line, expected.c_str()); + EXPECT_EQ(getline(&line, &len, logFile), EOF); + if (line) { free(line); } From 2485e223393b940a3892a361123e28030d00b6e8 Mon Sep 17 00:00:00 2001 From: NoRePercussions Date: Wed, 8 Feb 2023 19:36:24 -0500 Subject: [PATCH 3/4] Fix: Make TestLazyLogging actually check logging In its previous form, TestLazyLogging checked that logging didn't error and that the lambda used was scoped. This version will check that it logs the contents given and no more. --- test/loggerTests.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/test/loggerTests.cpp b/test/loggerTests.cpp index 9fb71218b..2626200ea 100644 --- a/test/loggerTests.cpp +++ b/test/loggerTests.cpp @@ -165,22 +165,23 @@ TEST_F(LoggerTest, DebugLevel) { } TEST_F(LoggerTest, TestLazyLogging) { - rewind(logFile); logger = std::make_shared( - std::make_unique(0_ms), logFile, Logger::LogLevel::info); + std::make_unique(0_ms), logFile, Logger::LogLevel::debug); - int x = 0; - logger->debug([=, &x]() { - x++; - return std::string(""); + logger->debug([=]() { + return std::string("MSG"); }); - EXPECT_EQ(x, 0); + rewind(logFile); char *line = nullptr; size_t len; getline(&line, &len, logFile); + std::string expected = "0 (" + CrossplatformThread::getName() + ") DEBUG: MSG\n"; + EXPECT_STREQ(line, expected.c_str()); + + EXPECT_EQ(getline(&line, &len, logFile), EOF); if (line) { free(line); From 2c85254b8e8e34768d73036f7c56432504618a01 Mon Sep 17 00:00:00 2001 From: NoRePercussions Date: Wed, 8 Feb 2023 20:05:24 -0500 Subject: [PATCH 4/4] Fix leaked memory in tests --- test/loggerTests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/loggerTests.cpp b/test/loggerTests.cpp index 2626200ea..8f01a1b1d 100644 --- a/test/loggerTests.cpp +++ b/test/loggerTests.cpp @@ -23,6 +23,7 @@ class LoggerTest : public ::testing::Test { if (logger) { logger->close(); } + delete[] logBuffer; } void logData(const std::shared_ptr &) const {