Merge pull request #732 from google/log-cleaner-tests

added log cleaner tests and fixed paths
2021-10-29 22:58:38 +02:00 · 2021-10-29 22:58:38 +02:00 · e8e40f76db
commit e8e40f76db
parent 9cf0eb7c98 3362cc6c79
6 changed files with 320 additions and 28 deletions
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@ -921,6 +921,44 @@ if (BUILD_TESTING)
    FIXTURES_REQUIRED cmake_package_config)
  set_tests_properties (cmake_package_config_build PROPERTIES
    FIXTURES_REQUIRED "cmake_package_config;cmake_package_config_working")
+
+  add_executable (cleanup_immediately_unittest
+    src/cleanup_immediately_unittest.cc)
+
+  target_link_libraries (cleanup_immediately_unittest PRIVATE ${_GLOG_TEST_LIBS})
+
+  add_executable (cleanup_with_prefix_unittest
+    src/cleanup_with_prefix_unittest.cc)
+
+  target_link_libraries (cleanup_with_prefix_unittest PRIVATE ${_GLOG_TEST_LIBS})
+
+  set (CLEANUP_LOG_DIR ${CMAKE_CURRENT_BINARY_DIR}/cleanup_tests)
+
+  add_test (NAME cleanup_init COMMAND
+    ${CMAKE_COMMAND} -E make_directory ${CLEANUP_LOG_DIR})
+  add_test (NAME cleanup_logdir COMMAND
+    ${CMAKE_COMMAND} -E remove_directory ${CLEANUP_LOG_DIR})
+  add_test (NAME cleanup_immediately COMMAND
+    ${CMAKE_COMMAND}
+    -DLOGCLEANUP=$<TARGET_FILE:cleanup_immediately_unittest>
+    # NOTE The trailing slash is important
+    -DTEST_DIR=${CLEANUP_LOG_DIR}/
+    -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/RunCleanerTest1.cmake
+    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
+  add_test (NAME cleanup_with_prefix COMMAND
+    ${CMAKE_COMMAND}
+    -DLOGCLEANUP=$<TARGET_FILE:cleanup_with_prefix_unittest>
+    -DTEST_DIR=${CMAKE_CURRENT_BINARY_DIR}/
+    -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/RunCleanerTest2.cmake
+    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
+
+  # Fixtures setup
+  set_tests_properties (cleanup_init PROPERTIES FIXTURES_SETUP logcleanuptest)
+  ## Fixtures cleanup
+  set_tests_properties (cleanup_logdir PROPERTIES FIXTURES_CLEANUP logcleanuptest)
+  # Fixture requirements
+  set_tests_properties (cleanup_immediately PROPERTIES FIXTURES_REQUIRED logcleanuptest)
+  set_tests_properties (cleanup_with_prefix PROPERTIES FIXTURES_REQUIRED logcleanuptest)
 endif (BUILD_TESTING)

 install (TARGETS glog
--- a/cmake/RunCleanerTest1.cmake
+++ b/cmake/RunCleanerTest1.cmake
@ -0,0 +1,22 @@
+set (RUNS 3)
+
+foreach (iter RANGE 1 ${RUNS})
+  set (ENV{GOOGLE_LOG_DIR} ${TEST_DIR})
+  execute_process (COMMAND ${LOGCLEANUP} RESULT_VARIABLE _RESULT)
+
+  if (NOT _RESULT EQUAL 0)
+    message (FATAL_ERROR "Failed to run logcleanup_unittest (error: ${_RESULT})")
+  endif (NOT _RESULT EQUAL 0)
+
+  # Ensure the log files to have different modification timestamps such that
+  # exactly one log file remains at the end. Otherwise all log files will be
+  # retained.
+  execute_process (COMMAND ${CMAKE_COMMAND} -E sleep 1)
+endforeach (iter)
+
+file (GLOB LOG_FILES ${TEST_DIR}/*.foobar)
+list (LENGTH LOG_FILES NUM_FILES)
+
+if (NOT NUM_FILES EQUAL 1)
+  message (SEND_ERROR "Expected 1 log file in log directory but found ${NUM_FILES}")
+endif (NOT NUM_FILES EQUAL 1)
--- a/cmake/RunCleanerTest2.cmake
+++ b/cmake/RunCleanerTest2.cmake
@ -0,0 +1,22 @@
+set (RUNS 3)
+
+foreach (iter RANGE 1 ${RUNS})
+  execute_process (COMMAND ${LOGCLEANUP} -log_dir=${TEST_DIR}
+    RESULT_VARIABLE _RESULT)
+
+  if (NOT _RESULT EQUAL 0)
+    message (FATAL_ERROR "Failed to run logcleanup_unittest (error: ${_RESULT})")
+  endif (NOT _RESULT EQUAL 0)
+
+  # Ensure the log files to have different modification timestamps such that
+  # exactly one log file remains at the end. Otherwise all log files will be
+  # retained.
+  execute_process (COMMAND ${CMAKE_COMMAND} -E sleep 1)
+endforeach (iter)
+
+file (GLOB LOG_FILES ${TEST_DIR}/test_cleanup_*.barfoo)
+list (LENGTH LOG_FILES NUM_FILES)
+
+if (NOT NUM_FILES EQUAL 1)
+  message (SEND_ERROR "Expected 1 log file in build directory ${TEST_DIR} but found ${NUM_FILES}")
+endif (NOT NUM_FILES EQUAL 1)
--- a/src/cleanup_immediately_unittest.cc
+++ b/src/cleanup_immediately_unittest.cc
@ -0,0 +1,96 @@
+// Copyright (c) 2021, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include <glog/logging.h>
+#include <glog/raw_logging.h>
+
+#include "base/commandlineflags.h"
+#include "googletest.h"
+
+#ifdef HAVE_LIB_GFLAGS
+#include <gflags/gflags.h>
+using namespace GFLAGS_NAMESPACE;
+#endif
+
+#ifdef HAVE_LIB_GMOCK
+#include <gmock/gmock.h>
+
+#include "mock-log.h"
+// Introduce several symbols from gmock.
+using GOOGLE_NAMESPACE::glog_testing::ScopedMockLog;
+using testing::_;
+using testing::AllOf;
+using testing::AnyNumber;
+using testing::HasSubstr;
+using testing::InitGoogleMock;
+using testing::StrictMock;
+using testing::StrNe;
+#endif
+
+using namespace GOOGLE_NAMESPACE;
+
+TEST(CleanImmediately, logging) {
+  google::SetLogFilenameExtension(".foobar");
+  google::EnableLogCleaner(0);
+
+  for (unsigned i = 0; i < 1000; ++i) {
+    LOG(INFO) << "cleanup test";
+  }
+
+  google::DisableLogCleaner();
+}
+
+int main(int argc, char **argv) {
+  FLAGS_colorlogtostderr = false;
+  FLAGS_timestamp_in_logfile_name = true;
+#ifdef HAVE_LIB_GFLAGS
+  ParseCommandLineFlags(&argc, &argv, true);
+#endif
+  // Make sure stderr is not buffered as stderr seems to be buffered
+  // on recent windows.
+  setbuf(stderr, NULL);
+
+  // Test some basics before InitGoogleLogging:
+  CaptureTestStderr();
+  const string early_stderr = GetCapturedTestStderr();
+
+  EXPECT_FALSE(IsGoogleLoggingInitialized());
+
+  InitGoogleLogging(argv[0]);
+
+  EXPECT_TRUE(IsGoogleLoggingInitialized());
+
+  InitGoogleTest(&argc, argv);
+#ifdef HAVE_LIB_GMOCK
+  InitGoogleMock(&argc, argv);
+#endif
+
+  // so that death tests run before we use threads
+  CHECK_EQ(RUN_ALL_TESTS(), 0);
+}
--- a/src/cleanup_with_prefix_unittest.cc
+++ b/src/cleanup_with_prefix_unittest.cc
@ -0,0 +1,97 @@
+// Copyright (c) 2021, Google Inc.
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include <glog/logging.h>
+#include <glog/raw_logging.h>
+
+#include "base/commandlineflags.h"
+#include "googletest.h"
+
+#ifdef HAVE_LIB_GFLAGS
+#include <gflags/gflags.h>
+using namespace GFLAGS_NAMESPACE;
+#endif
+
+#ifdef HAVE_LIB_GMOCK
+#include <gmock/gmock.h>
+
+#include "mock-log.h"
+// Introduce several symbols from gmock.
+using GOOGLE_NAMESPACE::glog_testing::ScopedMockLog;
+using testing::_;
+using testing::AllOf;
+using testing::AnyNumber;
+using testing::HasSubstr;
+using testing::InitGoogleMock;
+using testing::StrictMock;
+using testing::StrNe;
+#endif
+
+using namespace GOOGLE_NAMESPACE;
+
+TEST(CleanImmediatelyWithPrefix, logging) {
+  google::EnableLogCleaner(0);
+  google::SetLogFilenameExtension(".barfoo");
+  google::SetLogDestination(GLOG_INFO, "test_cleanup_");
+
+  for (unsigned i = 0; i < 1000; ++i) {
+    LOG(INFO) << "cleanup test";
+  }
+
+  google::DisableLogCleaner();
+}
+
+int main(int argc, char **argv) {
+  FLAGS_colorlogtostderr = false;
+  FLAGS_timestamp_in_logfile_name = true;
+#ifdef HAVE_LIB_GFLAGS
+  ParseCommandLineFlags(&argc, &argv, true);
+#endif
+  // Make sure stderr is not buffered as stderr seems to be buffered
+  // on recent windows.
+  setbuf(stderr, NULL);
+
+  // Test some basics before InitGoogleLogging:
+  CaptureTestStderr();
+  const string early_stderr = GetCapturedTestStderr();
+
+  EXPECT_FALSE(IsGoogleLoggingInitialized());
+
+  InitGoogleLogging(argv[0]);
+
+  EXPECT_TRUE(IsGoogleLoggingInitialized());
+
+  InitGoogleTest(&argc, argv);
+#ifdef HAVE_LIB_GMOCK
+  InitGoogleMock(&argc, argv);
+#endif
+
+  // so that death tests run before we use threads
+  CHECK_EQ(RUN_ALL_TESTS(), 0);
+}
--- a/src/logging.cc
+++ b/src/logging.cc
@ -502,7 +502,6 @@ class LogCleaner {

  bool enabled_;
  int overdue_days_;
-  char dir_delim_;  // filepath delimiter ('/' or '\\')
 };

 LogCleaner log_cleaner;
@ -916,6 +915,13 @@ void SetApplicationFingerprint(const std::string& fingerprint) {

 namespace {

+// Directory delimiter; Windows supports both forward slashes and backslashes
+#ifdef GLOG_OS_WINDOWS
+const char possible_dir_delim[] = {'\\', '/'};
+#else
+const char possible_dir_delim[] = {'/'};
+#endif
+
 string PrettyDuration(int secs) {
  std::stringstream result;
  int mins = secs / 60;
@ -1288,7 +1294,7 @@ void LogFileObject::Write(bool force_flush,
    }
 #endif

-    // Perform clean up for old logs
+    // Remove old logs
    if (log_cleaner.enabled()) {
      if (base_filename_selected_ && base_filename_.empty()) {
        return;
@ -1300,17 +1306,11 @@ void LogFileObject::Write(bool force_flush,
  }
 }

-
-LogCleaner::LogCleaner() : enabled_(false), overdue_days_(7), dir_delim_('/') {
-#ifdef GLOG_OS_WINDOWS
-  dir_delim_ = '\\';
-#endif
-}
+LogCleaner::LogCleaner() : enabled_(false), overdue_days_(7) {}

 void LogCleaner::Enable(int overdue_days) {
-  // Setting overdue_days to 0 day should not be allowed!
-  // Since all logs will be deleted immediately, which will cause troubles.
-  assert(overdue_days > 0);
+  // Setting overdue_days to 0 days will delete all logs.
+  assert(overdue_days >= 0);

  enabled_ = true;
  overdue_days_ = overdue_days;
@ -1323,15 +1323,21 @@ void LogCleaner::Disable() {
 void LogCleaner::Run(bool base_filename_selected,
                     const string& base_filename,
                     const string& filename_extension) const {
-  assert(enabled_ && overdue_days_ > 0);
+  assert(enabled_ && overdue_days_ >= 0);

  vector<string> dirs;

-  if (base_filename_selected) {
-    string dir = base_filename.substr(0, base_filename.find_last_of(dir_delim_) + 1);
-    dirs.push_back(dir);
-  } else {
+  if (!base_filename_selected) {
    dirs = GetLoggingDirectories();
+  } else {
+    size_t pos = base_filename.find_last_of(possible_dir_delim, 0,
+                                            sizeof(possible_dir_delim));
+    if (pos != string::npos) {
+      string dir = base_filename.substr(0, pos + 1);
+      dirs.push_back(dir);
+    } else {
+      dirs.push_back(".");
+    }
  }

  for (size_t i = 0; i < dirs.size(); i++) {
@ -1356,17 +1362,23 @@ vector<string> LogCleaner::GetOverdueLogNames(string log_directory,
  DIR *dir;
  struct dirent *ent;

-  // If log_directory doesn't end with a slash, append a slash to it.
-  if (log_directory.at(log_directory.size() - 1) != dir_delim_) {
-    log_directory += dir_delim_;
-  }

-  if ((dir=opendir(log_directory.c_str()))) {
-    while ((ent=readdir(dir))) {
-      if (!strcmp(ent->d_name, ".") || !strcmp(ent->d_name, "..")) {
+  if ((dir = opendir(log_directory.c_str()))) {
+    while ((ent = readdir(dir))) {
+      if (strcmp(ent->d_name, ".") == 0 || strcmp(ent->d_name, "..") == 0) {
        continue;
      }
-      string filepath = log_directory + ent->d_name;
+
+      string filepath = ent->d_name;
+      const char* const dir_delim_end =
+          possible_dir_delim + sizeof(possible_dir_delim);
+
+      if (!log_directory.empty() &&
+          std::find(possible_dir_delim, dir_delim_end,
+                    log_directory[log_directory.size() - 1]) != dir_delim_end) {
+        filepath = log_directory + filepath;
+      }
+
      if (IsLogFromCurrentProject(filepath, base_filename, filename_extension) &&
          IsLogLastModifiedOver(filepath, days)) {
        overdue_log_names.push_back(filepath);
@ -1386,14 +1398,19 @@ bool LogCleaner::IsLogFromCurrentProject(const string& filepath,
  // after:  "/tmp/<base_filename>.<create_time>.<pid>"
  string cleaned_base_filename;

+  const char* const dir_delim_end =
+      possible_dir_delim + sizeof(possible_dir_delim);
+
  size_t real_filepath_size = filepath.size();
  for (size_t i = 0; i < base_filename.size(); ++i) {
    const char& c = base_filename[i];

    if (cleaned_base_filename.empty()) {
      cleaned_base_filename += c;
-    } else if (c != dir_delim_ ||
-        c != cleaned_base_filename.at(cleaned_base_filename.size() - 1)) {
+    } else if (std::find(possible_dir_delim, dir_delim_end, c) ==
+                   dir_delim_end ||
+               (!cleaned_base_filename.empty() &&
+                c != cleaned_base_filename[cleaned_base_filename.size() - 1])) {
      cleaned_base_filename += c;
    }
  }
@ -1457,10 +1474,10 @@ bool LogCleaner::IsLogLastModifiedOver(const string& filepath, int days) const {
  struct stat file_stat;

  if (stat(filepath.c_str(), &file_stat) == 0) {
-    // A day is 86400 seconds, so 7 days is 86400 * 7 = 604800 seconds.
+    const time_t seconds_in_a_day = 60 * 60 * 24;
    time_t last_modified_time = file_stat.st_mtime;
    time_t current_time = time(NULL);
-    return difftime(current_time, last_modified_time) > days * 86400;
+    return difftime(current_time, last_modified_time) > days * seconds_in_a_day;
  }

  // If failed to get file stat, don't return true!