LogCleaner: Fix relative paths and add a new test

Signed-off-by: Marco Wang <m.aesophor@gmail.com>

CMakeLists.txt

@@ -932,10 +932,15 @@ if (BUILD_TESTING)
 
   target_link_libraries (cleanup_immediately_unittest PRIVATE ${_GLOG_TEST_LIBS})
 
-  add_executable (cleanup_with_prefix_unittest
+  add_executable (cleanup_with_absolute_prefix_unittest
-    src/cleanup_with_prefix_unittest.cc)
+    src/cleanup_with_absolute_prefix_unittest.cc)
 
-  target_link_libraries (cleanup_with_prefix_unittest PRIVATE ${_GLOG_TEST_LIBS})
+  target_link_libraries (cleanup_with_absolute_prefix_unittest PRIVATE ${_GLOG_TEST_LIBS})
+
+  add_executable (cleanup_with_relative_prefix_unittest
+    src/cleanup_with_relative_prefix_unittest.cc)
+
+  target_link_libraries (cleanup_with_relative_prefix_unittest PRIVATE ${_GLOG_TEST_LIBS})
 
   set (CLEANUP_LOG_DIR ${CMAKE_CURRENT_BINARY_DIR}/cleanup_tests)
 
@@ -950,12 +955,19 @@ if (BUILD_TESTING)
     -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
+  add_test (NAME cleanup_with_absolute_prefix COMMAND
     ${CMAKE_COMMAND}
-    -DLOGCLEANUP=$<TARGET_FILE:cleanup_with_prefix_unittest>
+    -DLOGCLEANUP=$<TARGET_FILE:cleanup_with_absolute_prefix_unittest>
     -DTEST_DIR=${CMAKE_CURRENT_BINARY_DIR}/
     -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/RunCleanerTest2.cmake
     WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
+  add_test (NAME cleanup_with_relative_prefix COMMAND
+    ${CMAKE_COMMAND}
+    -DLOGCLEANUP=$<TARGET_FILE:cleanup_with_relative_prefix_unittest>
+    -DTEST_DIR=${CMAKE_CURRENT_BINARY_DIR}/
+    -DTEST_SUBDIR=test_subdir/
+    -P ${CMAKE_CURRENT_SOURCE_DIR}/cmake/RunCleanerTest3.cmake
+    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
 
   # Fixtures setup
   set_tests_properties (cleanup_init PROPERTIES FIXTURES_SETUP logcleanuptest)
@@ -963,7 +975,8 @@ if (BUILD_TESTING)
   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)
+  set_tests_properties (cleanup_with_absolute_prefix PROPERTIES FIXTURES_REQUIRED logcleanuptest)
+  set_tests_properties (cleanup_with_relative_prefix PROPERTIES FIXTURES_REQUIRED logcleanuptest)
 endif (BUILD_TESTING)
 
 install (TARGETS glog

cmake/RunCleanerTest3.cmake

@@ -0,0 +1,28 @@
+set (RUNS 3)
+
+# Create the subdirectory required by this unit test.
+file (MAKE_DIRECTORY ${TEST_DIR}/${TEST_SUBDIR})
+
+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 2)
+endforeach (iter)
+
+file (GLOB LOG_FILES ${TEST_DIR}/${TEST_SUBDIR}/test_cleanup_*.relativefoo)
+list (LENGTH LOG_FILES NUM_FILES)
+
+if (NOT NUM_FILES EQUAL 1)
+  message (SEND_ERROR "Expected 1 log file in build directory ${TEST_DIR}${TEST_SUBDIR} but found ${NUM_FILES}")
+endif (NOT NUM_FILES EQUAL 1)
+
+# Remove the subdirectory required by this unit test.
+file (REMOVE_RECURSE ${TEST_DIR}/${TEST_SUBDIR})

src/cleanup_with_absolute_prefix_unittest.cc

@@ -55,7 +55,7 @@ using testing::StrNe;
 
 using namespace GOOGLE_NAMESPACE;
 
-TEST(CleanImmediatelyWithPrefix, logging) {
+TEST(CleanImmediatelyWithAbsolutePrefix, logging) {
   google::EnableLogCleaner(0);
   google::SetLogFilenameExtension(".barfoo");
   google::SetLogDestination(GLOG_INFO, "test_cleanup_");

src/cleanup_with_relative_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(CleanImmediatelyWithRelativePrefix, logging) {
+  google::EnableLogCleaner(0);
+  google::SetLogFilenameExtension(".relativefoo");
+  google::SetLogDestination(GLOG_INFO, "test_subdir/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);
+}

src/logging.cc

@@ -1322,7 +1322,7 @@ void LogCleaner::Run(bool base_filename_selected,
   if (!base_filename_selected) {
     dirs = GetLoggingDirectories();
   } else {
-    size_t pos = base_filename.find_last_of(possible_dir_delim, 0,
+    size_t pos = base_filename.find_last_of(possible_dir_delim, string::npos,
                                             sizeof(possible_dir_delim));
     if (pos != string::npos) {
       string dir = base_filename.substr(0, pos + 1);