From 9dcc885ace7c87d543fa241fab6586a063df903d Mon Sep 17 00:00:00 2001 From: Honza Javorek Date: Thu, 24 Jan 2019 17:59:17 +0100 Subject: [PATCH] refactor: remove the --timestamp option BREAKING CHANGE: The --timestamp/-t option is no longer available. Instead, the --loglevel=debug automatically displays timestamps as well for the application logging entries. --- docs/usage-cli.rst | 1 - docs/usage-js.rst | 3 +- lib/configuration.js | 19 +++----- lib/options.json | 7 +-- lib/reporters/reporterOutputLogger.js | 2 +- test/integration/cli/hookfiles-cli-test.js | 6 +-- test/unit/CLI-test.js | 1 - test/unit/configUtils-test.js | 3 -- test/unit/configuration-test.js | 52 ++++++++-------------- 9 files changed, 30 insertions(+), 64 deletions(-) diff --git a/docs/usage-cli.rst b/docs/usage-cli.rst index 849a41348..641b53530 100644 --- a/docs/usage-cli.rst +++ b/docs/usage-cli.rst @@ -71,7 +71,6 @@ See below how sample configuration file could look like. The structure is the sa details: false method: [] loglevel: warning - timestamp: false path: [] blueprint: api-description.apib endpoint: "http://127.0.0.1:3000" diff --git a/docs/usage-js.rst b/docs/usage-js.rst index 06f6c1607..b294d0d0a 100644 --- a/docs/usage-js.rst +++ b/docs/usage-js.rst @@ -56,8 +56,7 @@ Let’s have a look at an example configuration first. (Please also see the :ref 'require': null, // String, When using nodejs hooks, require the given module before executing hooks - 'color': true, - 'timestamp': false + 'no-color': false, }, 'emitter': EventEmitterInstance, // optional - listen to test progress, your own instance of EventEmitter diff --git a/lib/configuration.js b/lib/configuration.js index 610441e4b..8c12de9e5 100644 --- a/lib/configuration.js +++ b/lib/configuration.js @@ -18,9 +18,8 @@ function coerceToArray(value) { function applyLoggingOptions(options) { - // The CLI layer should handle 'color' and 'timestamp' and they should be - // implemented as --color/--no-color and --timestamp/--no-timestamp boolean - // options with no values + // The CLI layer should handle 'color' and it should be implemented + // as a --color/--no-color boolean option with no values if (options.color === 'false') { options.color = false; } else if (options.color === 'true') { @@ -29,9 +28,6 @@ function applyLoggingOptions(options) { logger.transports.console.colorize = options.color; reporterOutputLogger.transports.console.colorize = options.color; - logger.transports.console.timestamp = options.timestamp; - reporterOutputLogger.transports.console.timestamp = options.timestamp; - // Handling the 'loglevel' value if (options.loglevel) { const loglevel = options.loglevel.toLowerCase(); @@ -39,7 +35,10 @@ function applyLoggingOptions(options) { logger.transports.console.silent = true; } else if (loglevel === 'warning') { logger.transports.console.level = 'warn'; - } else if (['debug', 'warn', 'error'].includes(loglevel)) { + } else if (loglevel === 'debug') { + logger.transports.console.level = 'debug'; + logger.transports.console.timestamp = true; + } else if (['warn', 'error'].includes(loglevel)) { logger.transports.console.level = loglevel; } else { throw new Error(`Unsupported logging level: '${loglevel}'`); @@ -48,11 +47,6 @@ function applyLoggingOptions(options) { logger.transports.console.level = 'warn'; } - // Once the reporters' output is handled in a different way than with - // a logger, this part will not be necessary anymore - reporterOutputLogger.transports.console.silent = false; - reporterOutputLogger.transports.console.level = 'info'; - return options; } @@ -78,7 +72,6 @@ function applyConfiguration(config) { only: [], color: true, loglevel: 'warning', - timestamp: false, sorted: false, names: false, hookfiles: null, diff --git a/lib/options.json b/lib/options.json index 6ac1667d0..4d3009c82 100644 --- a/lib/options.json +++ b/lib/options.json @@ -94,14 +94,9 @@ }, "loglevel": { "alias": "l", - "description": "The level of logging to output. Options: 'debug', 'warning', 'error', 'silent'.", + "description": "Application logging level. Supported levels: 'debug', 'warning', 'error', 'silent'. The value 'debug' also displays timestamps.", "default": "warning" }, - "timestamp": { - "alias": "t", - "description": "Determines whether console output should include timestamps.", - "default": false - }, "path": { "alias": "p", "description": "Additional API description paths or URLs. Can be used multiple times with glob pattern for paths.", diff --git a/lib/reporters/reporterOutputLogger.js b/lib/reporters/reporterOutputLogger.js index 0cd806d75..93b8cf03c 100644 --- a/lib/reporters/reporterOutputLogger.js +++ b/lib/reporters/reporterOutputLogger.js @@ -2,7 +2,7 @@ const winston = require('winston'); module.exports = new (winston.Logger)({ transports: [ - new (winston.transports.Console)({ colorize: true }), + new (winston.transports.Console)({ colorize: true, level: 'info' }), ], levels: { info: 10, diff --git a/test/integration/cli/hookfiles-cli-test.js b/test/integration/cli/hookfiles-cli-test.js index 4126357e6..b65f6f95c 100644 --- a/test/integration/cli/hookfiles-cli-test.js +++ b/test/integration/cli/hookfiles-cli-test.js @@ -557,7 +557,7 @@ describe('CLI', () => { }); }); - describe('when showing timestamps with -t', () => { + describe('when showing timestamps with --loglevel=debug', () => { let runtimeInfo; before((done) => { @@ -567,7 +567,7 @@ describe('CLI', () => { const args = [ './test/fixtures/single-get.apib', `http://127.0.0.1:${DEFAULT_SERVER_PORT}`, - '-t', + '--loglevel=debug', ]; runCLIWithServer(args, app, (err, info) => { runtimeInfo = info; @@ -577,7 +577,7 @@ describe('CLI', () => { it('should display timestamps', () => { // Look for the prefix for cli output with timestamps - assert.notEqual(runtimeInfo.dredd.stdout.indexOf('Z -'), -1); + assert.include(runtimeInfo.dredd.stderr, 'Z -'); }); }); }); diff --git a/test/unit/CLI-test.js b/test/unit/CLI-test.js index 75e479831..10ef76b05 100644 --- a/test/unit/CLI-test.js +++ b/test/unit/CLI-test.js @@ -327,7 +327,6 @@ describe('CLI class', () => { method: [], color: true, loglevel: 'warning', - timestamp: false, path: [], $0: 'node ./bin/dredd', })); diff --git a/test/unit/configUtils-test.js b/test/unit/configUtils-test.js index a04b490e5..96182bc70 100644 --- a/test/unit/configUtils-test.js +++ b/test/unit/configUtils-test.js @@ -51,8 +51,6 @@ const argvData = { c: true, loglevel: 'warning', l: 'warning', - timestamp: false, - t: false, path: [], p: [], $0: 'node ./bin/dredd', @@ -162,7 +160,6 @@ details: false method: [] color: true loglevel: info -timestamp: false path: [] blueprint: blueprint endpoint: endpoint\ diff --git a/test/unit/configuration-test.js b/test/unit/configuration-test.js index 031f80d2a..78480dbbc 100644 --- a/test/unit/configuration-test.js +++ b/test/unit/configuration-test.js @@ -99,34 +99,6 @@ describe('configuration.applyLoggingOptions()', () => { }); }); - describe('with timestamp set to true', () => { - beforeEach(() => { - configuration.applyLoggingOptions({ timestamp: true }); - }); - - it('the application logger should be set to add timestamps', () => { - assert.isTrue(logger.transports.console.timestamp); - }); - - it('the application output should be set to add timestamps', () => { - assert.isTrue(reporterOutputLogger.transports.console.timestamp); - }); - }); - - describe('with timestamp set to false', () => { - beforeEach(() => { - configuration.applyLoggingOptions({ timestamp: false }); - }); - - it('the application logger should be set not to add timestamps', () => { - assert.isFalse(logger.transports.console.timestamp); - }); - - it('the application output should be set not to add timestamps', () => { - assert.isFalse(reporterOutputLogger.transports.console.timestamp); - }); - }); - describe('with loglevel not set', () => { beforeEach(() => { configuration.applyLoggingOptions({}); @@ -162,7 +134,7 @@ describe('configuration.applyLoggingOptions()', () => { configuration.applyLoggingOptions({ loglevel: 'ERROR' }); }); - it('the application logger level is set', () => { + it('the value is understood', () => { assert.equal(logger.transports.console.level, 'error'); }); }); @@ -190,24 +162,28 @@ describe('configuration.applyLoggingOptions()', () => { }); }); - describe('with loglevel set to warn', () => { + describe('with loglevel set to warning', () => { beforeEach(() => { - configuration.applyLoggingOptions({ loglevel: 'warn' }); + configuration.applyLoggingOptions({ loglevel: 'warning' }); }); - it('the application logger level is set to warn', () => { + it('the value is understood as warn', () => { assert.equal(logger.transports.console.level, 'warn'); }); }); - describe('with loglevel set to warning', () => { + describe('with loglevel set to warn', () => { beforeEach(() => { - configuration.applyLoggingOptions({ loglevel: 'warning' }); + configuration.applyLoggingOptions({ loglevel: 'warn' }); }); it('the application logger level is set to warn', () => { assert.equal(logger.transports.console.level, 'warn'); }); + + it('the application logger is not set to add timestamps', () => { + assert.isFalse(logger.transports.console.timestamp); + }); }); describe('with loglevel set to error', () => { @@ -218,6 +194,10 @@ describe('configuration.applyLoggingOptions()', () => { it('the application logger level is set to error', () => { assert.equal(logger.transports.console.level, 'error'); }); + + it('the application logger is not set to add timestamps', () => { + assert.isFalse(logger.transports.console.timestamp); + }); }); describe('with loglevel set to debug', () => { @@ -228,6 +208,10 @@ describe('configuration.applyLoggingOptions()', () => { it('the application logger level is set to debug', () => { assert.equal(logger.transports.console.level, 'debug'); }); + + it('the application logger is set to add timestamps', () => { + assert.isTrue(logger.transports.console.timestamp); + }); }); });