From d26b1a147bb7905ba90fb63e9f84a9243a513c60 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Wed, 8 Feb 2017 09:10:14 +1100 Subject: [PATCH] refactor: fixed a few more tests --- lib/appenders/logLevelFilter.js | 7 +-- lib/appenders/logstashUDP.js | 46 +++++++++--------- lib/appenders/mailgun.js | 36 +++++--------- lib/appenders/multiprocess.js | 46 ++++++++---------- lib/log4js.js | 2 +- test/tap/logLevelFilter-test.js | 67 ++++++++++++++++++++------ test/tap/logstashUDP-test.js | 40 ++++++++++++--- test/tap/mailgunAppender-test.js | 24 ++++----- test/tap/multiprocess-shutdown-test.js | 10 ++-- test/tap/with-logLevelFilter.json | 41 ---------------- 10 files changed, 164 insertions(+), 155 deletions(-) delete mode 100644 test/tap/with-logLevelFilter.json diff --git a/lib/appenders/logLevelFilter.js b/lib/appenders/logLevelFilter.js index ea0d420..19239da 100644 --- a/lib/appenders/logLevelFilter.js +++ b/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; diff --git a/lib/appenders/logstashUDP.js b/lib/appenders/logstashUDP.js index 7805e09..4d6ce8a 100644 --- a/lib/appenders/logstashUDP.js +++ b/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; diff --git a/lib/appenders/mailgun.js b/lib/appenders/mailgun.js index 11341ff..41ee19d 100644 --- a/lib/appenders/mailgun.js +++ b/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; diff --git a/lib/appenders/multiprocess.js b/lib/appenders/multiprocess.js index d56a714..ab0d97b 100644 --- a/lib/appenders/multiprocess.js +++ b/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; diff --git a/lib/log4js.js b/lib/log4js.js index 23119d2..c1a9887 100644 --- a/lib/log4js.js +++ b/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; } diff --git a/test/tap/logLevelFilter-test.js b/test/tap/logLevelFilter-test.js index 9a09aef..68fdab3 100644 --- a/test/tap/logLevelFilter-test.js +++ b/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'); diff --git a/test/tap/logstashUDP-test.js b/test/tap/logstashUDP-test.js index 5bacc4e..b50b1be 100644 --- a/test/tap/logstashUDP-test.js +++ b/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(); }); diff --git a/test/tap/mailgunAppender-test.js b/test/tap/mailgunAppender-test.js index 3408a38..248bd4a 100644 --- a/test/tap/mailgunAppender-test.js +++ b/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', { diff --git a/test/tap/multiprocess-shutdown-test.js b/test/tap/multiprocess-shutdown-test.js index 1bde591..660e1b4 100644 --- a/test/tap/multiprocess-shutdown-test.js +++ b/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(() => { diff --git a/test/tap/with-logLevelFilter.json b/test/tap/with-logLevelFilter.json deleted file mode 100644 index 0995d35..0000000 --- a/test/tap/with-logLevelFilter.json +++ /dev/null @@ -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" - } -}