refactor: fixed a few more tests

lib/appenders/logLevelFilter.js

@@ -1,7 +1,6 @@
 'use strict';
 
 const levels = require('../levels');
-const log4js = require('../log4js');
 
 function logLevelFilter(minLevelString, maxLevelString, appender) {
   const minLevel = levels.toLevel(minLevelString);
@@ -14,11 +13,9 @@ function logLevelFilter(minLevelString, maxLevelString, appender) {
   };
 }
 
-function configure(config, options) {
-  log4js.loadAppender(config.appender.type);
-  const appender = log4js.appenderMakers[config.appender.type](config.appender, options);
+function configure(config, layouts, findAppender) {
+  const appender = findAppender(config.appender);
   return logLevelFilter(config.level, config.maxLevel, appender);
 }
 
-module.exports.appender = logLevelFilter;
 module.exports.configure = configure;

lib/appenders/logstashUDP.js

@@ -1,19 +1,29 @@
 'use strict';
 
-const layouts = require('../layouts');
 const dgram = require('dgram');
 const util = require('util');
 
+function sendLog(udp, host, port, logObject) {
+  const buffer = new Buffer(JSON.stringify(logObject));
+
+  /* eslint no-unused-vars:0 */
+  udp.send(buffer, 0, buffer.length, port, host, (err, bytes) => {
+    if (err) {
+      console.error('log4js.logstashUDP - %s:%p Error: %s', host, port, util.inspect(err));
+    }
+  });
+}
+
+
 function logstashUDP(config, layout) {
   const udp = dgram.createSocket('udp4');
   const type = config.logType ? config.logType : config.category;
-  layout = layout || layouts.dummyLayout;
 
   if (!config.fields) {
     config.fields = {};
   }
 
-  return function log(loggingEvent) {
+  function log(loggingEvent) {
     /*
      https://gist.github.com/jordansissel/2996677
      {
@@ -30,11 +40,9 @@ function logstashUDP(config, layout) {
     /* eslint no-prototype-builtins:1,no-restricted-syntax:[1, "ForInStatement"] */
     if (loggingEvent.data.length > 1) {
       const secondEvData = loggingEvent.data[1];
-      for (const key in secondEvData) {
-        if (secondEvData.hasOwnProperty(key)) {
-          config.fields[key] = secondEvData[key];
-        }
-      }
+      Object.keys(secondEvData).forEach((key) => {
+        config.fields[key] = secondEvData[key];
+      });
     }
     config.fields.level = loggingEvent.level.levelStr;
     config.fields.category = loggingEvent.categoryName;
@@ -52,22 +60,17 @@ function logstashUDP(config, layout) {
       logObject[keys[i]] = config.fields[keys[i]];
     }
     sendLog(udp, config.host, config.port, logObject);
+  }
+
+  log.shutdown = function (cb) {
+    udp.close(cb);
   };
+
+  return log;
 }
 
-function sendLog(udp, host, port, logObject) {
-  const buffer = new Buffer(JSON.stringify(logObject));
-
-  /* eslint no-unused-vars:0 */
-  udp.send(buffer, 0, buffer.length, port, host, (err, bytes) => {
-    if (err) {
-      console.error('log4js.logstashUDP - %s:%p Error: %s', host, port, util.inspect(err));
-    }
-  });
-}
-
-function configure(config) {
-  let layout;
+function configure(config, layouts) {
+  let layout = layouts.dummyLayout;
   if (config.layout) {
     layout = layouts.layout(config.layout.type, config.layout);
   }
@@ -75,5 +78,4 @@ function configure(config) {
   return logstashUDP(config, layout);
 }
 
-module.exports.appender = logstashUDP;
 module.exports.configure = configure;

lib/appenders/mailgun.js

@@ -1,21 +1,18 @@
 'use strict';
 
-const layouts = require('../layouts');
 const mailgunFactory = require('mailgun-js');
 
-let layout;
-let config;
-let mailgun;
-
-function mailgunAppender(_config, _layout) {
-  config = _config;
-  layout = _layout || layouts.basicLayout;
+function mailgunAppender(config, layout) {
+  const mailgun = mailgunFactory({
+    apiKey: config.apikey,
+    domain: config.domain
+  });
 
   return (loggingEvent) => {
     const data = {
-      from: _config.from,
-      to: _config.to,
-      subject: _config.subject,
+      from: config.from,
+      to: config.to,
+      subject: config.subject,
       text: layout(loggingEvent, config.timezoneOffset)
     };
 
@@ -26,20 +23,13 @@ function mailgunAppender(_config, _layout) {
   };
 }
 
-function configure(_config) {
-  config = _config;
-
-  if (_config.layout) {
-    layout = layouts.layout(_config.layout.type, _config.layout);
+function configure(config, layouts) {
+  let layout = layouts.basicLayout;
+  if (config.layout) {
+    layout = layouts.layout(config.layout.type, config.layout);
   }
 
-  mailgun = mailgunFactory({
-    apiKey: _config.apikey,
-    domain: _config.domain
-  });
-
-  return mailgunAppender(_config, layout);
+  return mailgunAppender(config, layout);
 }
 
-module.exports.appender = mailgunAppender;
 module.exports.configure = configure;

lib/appenders/multiprocess.js

@@ -1,10 +1,9 @@
 'use strict';
 
-const log4js = require('../log4js');
+const levels = require('../levels');
 const net = require('net');
 
 const END_MSG = '__LOG4JS__';
-const servers = [];
 
 /**
  * Creates a server, listening on config.loggerPort, config.loggerHost.
@@ -21,13 +20,13 @@ function logServer(config) {
     try {
       loggingEvent = JSON.parse(msg);
       loggingEvent.startTime = new Date(loggingEvent.startTime);
-      loggingEvent.level = log4js.levels.toLevel(loggingEvent.level.levelStr);
+      loggingEvent.level = levels.toLevel(loggingEvent.level.levelStr);
     } catch (e) {
       // JSON.parse failed, just log the contents probably a naughty.
       loggingEvent = {
         startTime: new Date(),
         categoryName: 'log4js',
-        level: log4js.levels.ERROR,
+        level: levels.ERROR,
         data: ['Unable to parse log:', msg]
       };
     }
@@ -68,12 +67,19 @@ function logServer(config) {
   });
 
   server.listen(config.loggerPort || 5000, config.loggerHost || 'localhost', function () {
-    servers.push(server);
     // allow the process to exit, if this is the only socket active
     server.unref();
   });
 
-  return actualAppender;
+  function app(event) {
+    return actualAppender(event);
+  }
+
+  app.shutdown = function (cb) {
+    server.close(cb);
+  };
+
+  return app;
 }
 
 function workerAppender(config) {
@@ -114,13 +120,18 @@ function workerAppender(config) {
 
   createSocket();
 
-  return function log(loggingEvent) {
+  function log(loggingEvent) {
     if (canWrite) {
       write(loggingEvent);
     } else {
       buffer.push(loggingEvent);
     }
+  }
+  log.shutdown = function (cb) {
+    socket.removeAllListeners('close');
+    socket.close(cb);
   };
+  return log;
 }
 
 function createAppender(config) {
@@ -131,28 +142,11 @@ function createAppender(config) {
   return workerAppender(config);
 }
 
-function configure(config, options) {
-  let actualAppender;
+function configure(config, layouts, findAppender) {
   if (config.appender && config.mode === 'master') {
-    log4js.loadAppender(config.appender.type);
-    actualAppender = log4js.appenderMakers[config.appender.type](config.appender, options);
-    config.actualAppender = actualAppender;
+    config.actualAppender = findAppender(config.appender);
   }
   return createAppender(config);
 }
 
-function shutdown(done) {
-  let toBeClosed = servers.length;
-  servers.forEach(function (server) {
-    server.close(function () {
-      toBeClosed -= 1;
-      if (toBeClosed < 1) {
-        done();
-      }
-    });
-  });
-}
-
-module.exports.appender = createAppender;
 module.exports.configure = configure;
-module.exports.shutdown = shutdown;

lib/log4js.js

@@ -133,7 +133,7 @@ function shutdown(cb) {
     return cb();
   }
 
-  appenders.forEach(a => a.shutdown(complete));
+  appenders.filter(a => a.shutdown).forEach(a => a.shutdown(complete));
 
   return null;
 }

test/tap/logLevelFilter-test.js

@@ -17,20 +17,17 @@ function remove(filename) {
 test('log4js logLevelFilter', (batch) => {
   batch.test('appender', (t) => {
     const log4js = require('../../lib/log4js');
-    const logEvents = [];
+    const recording = require('../../lib/appenders/recording');
 
-    log4js.clearAppenders();
-    log4js.addAppender(
-      require('../../lib/appenders/logLevelFilter')
-        .appender(
-          'ERROR',
-          undefined,
-          (evt) => {
-            logEvents.push(evt);
-          }
-        ),
-      'logLevelTest'
-    );
+    log4js.configure({
+      appenders: {
+        recorder: { type: 'recording' },
+        filtered: { type: 'logLevelFilter', appender: 'recorder', level: 'ERROR' }
+      },
+      categories: {
+        default: { appenders: ['filtered'], level: 'debug' }
+      }
+    });
 
     const logger = log4js.getLogger('logLevelTest');
     logger.debug('this should not trigger an event');
@@ -38,6 +35,8 @@ test('log4js logLevelFilter', (batch) => {
     logger.error('this should, though');
     logger.fatal('so should this');
 
+    const logEvents = recording.replay();
+
     t.test('should only pass log events greater than or equal to its own level', (assert) => {
       assert.equal(logEvents.length, 2);
       assert.equal(logEvents[0].data[0], 'this should, though');
@@ -54,7 +53,47 @@ test('log4js logLevelFilter', (batch) => {
     remove(`${__dirname}/logLevelFilter-warnings.log`);
     remove(`${__dirname}/logLevelFilter-debugs.log`);
 
-    log4js.configure('test/tap/with-logLevelFilter.json');
+    t.tearDown(() => {
+      remove(`${__dirname}/logLevelFilter.log`);
+      remove(`${__dirname}/logLevelFilter-warnings.log`);
+      remove(`${__dirname}/logLevelFilter-debugs.log`);
+    });
+
+    log4js.configure({
+      appenders: {
+        'warning-file': {
+          type: 'file',
+          filename: 'test/tap/logLevelFilter-warnings.log',
+          layout: { type: 'messagePassThrough' }
+        },
+        warnings: {
+          type: 'logLevelFilter',
+          level: 'WARN',
+          appender: 'warning-file'
+        },
+        'debug-file': {
+          type: 'file',
+          filename: 'test/tap/logLevelFilter-debugs.log',
+          layout: { type: 'messagePassThrough' }
+        },
+        debugs: {
+          type: 'logLevelFilter',
+          level: 'TRACE',
+          maxLevel: 'DEBUG',
+          appender: 'debug-file'
+        },
+        tests: {
+          type: 'file',
+          filename: 'test/tap/logLevelFilter.log',
+          layout: {
+            type: 'messagePassThrough'
+          }
+        }
+      },
+      categories: {
+        default: { appenders: ['tests', 'warnings', 'debugs'], level: 'trace' }
+      }
+    });
     const logger = log4js.getLogger('tests');
     logger.debug('debug');
     logger.info('info');

test/tap/logstashUDP-test.js

@@ -1,11 +1,11 @@
 'use strict';
 
 const test = require('tap').test;
-const log4js = require('../../lib/log4js');
 const sandbox = require('sandboxed-module');
 
 function setupLogging(category, options) {
   const udpSent = {};
+  const socket = { closed: false };
 
   const fakeDgram = {
     createSocket: function () {
@@ -18,23 +18,33 @@ function setupLogging(category, options) {
           udpSent.offset = 0;
           udpSent.buffer = buffer;
           callback(undefined, length);
+        },
+        close: function (cb) {
+          socket.closed = true;
+          cb();
         }
       };
     }
   };
 
-  const logstashModule = sandbox.require('../../lib/appenders/logstashUDP', {
-    singleOnly: true,
+  const log4js = sandbox.require('../../lib/log4js', {
     requires: {
       dgram: fakeDgram
     }
   });
-  log4js.clearAppenders();
-  log4js.addAppender(logstashModule.configure(options), category);
+
+  options = options || {};
+  options.type = 'logstashUDP';
+  log4js.configure({
+    appenders: { logstash: options },
+    categories: { default: { appenders: ['logstash'], level: 'trace' } }
+  });
 
   return {
     logger: log4js.getLogger(category),
-    results: udpSent
+    log4js: log4js,
+    results: udpSent,
+    socket: socket
   };
 }
 
@@ -72,7 +82,7 @@ test('logstashUDP appender', (batch) => {
 
     const keys = Object.keys(fields);
     for (let i = 0, length = keys.length; i < length; i += 1) {
-        t.equal(json[keys[i]], fields[keys[i]]);
+      t.equal(json[keys[i]], fields[keys[i]]);
     }
 
     t.equal(JSON.stringify(json.fields), JSON.stringify(fields));
@@ -133,5 +143,21 @@ test('logstashUDP appender', (batch) => {
     t.end();
   });
 
+  batch.test('shutdown should close sockets', (t) => {
+    const setup = setupLogging('myLogger', {
+      host: '127.0.0.1',
+      port: 10001,
+      type: 'logstashUDP',
+      category: 'myLogger',
+      layout: {
+        type: 'dummy'
+      }
+    });
+    setup.log4js.shutdown(() => {
+      t.ok(setup.socket.closed);
+      t.end();
+    });
+  });
+
   batch.end();
 });

test/tap/mailgunAppender-test.js

@@ -1,7 +1,7 @@
 'use strict';
 
 const test = require('tap').test;
-const log4js = require('../../lib/log4js');
+const layouts = require('../../lib/layouts');
 const sandbox = require('sandboxed-module');
 
 function setupLogging(category, options) {
@@ -30,10 +30,10 @@ function setupLogging(category, options) {
     layout: function (type, config) {
       this.type = type;
       this.config = config;
-      return log4js.layouts.messagePassThroughLayout;
+      return layouts.messagePassThroughLayout;
     },
-    basicLayout: log4js.layouts.basicLayout,
-    messagePassThroughLayout: log4js.layouts.messagePassThroughLayout
+    basicLayout: layouts.basicLayout,
+    messagePassThroughLayout: layouts.messagePassThroughLayout
   };
 
   const fakeConsole = {
@@ -47,19 +47,21 @@ function setupLogging(category, options) {
     }
   };
 
-
-  const mailgunModule = sandbox.require('../../lib/appenders/mailgun', {
+  const log4js = sandbox.require('../../lib/log4js', {
     requires: {
       'mailgun-js': fakeMailgun,
-      '../layouts': fakeLayouts
+      './layouts': fakeLayouts
     },
     globals: {
       console: fakeConsole
     }
   });
-
-
-  log4js.addAppender(mailgunModule.configure(options), category);
+  options = options || {};
+  options.type = 'mailgun';
+  log4js.configure({
+    appenders: { mailgun: options },
+    categories: { default: { appenders: ['mailgun'], level: 'trace' } }
+  });
 
   return {
     logger: log4js.getLogger(category),
@@ -80,8 +82,6 @@ function checkMessages(assert, result) {
   }
 }
 
-log4js.clearAppenders();
-
 test('log4js mailgunAppender', (batch) => {
   batch.test('mailgun setup', (t) => {
     const result = setupLogging('mailgun setup', {

test/tap/multiprocess-shutdown-test.js

@@ -6,14 +6,16 @@ const net = require('net');
 
 test('multiprocess appender shutdown (master)', { timeout: 2000 }, (t) => {
   log4js.configure({
-    appenders: [
-      {
+    appenders: {
+      stdout: { type: 'stdout' },
+      multi: {
         type: 'multiprocess',
         mode: 'master',
         loggerPort: 12345,
-        appender: { type: 'stdout' }
+        appender: 'stdout'
       }
-    ]
+    },
+    categories: { default: { appenders: ['multi'], level: 'debug' } }
   });
 
   setTimeout(() => {

test/tap/with-logLevelFilter.json

@@ -1,41 +0,0 @@
-{
-  "appenders": [
-    {
-      "category": "tests",
-      "type": "logLevelFilter",
-      "level": "WARN",
-      "appender": {
-        "type": "file",
-        "filename": "test/tap/logLevelFilter-warnings.log",
-        "layout": {
-          "type": "messagePassThrough"
-        }
-      }
-    },
-    {
-      "category": "tests",
-      "type": "logLevelFilter",
-      "level": "TRACE",
-      "maxLevel": "DEBUG",
-      "appender": {
-        "type": "file",
-        "filename": "test/tap/logLevelFilter-debugs.log",
-        "layout": {
-          "type": "messagePassThrough"
-          }
-        }
-    },
-    {
-      "category": "tests",
-      "type": "file",
-      "filename": "test/tap/logLevelFilter.log",
-      "layout": {
-        "type": "messagePassThrough"
-      }
-    }
-  ],
-
-  "levels": {
-    "tests":  "TRACE"
-  }
-}