fix file rolling when tailing is set to true (#1420)

* fix issue #1194 - where rotation of files isn't done when tailable is set to true

* more fixes to tail rolling + fix & enable unit tests

* fix unit tests interference from 01-file-maxsize - not sure why a new file gets created on an unrelated transport when adding a new Transport

* Update 01-file-maxsize.test.js

* Update file.js

Author: ledbit

lib/winston/transports/file.js

@@ -594,28 +594,26 @@ module.exports = class File extends TransportStream {
 
     // const isZipped = this.zippedArchive ? '.gz' : '';
     const isZipped = this.zippedArchive ? '.gz' : '';
-    for (let i = this.maxFiles - 1; i > 0; i--) {
-      tasks.push(() => {
-        return cb => {
-          let fileName = `${basename}${(i - 1)}${ext}${isZipped}`;
-          const tmppath = path.join(this.dirname, fileName);
-
-          fs.exists(tmppath, exists => {
-            if (!exists) {
-              return cb(null);
-            }
-
-            fileName = `${basename}${i}${ext}${isZipped}`;
-            fs.rename(tmppath, path.join(this.dirname, fileName), cb);
-          });
-        };
-      });
+    for (let x = this.maxFiles - 1; x > 1; x--) {
+      tasks.push(function (i, cb) {
+        let fileName = `${basename}${(i - 1)}${ext}${isZipped}`;
+        const tmppath = path.join(this.dirname, fileName);
+
+        fs.exists(tmppath, exists => {
+          if (!exists) {
+            return cb(null);
+          }
+
+          fileName = `${basename}${i}${ext}${isZipped}`;
+          fs.rename(tmppath, path.join(this.dirname, fileName), cb);
+        });
+      }.bind(this, x));
     }
 
     asyncSeries(tasks, () => {
       fs.rename(
         path.join(this.dirname, `${basename}${ext}`),
-        path.join(this.dirname, `${basename}1${ext}`),
+        path.join(this.dirname, `${basename}1${ext}${isZipped}`),
         callback
       );
     });

test/transports/01-file-maxsize.test.js

@@ -23,8 +23,12 @@ function removeFixtures(done) {
 describe('File (maxsize)', function () {
   this.timeout(10000);
 
+  let testDone = false;
   before(removeFixtures);
-  after(removeFixtures);
+  after(done => {
+    testDone = true;
+    removeFixtures(done);
+  });
 
   it('should create multiple files correctly when passed more than the maxsize', function (done) {
     const fillWith = ['a', 'b', 'c', 'd', 'e'];
@@ -101,6 +105,8 @@ describe('File (maxsize)', function () {
     }
 
     maxsizeTransport.on('open', function (file) {
+      if (testDone) return; // ignore future notifications
+
       const match = file.match(/(\d+)\.log$/);
       const count = match ? match[1] : 0;
 

test/transports/file-tailrolling-test.js

@@ -0,0 +1,100 @@
+var assert = require('assert'),
+    rimraf = require('rimraf'),
+    fs = require('fs'),
+    path = require('path'),
+    winston = require('../../lib/winston'),
+    helpers = require('../helpers');
+const asyncSeries = require('async/series');
+
+
+
+const { MESSAGE, LEVEL } = require('triple-beam');
+
+
+
+//
+// Remove all log fixtures
+//
+function removeFixtures(done) {
+  rimraf(path.join(__dirname, '..', 'fixtures', 'logs', 'testtailrollingfiles*'), done);
+}
+
+
+
+let tailrollTransport = null;
+
+describe('winston/transports/file/tailrolling', function(){
+  describe("An instance of the File Transport", function(){
+    before(removeFixtures);
+    after(removeFixtures);
+
+    it('init logger AFTER cleaning up old files', function(){
+	tailrollTransport = new winston.transports.File({
+	  timestamp: false,
+	  json: false,
+	  filename: path.join(__dirname, '..', 'fixtures', 'logs', 'testtailrollingfiles.log'),
+	  maxsize: 4096,
+	  maxFiles: 3,
+	  tailable: true
+	})
+        .on('open', console.log)
+    });
+
+    it("and when passed more files than the maxFiles", function(done){
+          let created = 0;
+          let loggedTotal = 0;
+   
+          function data(ch, kb) {
+            return String.fromCharCode(65 + ch).repeat(kb*1024 - 1);
+          };
+
+          function logKbytes(kbytes, txt) {
+            const toLog = {};
+	    toLog[MESSAGE] = data(txt, kbytes)
+            tailrollTransport.log(toLog);
+          }
+
+          tailrollTransport.on('logged', function (info) {
+            loggedTotal += info[MESSAGE].length + 1
+            if (loggedTotal >= 14*1024) { // just over 3 x 4kb files
+              return done();
+            }
+
+            if(loggedTotal % 4096 === 0) {
+              created ++;
+            }
+            setTimeout(() => logKbytes(1, created), 100);
+          });
+
+          logKbytes(1, created);
+       });
+
+       it("should be 3 log files, base to maxFiles - 1", function () {
+          var file, fullpath;
+          for (var num = 0; num < 4; num++) {
+            file = !num ? 'testtailrollingfiles.log' : 'testtailrollingfiles' + num + '.log';
+            fullpath = path.join(__dirname, '..', 'fixtures', 'logs', file);
+
+            if (num == 3) {
+              return assert.ok(!fs.existsSync(fullpath));
+            }
+
+            assert.ok(fs.existsSync(fullpath));
+          }
+          
+          return false;
+        });
+
+        it("should have files in correct order", function () {
+          var file, fullpath, content;
+          ['D', 'C', 'B'].forEach(function (letter, i) {
+            file = !i ? 'testtailrollingfiles.log' : 'testtailrollingfiles' + i + '.log';
+            content = fs.readFileSync(path.join(__dirname, '..', 'fixtures', 'logs', file), 'ascii');
+            content = content.replace(/\s+/g, '');
+
+            assert(content.match(new RegExp(letter, 'g'))[0].length, content.length);
+          });
+        });
+    })
+})
+