From a047409ae919ea6cfffa3d87d57175378bfbf3f6 Mon Sep 17 00:00:00 2001 From: krad Date: Mon, 23 Feb 2015 11:59:39 -0800 Subject: [PATCH] Fixed a bug in the test case Summary: The unit test was supposed to check that the old file and the new file contains the header message. Test Plan: Run the unit test. Reviewers: sdong Reviewed By: sdong Subscribers: dhruba Differential Revision: https://reviews.facebook.net/D33705 --- util/auto_roll_logger.h | 5 +++ util/auto_roll_logger_test.cc | 73 +++++++++++++++++++++++++++-------- 2 files changed, 62 insertions(+), 16 deletions(-) mode change 100755 => 100644 util/auto_roll_logger_test.cc diff --git a/util/auto_roll_logger.h b/util/auto_roll_logger.h index ff995f0e0..b55b07bd1 100644 --- a/util/auto_roll_logger.h +++ b/util/auto_roll_logger.h @@ -69,6 +69,11 @@ class AutoRollLogger : public Logger { call_NowMicros_every_N_records_ = call_NowMicros_every_N_records; } + // Expose the log file path for testing purpose + std::string TEST_log_fname() const { + return log_fname_; + } + private: bool LogExpired(); Status ResetLogger(); diff --git a/util/auto_roll_logger_test.cc b/util/auto_roll_logger_test.cc old mode 100755 new mode 100644 index 117ecd46b..6d2cb7d55 --- a/util/auto_roll_logger_test.cc +++ b/util/auto_roll_logger_test.cc @@ -4,6 +4,7 @@ // of patent rights can be found in the PATENTS file in the same directory. // #include +#include #include #include #include @@ -287,6 +288,43 @@ TEST(AutoRollLoggerTest, InfoLogLevel) { // Test the logger Header function for roll over logs // We expect the new logs creates as roll over to carry the headers specified +static list GetOldFileNames(const string& path) { + const string dirname = path.substr(/*start=*/ 0, path.find_last_of("/")); + const string fname = path.substr(path.find_last_of("/") + 1); + + vector children; + Env::Default()->GetChildren(dirname, &children); + + // We know that the old log files are named [path] + // Return all entities that match the pattern + list ret; + for (auto child : children) { + if (fname != child && child.find(fname) == 0) { + ret.push_back(dirname + "/" + child); + } + } + + return ret; +} + +// Return the number of lines where a given pattern was found in the file +static size_t GetLinesCount(const string& fname, const string& pattern) { + stringstream ssbuf; + string line; + size_t count = 0; + + ifstream inFile(fname.c_str()); + ssbuf << inFile.rdbuf(); + + while (getline(ssbuf, line)) { + if (line.find(pattern) != std::string::npos) { + count++; + } + } + + return count; +} + TEST(AutoRollLoggerTest, LogHeaderTest) { static const size_t MAX_HEADERS = 10; static const size_t LOG_MAX_SIZE = 1024 * 5; @@ -302,29 +340,32 @@ TEST(AutoRollLoggerTest, LogHeaderTest) { Header(&logger, "%s %d", HEADER_STR.c_str(), i); } + const string& newfname = logger.TEST_log_fname().c_str(); + // log enough data to cause a roll over - size_t i = 0; - while (logger.GetLogFileSize() < LOG_MAX_SIZE) { - Info(&logger, (kSampleMessage + ":LogHeaderTest line %d").c_str(), i); - ++i; + int i = 0; + for (size_t iter = 0; iter < 2; iter++) { + while (logger.GetLogFileSize() < LOG_MAX_SIZE) { + Info(&logger, (kSampleMessage + ":LogHeaderTest line %d").c_str(), i); + ++i; + } + + Info(&logger, "Rollover"); } + + // Flus the log for the latest file LogFlush(&logger); - // verify that the new log contains all the header logs - std::stringstream ssbuf; - std::string line; - size_t count = 0; + const list oldfiles = GetOldFileNames(newfname); - std::ifstream inFile(AutoRollLoggerTest::kLogFile.c_str()); - ssbuf << inFile.rdbuf(); + ASSERT_EQ(oldfiles.size(), (size_t) 2); - while (getline(ssbuf, line)) { - if (line.find(HEADER_STR) != std::string::npos) { - count++; - } + for (auto oldfname : oldfiles) { + // verify that the files rolled over + ASSERT_NE(oldfname, newfname); + // verify that the old log contains all the header logs + ASSERT_EQ(GetLinesCount(oldfname, HEADER_STR), MAX_HEADERS); } - - ASSERT_EQ(count, MAX_HEADERS); } TEST(AutoRollLoggerTest, LogFileExistence) {