Merge branch 'iantruslove-master' into dev

CHANGELOG.md

@@ -1,7 +1,8 @@
-## Pending / unreleased
+## v3.1.4 / 2014 Mar 13
 
  * NEW: Add `profiling/p*` macro.
  * CHANGE: Include `p`, `p*` in `refer-timbre` imports.
+ * FIX: rotor appender not rotating (iantruslove, kurtharriger).
 
 
 ## v3.1.3 / 2014 Mar 11

src/taoensso/timbre/appenders/rotor.clj

@@ -37,20 +37,13 @@
   the naming of archived logs."
   [basepath max-count]
   (let [abs-path (-> basepath io/file (.getAbsolutePath))
-        logs (->> basepath
-                  matching-files
-                  (take max-count)
-                  (map (fn [^File x] (.getAbsolutePath x)))
-                  sort
-                  reverse)
-        num-logs (count logs)
-        overflow? (> num-logs max-count)]
-    (when overflow?
-      (io/delete-file (first logs)))
-    (loop [[log & more] (if overflow? (rest logs) logs) n num-logs]
-      (when log
-        (.renameTo (io/file log) (io/file (format "%s.%03d" abs-path n)))
-        (recur more (dec n))))))
+        logs     (-> basepath matching-files sort)
+        [logs-to-rotate logs-to-delete] (split-at max-count logs)]
+    (doseq [log-to-delete logs-to-delete]
+      (io/delete-file log-to-delete))
+    (doseq [[^File log-file n]
+            (reverse (map vector logs-to-rotate (iterate inc 1)))]
+      (.renameTo log-file (io/file (format "%s.%03d" abs-path n))))))
 
 (defn appender-fn [{:keys [ap-config output]}]
   (let [{:keys [path max-size backlog]

test/taoensso/test/timbre/appenders/rotor.clj

@@ -0,0 +1,38 @@
+(ns taoensso.test.timbre.appenders.rotor
+  (:require [taoensso.timbre.appenders.rotor :as rotor :refer :all]
+            [clojure.test                              :refer :all]
+            [clojure.java.io                           :refer [file]]))
+
+(defn with-temp-dir-containing-log-files
+  "Call f with the temp directory name, that directory having n log
+  files created within it"
+  [n f]
+  (let [tmp-dir (java.io.File/createTempFile "test" "")
+        log-file-basename "log"
+        log-files (into [log-file-basename]
+                        (map #(format "%s.%03d" log-file-basename %) (range 1 n)))]
+    (.delete tmp-dir)
+    (.mkdirs tmp-dir)
+    (doseq [filename log-files] (.createNewFile (file tmp-dir filename)))
+    (try
+      (f (.getAbsolutePath (file tmp-dir (first log-files))))
+      (finally
+        (doseq [filename log-files] (.delete (file tmp-dir filename)))
+        (.delete (file tmp-dir))))))
+
+(deftest test-rotor
+  (testing "rotating logs"
+    (testing "when rotating with a full backlog of files, the last should be deleted"
+      (with-temp-dir-containing-log-files 5
+        (fn [basepath]
+          (#'rotor/rotate-logs basepath 2)
+          (is (not (.exists (file (str basepath))))
+              "log should have been rotated to log.001")
+          (is (.exists (file (str basepath ".001")))
+              "log.001 should remain")
+          (is (.exists (file (str basepath ".002")))
+              "log.002 should remain")
+          (is (not (.exists (file (str basepath ".003"))))
+              "log.003 should be deleted because it is past the max-count threshold")
+          (is (not (.exists (file (str basepath ".004"))))
+              "log.004 should be deleted because it is past the max-count threshold"))))))