From 9ec199f7ab6e1988654066dc00dd48c4930d5896 Mon Sep 17 00:00:00 2001 From: Honza Javorek Date: Tue, 5 Feb 2019 14:54:17 +0100 Subject: [PATCH] refactor: properly deprecate options --- lib/CLI.js | 28 +-- lib/configuration.js | 200 +++++++++++---- lib/options.json | 2 +- test/unit/configuration-test.js | 418 +++++++++++++++++++++++++++++++- 4 files changed, 575 insertions(+), 73 deletions(-) diff --git a/lib/CLI.js b/lib/CLI.js index 5df6ecd83..2372d2ecc 100644 --- a/lib/CLI.js +++ b/lib/CLI.js @@ -269,22 +269,22 @@ ${packageData.name} v${packageData.version} \ } run() { - for (const task of [ - this.setOptimistArgv, - this.parseCustomConfig, - this.runExitingActions, - this.loadDreddFile, - this.checkRequiredArgs, - this.moveBlueprintArgToPath, - ]) { - task.call(this); - if (this.finished) { return; } - } + try { + for (const task of [ + this.setOptimistArgv, + this.parseCustomConfig, + this.runExitingActions, + this.loadDreddFile, + this.checkRequiredArgs, + this.moveBlueprintArgToPath, + ]) { + task.call(this); + if (this.finished) { return; } + } - const configurationForDredd = this.initConfig(); - this.logDebuggingInfo(configurationForDredd); + const configurationForDredd = this.initConfig(); + this.logDebuggingInfo(configurationForDredd); - try { this.dreddInstance = this.initDredd(configurationForDredd); } catch (e) { this.exitWithStatus(e); diff --git a/lib/configuration.js b/lib/configuration.js index 66d5686e9..331c9263a 100644 --- a/lib/configuration.js +++ b/lib/configuration.js @@ -6,17 +6,29 @@ const reporterOutputLogger = require('./reporters/reporterOutputLogger'); function coerceToArray(value) { - if (Array.isArray(value)) { - return value; - } if (typeof value === 'string') { - return [value]; - } if ((value == null)) { - return []; - } + if (Array.isArray(value)) return value; + if (typeof value === 'string') return [value]; + if (value == null) return []; return value; } +function coerceToBoolean(value) { + if (value === 'true') return true; + if (value === 'false') return false; + if (value) return true; + return false; +} + + +function cloneConfig(config) { + const emitter = config.emitter; + const clonedConfig = clone(config); + if (emitter) clonedConfig.emitter = emitter; + return clonedConfig; +} + + function applyLoggingOptions(options) { if (options.color === false) { logger.transports.console.colorize = false; @@ -36,7 +48,9 @@ function applyLoggingOptions(options) { } else if (['warn', 'error'].includes(loglevel)) { logger.transports.console.level = loglevel; } else { - throw new Error(`Unsupported logging level: '${loglevel}'`); + logger.transports.console.level = 'warn'; + throw new Error(`The logging level '${loglevel}' is unsupported, ` + + 'supported are: silent, error, warning, debug'); } } else { logger.transports.console.level = 'warn'; @@ -44,8 +58,92 @@ function applyLoggingOptions(options) { } -function applyConfiguration(config) { - const configuration = { +function coerceRemovedOptions(config = {}) { + const coercedConfig = cloneConfig(config); + + const errors = []; + const warnings = []; + + if (config.options) { + if (config.options.c != null) { + warnings.push('DEPRECATED: Dredd does not support ' + + '-c anymore, use --color/--no-color instead'); + coercedConfig.options.color = coerceToBoolean(config.options.c); + delete coercedConfig.options.c; + } + if (typeof config.options.color === 'string') { + warnings.push('DEPRECATED: Dredd does not support ' + + `--color=${config.options.color} anymore, use --color/--no-color instead`); + coercedConfig.options.color = coerceToBoolean(config.options.color); + } + if (config.options.level) { + warnings.push('DEPRECATED: Dredd does not support ' + + '--level anymore, use --loglevel instead'); + } + if (config.options.level || config.options.l) { + const level = config.options.level || config.options.l; + if (!['silent', 'error', 'warning', 'debug'].includes(level)) { + warnings.push('DEPRECATED: Dredd does not support ' + + `'${level}' log level anymore, use 'silent', 'error', ` + + "'warning', or 'debug' instead"); + } + let loglevel; + if (['silly', 'debug', 'verbose'].includes(level)) { + loglevel = 'debug'; + } else if (level === 'error') { + loglevel = 'error'; + } else if (level === 'silent') { + loglevel = 'silent'; + } else { + loglevel = 'warn'; + } + coercedConfig.options.loglevel = loglevel; + coercedConfig.options.l = loglevel; + delete coercedConfig.options.level; + } + if (config.options.timestamp || config.options.t) { + warnings.push('DEPRECATED: Dredd does not support ' + + '--timestamp anymore, use --loglevel=debug instead'); + coercedConfig.options.loglevel = 'debug'; + coercedConfig.options.l = 'debug'; + delete coercedConfig.options.timestamp; + delete coercedConfig.options.t; + } + if (config.options.silent || config.options.q) { + warnings.push('DEPRECATED: Dredd does not support ' + + '-q/--silent anymore, use --loglevel=silent instead'); + coercedConfig.options.loglevel = 'silent'; + coercedConfig.options.l = 'silent'; + delete coercedConfig.options.silent; + delete coercedConfig.options.q; + } + if (config.options.sandbox || config.options.b) { + errors.push('REMOVED: Dredd does not support ' + + 'sandboxed JS hooks anymore, use standard JS hooks instead'); + delete coercedConfig.options.sandbox; + delete coercedConfig.options.b; + } + } + if (config.hooksData) { + errors.push('REMOVED: Dredd does not support ' + + 'sandboxed JS hooks anymore, use standard JS hooks instead'); + delete coercedConfig.hooksData; + } + if (config.blueprintPath) { + warnings.push('DEPRECATED: Dredd does not support ' + + "the 'blueprintPath' option anymore, use 'path' instead"); + coercedConfig.options = coercedConfig.options || {}; + coercedConfig.options.path = coerceToArray(coercedConfig.path); + coercedConfig.options.path.push(config.blueprintPath); + delete coercedConfig.blueprintPath; + } + + return { config: coercedConfig, errors, warnings }; +} + + +function applyConfiguration(inConfig) { + const outConfig = { server: null, emitter: new EventEmitter(), custom: { // Used for custom settings of various APIs or reporters @@ -56,7 +154,6 @@ function applyConfiguration(config) { 'dry-run': false, reporter: null, output: null, - debug: false, header: null, user: null, 'inline-errors': false, @@ -80,56 +177,59 @@ function applyConfiguration(config) { }, }; - // Normalize options and config - for (const key of Object.keys(config || {})) { - // Copy anything except "custom" hash - const value = config[key]; - if (key !== 'custom') { - configuration[key] = value; - } else { - if (!configuration.custom) { configuration.custom = {}; } - const object = config.custom || {}; - for (const customKey of Object.keys(object || {})) { - const customVal = object[customKey]; - configuration.custom[customKey] = clone(customVal, false); - } - } - } + // Gracefully deal with the removed options + const coerceResult = coerceRemovedOptions(inConfig); + const coercedInConfig = coerceResult.config; + + // Apply the values from the coerced config over the default ones + const custom = coercedInConfig.custom || {}; + Object.keys(custom) + .forEach((key) => { outConfig.custom[key] = clone(custom[key], false); }); + + const options = coercedInConfig.options || {}; + Object.keys(options) + .forEach((key) => { outConfig.options[key] = options[key]; }); + + Object.keys(coercedInConfig) + .filter(key => !['custom', 'options'].includes(key)) + .forEach((key) => { outConfig[key] = coercedInConfig[key]; }); // Coerce single/multiple options into an array - configuration.options.reporter = coerceToArray(configuration.options.reporter); - configuration.options.output = coerceToArray(configuration.options.output); - configuration.options.header = coerceToArray(configuration.options.header); - configuration.options.method = coerceToArray(configuration.options.method); - configuration.options.only = coerceToArray(configuration.options.only); - configuration.options.path = coerceToArray(configuration.options.path); - - configuration.options.method = configuration.options.method.map(method => method.toUpperCase()); - - if (configuration.options.user) { - const authHeader = `Authorization:Basic ${Buffer.from(configuration.options.user).toString('base64')}`; - configuration.options.header.push(authHeader); + outConfig.options.reporter = coerceToArray(outConfig.options.reporter); + outConfig.options.output = coerceToArray(outConfig.options.output); + outConfig.options.header = coerceToArray(outConfig.options.header); + outConfig.options.method = coerceToArray(outConfig.options.method); + outConfig.options.only = coerceToArray(outConfig.options.only); + outConfig.options.path = coerceToArray(outConfig.options.path); + + outConfig.options.method = outConfig.options.method + .map(method => method.toUpperCase()); + + if (outConfig.options.user) { + const token = Buffer.from(outConfig.options.user).toString('base64'); + outConfig.options.header.push(`Authorization: Basic ${token}`); + delete outConfig.options.user; } - applyLoggingOptions(configuration.options); + applyLoggingOptions(outConfig.options); - if (config) { - if (config.hooksData - || (config.options && (config.options.b || config.options.sandbox))) { - throw new Error('DEPRECATED: Dredd does not support ' - + 'sandboxed JS hooks anymore. Use standard JS hooks instead.'); - } - if (config.blueprintPath) { - throw new Error('DEPRECATED: Dredd does not support ' - + "the 'blueprintPath' option anymore. Use 'path' instead."); - } + // Log accumulated errors and warnings + coerceResult.errors.forEach(message => logger.error(message)); + coerceResult.warnings.forEach(message => logger.warn(message)); + + // Abort Dredd if there has been any errors + if (coerceResult.errors.length) { + throw new Error('Could not configure Dredd'); } - return configuration; + return outConfig; } module.exports = { applyConfiguration, applyLoggingOptions, + + // only for the purpose of unit tests + _coerceRemovedOptions: coerceRemovedOptions, }; diff --git a/lib/options.json b/lib/options.json index 931baa757..104112074 100644 --- a/lib/options.json +++ b/lib/options.json @@ -88,7 +88,7 @@ "default": [] }, "color": { - "description": "Use --no-color to suppress colors on the output", + "description": "Use --color/--no-color to enable/disable colored output", "boolean": true, "default": true }, diff --git a/test/unit/configuration-test.js b/test/unit/configuration-test.js index ce18ebfbb..800cdc646 100644 --- a/test/unit/configuration-test.js +++ b/test/unit/configuration-test.js @@ -27,7 +27,19 @@ describe('configuration.applyLoggingOptions()', () => { it('the application logger should be set to colorize', () => { assert.isTrue(logger.transports.console.colorize); }); + it('the application output should be set to colorize', () => { + assert.isTrue(reporterOutputLogger.transports.console.colorize); + }); + }); + describe('with color set to true', () => { + beforeEach(() => { + configuration.applyLoggingOptions({ color: true }); + }); + + it('the application logger should be set to colorize', () => { + assert.isTrue(logger.transports.console.colorize); + }); it('the application output should be set to colorize', () => { assert.isTrue(reporterOutputLogger.transports.console.colorize); }); @@ -41,7 +53,6 @@ describe('configuration.applyLoggingOptions()', () => { it('the application logger should be set not to colorize', () => { assert.isFalse(logger.transports.console.colorize); }); - it('the application output should be set not to colorize', () => { assert.isFalse(reporterOutputLogger.transports.console.colorize); }); @@ -55,7 +66,6 @@ describe('configuration.applyLoggingOptions()', () => { it('the application logger level is set to warn', () => { assert.equal(logger.transports.console.level, 'warn'); }); - it('the application output logger is not influenced', () => { assert.isFalse(reporterOutputLogger.transports.console.silent); assert.equal(reporterOutputLogger.transports.console.level, 'info'); @@ -70,7 +80,6 @@ describe('configuration.applyLoggingOptions()', () => { it('the application logger level is set', () => { assert.equal(logger.transports.console.level, 'error'); }); - it('the application output logger is not influenced', () => { assert.isFalse(reporterOutputLogger.transports.console.silent); assert.equal(reporterOutputLogger.transports.console.level, 'info'); @@ -91,7 +100,7 @@ describe('configuration.applyLoggingOptions()', () => { it('throws an exception', () => { assert.throws(() => { configuration.applyLoggingOptions({ loglevel: 'verbose' }); - }, /unsupported.+verbose/i); + }, /verbose.+unsupported/i); }); }); @@ -103,7 +112,6 @@ describe('configuration.applyLoggingOptions()', () => { it('the application logger gets silenced', () => { assert.isTrue(logger.transports.console.silent); }); - it('the application output logger is not influenced', () => { assert.isFalse(reporterOutputLogger.transports.console.silent); assert.equal(reporterOutputLogger.transports.console.level, 'info'); @@ -128,7 +136,6 @@ describe('configuration.applyLoggingOptions()', () => { 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); }); @@ -142,7 +149,6 @@ 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); }); @@ -156,9 +162,405 @@ 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); }); }); }); + + +describe('configuration._coerceRemovedOptions()', () => { + describe("with -c set to string 'true'", () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { c: 'true' }, + }); + }); + + it('gets coerced to color set to boolean true', () => { + assert.deepEqual(coerceResult.config, { options: { color: true } }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces one warning', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); + + describe("with -c set to string 'false'", () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { c: 'false' }, + }); + }); + + it('gets coerced to color set to boolean false', () => { + assert.deepEqual(coerceResult.config, { options: { color: false } }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces one warning', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); + + describe('with -c set to true', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { c: true }, + }); + }); + + it('gets coerced to color set to boolean true', () => { + assert.deepEqual(coerceResult.config, { options: { color: true } }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces one warning', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); + + describe('with -c set to false', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { c: false }, + }); + }); + + it('gets coerced to color set to boolean false', () => { + assert.deepEqual(coerceResult.config, { options: { color: false } }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces one warning', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); + + describe("with --color set to string 'true'", () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { color: 'true' }, + }); + }); + + it('gets coerced to color set to boolean true', () => { + assert.deepEqual(coerceResult.config, { options: { color: true } }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces one warning', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); + + describe("with --color set to string 'false'", () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { color: 'false' }, + }); + }); + + it('gets coerced to color set to boolean false', () => { + assert.deepEqual(coerceResult.config, { options: { color: false } }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces one warning', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); + + describe('with --color set to true', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { color: true }, + }); + }); + + it('gets coerced to color set to boolean true', () => { + assert.deepEqual(coerceResult.config, { options: { color: true } }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces no warnings', () => { + assert.deepEqual(coerceResult.warnings, []); + }); + }); + + describe('with --color set to false', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { color: false }, + }); + }); + + it('gets coerced to color set to boolean false', () => { + assert.deepEqual(coerceResult.config, { options: { color: false } }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces no warnings', () => { + assert.deepEqual(coerceResult.warnings, []); + }); + }); + + describe('with --level/-l set to a supported value', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { l: 'debug', level: 'debug' }, + }); + }); + + it('gets coerced to loglevel set to the value', () => { + assert.deepEqual(coerceResult.config, { + options: { l: 'debug', loglevel: 'debug' }, + }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces one warning', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); + + describe('with --level/-l set to a consolidated value', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { l: 'verbose', level: 'verbose' }, + }); + }); + + it('gets coerced to loglevel set to a corresponding value', () => { + assert.deepEqual(coerceResult.config, { + options: { l: 'debug', loglevel: 'debug' }, + }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces two warnings', () => { + assert.lengthOf(coerceResult.warnings, 2); + }); + }); + + describe('with --level/-l set to a removed value', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { l: 'complete', level: 'complete' }, + }); + }); + + it('gets coerced to loglevel set to the default value', () => { + assert.deepEqual(coerceResult.config, { + options: { l: 'warn', loglevel: 'warn' }, + }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces two warnings', () => { + assert.lengthOf(coerceResult.warnings, 2); + }); + }); + + describe("with -l set to 'silent'", () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { l: 'silent' }, + }); + }); + + it('gets coerced to loglevel set to silent', () => { + assert.deepEqual(coerceResult.config, { + options: { l: 'silent', loglevel: 'silent' }, + }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces no warnings', () => { + assert.deepEqual(coerceResult.warnings, []); + }); + }); + + describe('with --timestamp/-t set', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { timestamp: true, t: true }, + }); + }); + + it('gets coerced to loglevel set to debug', () => { + assert.deepEqual(coerceResult.config, { + options: { l: 'debug', loglevel: 'debug' }, + }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces one warning', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); + + describe('with --silent/-q set', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { silent: true, q: true }, + }); + }); + + it('gets coerced to loglevel set to silent', () => { + assert.deepEqual(coerceResult.config, { + options: { l: 'silent', loglevel: 'silent' }, + }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces one warning', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); + + describe('with --sandbox/-b set', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + options: { sandbox: true, b: true }, + }); + }); + + it('gets removed', () => { + assert.deepEqual(coerceResult.config, { options: {} }); + }); + it('produces one error', () => { + assert.lengthOf(coerceResult.errors, 1); + }); + it('produces no warnings', () => { + assert.deepEqual(coerceResult.warnings, []); + }); + }); + + describe('with hooksData set', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + hooksData: {}, + }); + }); + + it('gets removed', () => { + assert.deepEqual(coerceResult.config, {}); + }); + it('produces one error', () => { + assert.lengthOf(coerceResult.errors, 1); + }); + it('produces no warnings', () => { + assert.deepEqual(coerceResult.warnings, []); + }); + }); + + describe('with blueprintPath set and empty path', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + blueprintPath: 'foo/bar', + }); + }); + + it('gets reassigned as path', () => { + assert.deepEqual(coerceResult.config, { options: { path: ['foo/bar'] } }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces no warnings', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); + + describe('with blueprintPath set and path set to a string', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + blueprintPath: 'foo/bar', + options: { path: 'moo.js' }, + }); + }); + + it('gets reassigned as path', () => { + assert.deepEqual(coerceResult.config, { options: { path: ['moo.js', 'foo/bar'] } }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces no warnings', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); + + describe('with blueprintPath set and path set to an array', () => { + let coerceResult; + + before(() => { + coerceResult = configuration._coerceRemovedOptions({ + blueprintPath: 'foo/bar', + options: { path: ['moo.js'] }, + }); + }); + + it('gets reassigned as path', () => { + assert.deepEqual(coerceResult.config, { options: { path: ['foo/bar'] } }); + }); + it('produces no errors', () => { + assert.deepEqual(coerceResult.errors, []); + }); + it('produces no warnings', () => { + assert.lengthOf(coerceResult.warnings, 1); + }); + }); +});