Skip to content

Commit

Permalink
refactor: remove the --timestamp option
Browse files Browse the repository at this point in the history
BREAKING CHANGE: The --timestamp/-t option is no longer available. Instead, the --loglevel=debug automatically displays timestamps as well for the application logging entries.
  • Loading branch information
honzajavorek committed Feb 1, 2019
1 parent 2f8f6b6 commit 8eb265b
Show file tree
Hide file tree
Showing 9 changed files with 30 additions and 64 deletions.
1 change: 0 additions & 1 deletion docs/usage-cli.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
3 changes: 1 addition & 2 deletions docs/usage-js.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 6 additions & 13 deletions lib/configuration.js
Original file line number Diff line number Diff line change
Expand Up @@ -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') {
Expand All @@ -29,17 +28,17 @@ 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();
if (loglevel === 'silent') {
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}'`);
Expand All @@ -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;
}

Expand All @@ -78,7 +72,6 @@ function applyConfiguration(config) {
only: [],
color: true,
loglevel: 'warning',
timestamp: false,
sorted: false,
names: false,
hookfiles: null,
Expand Down
7 changes: 1 addition & 6 deletions lib/options.json
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down
2 changes: 1 addition & 1 deletion lib/reporters/reporterOutputLogger.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions test/integration/cli/hookfiles-cli-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -557,7 +557,7 @@ describe('CLI', () => {
});
});

describe('when showing timestamps with -t', () => {
describe('when showing timestamps with --loglevel=debug', () => {
let runtimeInfo;

before((done) => {
Expand All @@ -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;
Expand All @@ -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 -');
});
});
});
Expand Down
1 change: 0 additions & 1 deletion test/unit/CLI-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,6 @@ describe('CLI class', () => {
method: [],
color: true,
loglevel: 'warning',
timestamp: false,
path: [],
$0: 'node ./bin/dredd',
}));
Expand Down
3 changes: 0 additions & 3 deletions test/unit/configUtils-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@ const argvData = {
c: true,
loglevel: 'warning',
l: 'warning',
timestamp: false,
t: false,
path: [],
p: [],
$0: 'node ./bin/dredd',
Expand Down Expand Up @@ -162,7 +160,6 @@ details: false
method: []
color: true
loglevel: info
timestamp: false
path: []
blueprint: blueprint
endpoint: endpoint\
Expand Down
52 changes: 18 additions & 34 deletions test/unit/configuration-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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({});
Expand Down Expand Up @@ -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');
});
});
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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);
});
});
});

Expand Down

0 comments on commit 8eb265b

Please sign in to comment.