From 3925f8fa16d3e08f26485e177456c44ca599e8ed Mon Sep 17 00:00:00 2001
From: Philip Miller <philip.miller@sri.com>
Date: Thu, 24 Jan 2019 00:06:15 -0500
Subject: [PATCH] streamline constructor logic

and improve test for rotate_on_open=true
---
 include/spdlog/sinks/rotating_file_sink.h | 20 ++++++++++++++++----
 tests/test_file_logging.cpp               | 19 ++++++++++++++++---
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/include/spdlog/sinks/rotating_file_sink.h b/include/spdlog/sinks/rotating_file_sink.h
index 0f057ad9..b2f52226 100644
--- a/include/spdlog/sinks/rotating_file_sink.h
+++ b/include/spdlog/sinks/rotating_file_sink.h
@@ -36,9 +36,14 @@ public:
         , max_size_(max_size)
         , max_files_(max_files)
     {
-        file_helper_.open(calc_filename(base_filename_, 0));
         if (rotate_on_open)
-            rotate_();
+        {
+            rotate_(false);
+        }
+        else
+        {
+            file_helper_.open(calc_filename(base_filename_, 0));
+        }
         current_size_ = file_helper_.size(); // expensive. called only once
     }
 
@@ -90,7 +95,7 @@ private:
     // log.1.txt -> log.2.txt
     // log.2.txt -> log.3.txt
     // log.3.txt -> delete
-    void rotate_()
+    void rotate_(bool reopen=true)
     {
         using details::os::filename_to_str;
         file_helper_.close();
@@ -118,7 +123,14 @@ private:
                 }
             }
         }
-        file_helper_.reopen(true);
+        if (reopen)
+        {
+            file_helper_.reopen(true);
+        }
+        else
+        {
+            file_helper_.open(base_filename_, true);
+        }
     }
 
     // delete the target if exists, and rename the src file  to target
diff --git a/tests/test_file_logging.cpp b/tests/test_file_logging.cpp
index 463f77e3..7df61f9e 100644
--- a/tests/test_file_logging.cpp
+++ b/tests/test_file_logging.cpp
@@ -38,7 +38,7 @@ TEST_CASE("flush_on", "[flush_on]]")
     REQUIRE(count_lines(filename) == 3);
 }
 
-TEST_CASE("rotating_file_logger1", "[rotating_logger]]")
+TEST_CASE("rotating_file_logger", "[rotating_logger]]")
 {
     prepare_logdir();
     size_t max_size = 1024 * 10;
@@ -55,12 +55,25 @@ TEST_CASE("rotating_file_logger1", "[rotating_logger]]")
     REQUIRE(count_lines(filename) == 10);
 }
 
-TEST_CASE("rotating_file_logger2", "[rotating_logger]]")
+TEST_CASE("rotating_file_logger2", "[rotating_logger2]]")
 {
     prepare_logdir();
     size_t max_size = 1024 * 10;
     std::string basename = "logs/rotating_log";
-    auto logger = spdlog::rotating_logger_mt("logger", basename, max_size, 1, true);
+
+    {
+        // make an initial logger to create the first output file
+        auto logger = spdlog::rotating_logger_mt("logger", basename, max_size, 2, true);
+        for (int i = 0; i < 10; ++i)
+        {
+            logger->info("Test message {}", i);
+        }
+        // drop causes the logger destructor to be called, which is required so the
+        // next logger can rename the first output file. 
+        spdlog::drop(logger->name());
+    }
+
+    auto logger = spdlog::rotating_logger_mt("logger", basename, max_size, 2, true);
     for (int i = 0; i < 10; ++i)
     {
         logger->info("Test message {}", i);