Merge pull request #618 from log4js-node/linting

Linting
log4js-node · Nov 18, 2017 · 1350753 · 1350753
2 parents 7e008b9 + f093279
commit 1350753
Show file tree

Hide file tree

Showing 16 changed files with 244 additions and 288 deletions.
diff --git a/.gitignore b/.gitignore
@@ -21,3 +21,5 @@ coverage/
 .nyc_output/
 _site
 Gemfile.lock
+Dockerfile
+docker-compose.yml
diff --git a/.npmignore b/.npmignore
@@ -14,3 +14,5 @@ Makefile
 coverage
 Gemfile
 Gemfile.lock
+docker-compose.yml
+Dockerfile
diff --git a/lib/configuration.js b/lib/configuration.js
@@ -40,7 +40,8 @@ class Configuration {
     const tests = Array.isArray(checks) ? checks : [checks];
     tests.forEach((test) => {
       if (test) {
-        throw new Error(`Problem with log4js configuration: (${util.inspect(this.candidate, { depth: 5 })}) - ${message}`);
+        throw new Error(`Problem with log4js configuration: (${util.inspect(this.candidate, { depth: 5 })})` +
+          ` - ${message}`);
       }
     });
   }

diff --git a/lib/connect-logger.js b/lib/connect-logger.js
@@ -22,7 +22,7 @@ function getUrl(req) {
 }
 
 
-  /**
+/**
    * Adds custom {token, replacement} objects to defaults,
    * overwriting the defaults if any tokens clash
    *
@@ -37,8 +37,8 @@ function assembleTokens(req, res, customTokens) {
     const a = array.concat();
     for (let i = 0; i < a.length; ++i) {
       for (let j = i + 1; j < a.length; ++j) {
-          // not === because token can be regexp object
-          /* eslint eqeqeq:0 */
+        // not === because token can be regexp object
+        /* eslint eqeqeq:0 */
         if (a[i].token == a[j].token) {
           a.splice(j--, 1);
         }
@@ -91,15 +91,15 @@ function assembleTokens(req, res, customTokens) {
     token: /:res\[([^\]]+)]/g,
     replacement: function (_, field) {
       return res._headers ?
-               (res._headers[field.toLowerCase()] || res.__headers[field])
-          : (res.__headers && res.__headers[field]);
+        (res._headers[field.toLowerCase()] || res.__headers[field])
+        : (res.__headers && res.__headers[field]);
     }
   });
 
   return arrayUniqueTokens(customTokens.concat(defaultTokens));
 }
 
-  /**
+/**
    * Return formatted log line.
    *
    * @param  {String} str
@@ -114,7 +114,7 @@ function format(str, tokens) {
   return str;
 }
 
-  /**
+/**
    * Return RegExp Object about nolog
    *
    * @param  {String|Array} nolog
@@ -154,7 +154,7 @@ function createNoLogCondition(nolog) {
     }
 
     if (Array.isArray(nolog)) {
-        // convert to strings
+      // convert to strings
       const regexpsAsStrings = nolog.map(reg => (reg.source ? reg.source : reg));
       regexp = new RegExp(regexpsAsStrings.join('|'));
     }

diff --git a/lib/layouts.js b/lib/layouts.js
@@ -112,10 +112,10 @@ function timestampLevelAndCategory(loggingEvent, colour, timezoneOffset) {
  */
 function basicLayout(loggingEvent, timezoneOffset) {
   return timestampLevelAndCategory(
-      loggingEvent,
-      undefined,
-      timezoneOffset
-    ) + formatLogData(loggingEvent.data);
+    loggingEvent,
+    undefined,
+    timezoneOffset
+  ) + formatLogData(loggingEvent.data);
 }
 
 /**
@@ -124,10 +124,10 @@ function basicLayout(loggingEvent, timezoneOffset) {
  */
 function colouredLayout(loggingEvent, timezoneOffset) {
   return timestampLevelAndCategory(
-      loggingEvent,
-      loggingEvent.level.colour,
-      timezoneOffset
-    ) + formatLogData(loggingEvent.data);
+    loggingEvent,
+    loggingEvent.level.colour,
+    timezoneOffset
+  ) + formatLogData(loggingEvent.data);
 }
 
 function messagePassThroughLayout(loggingEvent) {

diff --git a/lib/levels.js b/lib/levels.js
@@ -36,7 +36,6 @@ module.exports = function (customLevels) {
       }
       return this.level === otherLevel.level;
     }
-
   }
 
   const defaultLevels = {

diff --git a/package.json b/package.json
@@ -25,11 +25,10 @@
   },
   "scripts": {
     "clean": "find test -type f ! -name '*.json' ! -name '*.js' ! -name '.eslintrc' -delete && rm *.log",
-    "lint": "eslint lib/ test/",
     "prepush": "npm test",
     "commitmsg": "validate-commit-msg",
     "posttest": "npm run clean",
-    "pretest": "eslint lib/**/*",
+    "pretest": "eslint 'lib/**/*.js' 'test/**/*.js'",
     "test": "tap 'test/tap/**/*.js'",
     "coverage": "tap 'test/tap/**/*.js' --cov",
     "codecov": "tap 'test/tap/**/*.js' --cov --coverage-report=lcov && codecov"

diff --git a/test/tap/configuration-validation-test.js b/test/tap/configuration-validation-test.js
@@ -24,9 +24,8 @@ function testAppender(label) {
 test('log4js configuration validation', (batch) => {
   batch.test('should give error if config is just plain silly', (t) => {
     [null, undefined, '', ' ', []].forEach((config) => {
-      const expectedError = new Error(
-        `Problem with log4js configuration: (${util.inspect(config)}) - must be an object.`
-      );
+      const expectedError =
+        new Error(`Problem with log4js configuration: (${util.inspect(config)}) - must be an object.`);
       t.throws(
         () => new Configuration(config),
         expectedError
@@ -37,34 +36,32 @@ test('log4js configuration validation', (batch) => {
   });
 
   batch.test('should give error if config is an empty object', (t) => {
-    const expectedError = new Error(
-      'Problem with log4js configuration: ({}) - must have a property "appenders" of type object.'
-    );
+    const expectedError =
+      new Error('Problem with log4js configuration: ({}) - must have a property "appenders" of type object.');
     t.throws(() => new Configuration({}), expectedError);
     t.end();
   });
 
   batch.test('should give error if config has no appenders', (t) => {
-    const expectedError = new Error(
-      'Problem with log4js configuration: ({ categories: {} }) - must have a property "appenders" of type object.'
-    );
+    const expectedError =
+      new Error('Problem with log4js configuration: ({ categories: {} }) ' +
+      '- must have a property "appenders" of type object.');
     t.throws(() => new Configuration({ categories: {} }), expectedError);
     t.end();
   });
 
   batch.test('should give error if config has no categories', (t) => {
-    const expectedError = new Error(
-      'Problem with log4js configuration: ({ appenders: {} }) - must have a property "categories" of type object.'
-    );
+    const expectedError =
+      new Error('Problem with log4js configuration: ({ appenders: {} }) ' +
+      '- must have a property "categories" of type object.');
     t.throws(() => new Configuration({ appenders: {} }), expectedError);
     t.end();
   });
 
   batch.test('should give error if appenders is not an object', (t) => {
-    const error = new Error(
-      'Problem with log4js configuration: ({ appenders: [], categories: [] })' +
-      ' - must have a property "appenders" of type object.'
-    );
+    const error =
+      new Error('Problem with log4js configuration: ({ appenders: [], categories: [] })' +
+      ' - must have a property "appenders" of type object.');
     t.throws(
       () => new Configuration({ appenders: [], categories: [] }),
       error
@@ -73,10 +70,9 @@ test('log4js configuration validation', (batch) => {
   });
 
   batch.test('should give error if appenders are not all valid', (t) => {
-    const error = new Error(
-      'Problem with log4js configuration: ({ appenders: { thing: \'cheese\' }, categories: {} })' +
-      ' - appender "thing" is not valid (must be an object with property "type")'
-    );
+    const error =
+      new Error('Problem with log4js configuration: ({ appenders: { thing: \'cheese\' }, categories: {} })' +
+      ' - appender "thing" is not valid (must be an object with property "type")');
     t.throws(
       () => new Configuration({ appenders: { thing: 'cheese' }, categories: {} }),
       error
@@ -85,10 +81,8 @@ test('log4js configuration validation', (batch) => {
   });
 
   batch.test('should require at least one appender', (t) => {
-    const error = new Error(
-      'Problem with log4js configuration: ({ appenders: {}, categories: {} })' +
-      ' - must define at least one appender.'
-    );
+    const error = new Error('Problem with log4js configuration: ({ appenders: {}, categories: {} })' +
+      ' - must define at least one appender.');
     t.throws(
       () => new Configuration({ appenders: {}, categories: {} }),
       error
@@ -97,11 +91,9 @@ test('log4js configuration validation', (batch) => {
   });
 
   batch.test('should give error if categories are not all valid', (t) => {
-    const error = new Error(
-      'Problem with log4js configuration: ' +
+    const error = new Error('Problem with log4js configuration: ' +
       '({ appenders: { stdout: { type: \'stdout\' } },\n  categories: { thing: \'cheese\' } })' +
-      ' - category "thing" is not valid (must be an object with properties "appenders" and "level")'
-    );
+      ' - category "thing" is not valid (must be an object with properties "appenders" and "level")');
     t.throws(
       () => new Configuration({ appenders: { stdout: { type: 'stdout' } }, categories: { thing: 'cheese' } }),
       error
@@ -110,27 +102,24 @@ test('log4js configuration validation', (batch) => {
   });
 
   batch.test('should give error if default category not defined', (t) => {
-    const error = new Error(
-      'Problem with log4js configuration: ' +
+    const error = new Error('Problem with log4js configuration: ' +
       '({ appenders: { stdout: { type: \'stdout\' } },\n' +
       '  categories: { thing: { appenders: [ \'stdout\' ], level: \'ERROR\' } } })' +
-      ' - must define a "default" category.'
-    );
+      ' - must define a "default" category.');
     t.throws(
       () => new Configuration({
         appenders: { stdout: { type: 'stdout' } },
-        categories: { thing: { appenders: ['stdout'], level: 'ERROR' } } }
-      ),
+        categories: { thing: { appenders: ['stdout'], level: 'ERROR' } }
+      }),
       error
     );
     t.end();
   });
 
   batch.test('should require at least one category', (t) => {
-    const error = new Error(
-      'Problem with log4js configuration: ({ appenders: { stdout: { type: \'stdout\' } }, categories: {} })' +
-      ' - must define at least one category.'
-    );
+    const error =
+      new Error('Problem with log4js configuration: ({ appenders: { stdout: { type: \'stdout\' } }, categories: {} })' +
+      ' - must define at least one category.');
     t.throws(
       () => new Configuration({ appenders: { stdout: { type: 'stdout' } }, categories: {} }),
       error
@@ -139,12 +128,10 @@ test('log4js configuration validation', (batch) => {
   });
 
   batch.test('should give error if category.appenders is not an array', (t) => {
-    const error = new Error(
-      'Problem with log4js configuration: ' +
+    const error = new Error('Problem with log4js configuration: ' +
       '({ appenders: { stdout: { type: \'stdout\' } },\n' +
       '  categories: { thing: { appenders: {}, level: \'ERROR\' } } })' +
-      ' - category "thing" is not valid (appenders must be an array of appender names)'
-    );
+      ' - category "thing" is not valid (appenders must be an array of appender names)');
     t.throws(
       () => new Configuration({
         appenders: { stdout: { type: 'stdout' } },
@@ -156,12 +143,10 @@ test('log4js configuration validation', (batch) => {
   });
 
   batch.test('should give error if category.appenders is empty', (t) => {
-    const error = new Error(
-      'Problem with log4js configuration: ' +
+    const error = new Error('Problem with log4js configuration: ' +
       '({ appenders: { stdout: { type: \'stdout\' } },\n' +
       '  categories: { thing: { appenders: [], level: \'ERROR\' } } })' +
-      ' - category "thing" is not valid (appenders must contain at least one appender name)'
-    );
+      ' - category "thing" is not valid (appenders must contain at least one appender name)');
     t.throws(
       () => new Configuration({
         appenders: { stdout: { type: 'stdout' } },
@@ -173,12 +158,10 @@ test('log4js configuration validation', (batch) => {
   });
 
   batch.test('should give error if categories do not refer to valid appenders', (t) => {
-    const error = new Error(
-      'Problem with log4js configuration: ' +
+    const error = new Error('Problem with log4js configuration: ' +
       '({ appenders: { stdout: { type: \'stdout\' } },\n' +
       '  categories: { thing: { appenders: [ \'cheese\' ], level: \'ERROR\' } } })' +
-      ' - category "thing" is not valid (appender "cheese" is not defined)'
-    );
+      ' - category "thing" is not valid (appender "cheese" is not defined)');
     t.throws(
       () => new Configuration({
         appenders: { stdout: { type: 'stdout' } },
@@ -190,13 +173,11 @@ test('log4js configuration validation', (batch) => {
   });
 
   batch.test('should give error if category level is not valid', (t) => {
-    const error = new Error(
-      'Problem with log4js configuration: ' +
+    const error = new Error('Problem with log4js configuration: ' +
       '({ appenders: { stdout: { type: \'stdout\' } },\n' +
       '  categories: { default: { appenders: [ \'stdout\' ], level: \'Biscuits\' } } })' +
       ' - category "default" is not valid (level "Biscuits" not recognised; ' +
-      'valid levels are ALL, TRACE, DEBUG, INFO, WARN, ERROR, FATAL, MARK, OFF)'
-    );
+      'valid levels are ALL, TRACE, DEBUG, INFO, WARN, ERROR, FATAL, MARK, OFF)');
     t.throws(
       () => new Configuration({
         appenders: { stdout: { type: 'stdout' } },
@@ -208,12 +189,10 @@ test('log4js configuration validation', (batch) => {
   });
 
   batch.test('should give error if appender type cannot be found', (t) => {
-    const error = new Error(
-      'Problem with log4js configuration: ' +
+    const error = new Error('Problem with log4js configuration: ' +
       '({ appenders: { thing: { type: \'cheese\' } },\n' +
       '  categories: { default: { appenders: [ \'thing\' ], level: \'ERROR\' } } })' +
-      ' - appender "thing" is not valid (type "cheese" could not be found)'
-    );
+      ' - appender "thing" is not valid (type "cheese" could not be found)');
     t.throws(
       () => new Configuration({
         appenders: { thing: { type: 'cheese' } },
@@ -278,9 +257,7 @@ test('log4js configuration validation', (batch) => {
     sandboxConfig.requires[
       `${path.join(mainPath, '../../node_modules/tap/node_modules/nyc/bin/cheese')}`
     ] = testAppender('correct');
-    const SandboxedConfiguration = sandbox.require(
-      '../../lib/configuration', sandboxConfig
-    );
+    const SandboxedConfiguration = sandbox.require('../../lib/configuration', sandboxConfig);
 
     const config = new SandboxedConfiguration({
       appenders: { thing: { type: 'cheese' } },

diff --git a/test/tap/dateFileAppender-test.js b/test/tap/dateFileAppender-test.js
@@ -55,13 +55,15 @@ test('../../lib/appenders/dateFile', (batch) => {
     logger.info('this should not be written to the file');
     logger.warn('this should be written to the file');
 
-    t.teardown(() => { removeFile('date-file-test.log'); });
-
-    fs.readFile(path.join(__dirname, 'date-file-test.log'), 'utf8', (err, contents) => {
-      t.include(contents, `this should be written to the file${EOL}`);
-      t.equal(contents.indexOf('this should not be written to the file'), -1);
-      t.end();
+    log4js.shutdown(() => {
+      fs.readFile(path.join(__dirname, 'date-file-test.log'), 'utf8', (err, contents) => {
+        t.include(contents, `this should be written to the file${EOL}`);
+        t.equal(contents.indexOf('this should not be written to the file'), -1);
+        t.end();
+      });
     });
+
+    t.teardown(() => { removeFile('date-file-test.log'); });
   });
 
   batch.test('configure with options.alwaysIncludePattern', (t) => {
@@ -96,13 +98,13 @@ test('../../lib/appenders/dateFile', (batch) => {
     t.teardown(() => { removeFile(`date-file-test${thisTime}`); });
 
     // wait for filesystem to catch up
-    setTimeout(() => {
+    log4js.shutdown(() => {
       fs.readFile(path.join(__dirname, `date-file-test${thisTime}`), 'utf8', (err, contents) => {
         t.include(contents, 'this should be written to the file with the appended date');
         t.include(contents, 'this is existing data', 'should not overwrite the file on open (issue #132)');
         t.end();
       });
-    }, 100);
+    });
   });
 
   batch.test('should flush logs on shutdown', (t) => {

diff --git a/test/tap/fileAppender-test.js b/test/tap/fileAppender-test.js
@@ -88,7 +88,9 @@ test('log4js fileAppender', (batch) => {
           type: 'file', filename: testFile, maxLogSize: 100, backups: 0
         }
       },
-      categories: { default: { appenders: ['file'], level: 'debug' } }
+      categories: {
+        default: { appenders: ['file'], level: 'debug' }
+      }
     });
 
     logger.info('This is the first log message.');