From d8c84a0e0bae0b78ede6d7f65e6ed1426ab78558 Mon Sep 17 00:00:00 2001 From: Pierre Vanduynslager Date: Sun, 29 Jul 2018 23:28:33 -0400 Subject: [PATCH] fix: clarify `EPLUGINCONF` error message The message now specify if the step is required and if it allows to configure multiple plugins. --- docs/usage/configuration.md | 2 +- lib/definitions/errors.js | 10 ++-- lib/definitions/plugins.js | 28 ++++++---- lib/plugins/index.js | 7 +-- lib/plugins/utils.js | 18 +++++++ test/definitions/plugins.test.js | 88 -------------------------------- test/plugins/plugins.test.js | 21 ++++---- test/plugins/utils.test.js | 58 +++++++++++++++++++++ 8 files changed, 116 insertions(+), 116 deletions(-) create mode 100644 lib/plugins/utils.js create mode 100644 test/plugins/utils.test.js diff --git a/docs/usage/configuration.md b/docs/usage/configuration.md index 3f549a78..1a6da217 100644 --- a/docs/usage/configuration.md +++ b/docs/usage/configuration.md @@ -143,10 +143,10 @@ See [Plugins configuration](plugins.md#configuration) for more details. ### analyzeCommits -Type: `String`, `Object` Default: `'@semantic-release/commit-analyzer'` +Type: `Array`, `String`, `Object` CLI argument: `--analyze-commits` Define the [analyze commits plugin](plugins.md#analyzecommits-plugin). diff --git a/lib/definitions/errors.js b/lib/definitions/errors.js index f5aa40bc..64aa1fa5 100644 --- a/lib/definitions/errors.js +++ b/lib/definitions/errors.js @@ -55,11 +55,13 @@ Your configuration for the \`tagFormat\` option is \`${stringify(tagFormat)}\`.` Your configuration for the \`tagFormat\` option is \`${stringify(tagFormat)}\`.`, }), - EPLUGINCONF: ({type, pluginConf}) => ({ + EPLUGINCONF: ({type, multiple, required, pluginConf}) => ({ message: `The \`${type}\` plugin configuration is invalid.`, - details: `The [${type} plugin configuration](${linkify( - `docs/usage/plugins.md#${toLower(type)}-plugin` - )}) if defined, must be a single or an array of plugins definition. A plugin definition is either a string or an object with a \`path\` property. + details: `The [${type} plugin configuration](${linkify(`docs/usage/plugins.md#${toLower(type)}-plugin`)}) ${ + required ? 'is required and ' : '' + }must be ${ + multiple ? 'a single or an array of plugins' : 'a single plugin' + } definition. A plugin definition is either a string or an object with a \`path\` property. Your configuration for the \`${type}\` plugin is \`${stringify(pluginConf)}\`.`, }), diff --git a/lib/definitions/plugins.js b/lib/definitions/plugins.js index 2c25f1d2..bd37c3a2 100644 --- a/lib/definitions/plugins.js +++ b/lib/definitions/plugins.js @@ -1,18 +1,18 @@ -const {isString, isFunction, isArray, isPlainObject} = require('lodash'); +const {isString, isPlainObject} = require('lodash'); const {gitHead} = require('../git'); const {RELEASE_TYPE, RELEASE_NOTES_SEPARATOR} = require('./constants'); -const validatePluginConfig = conf => isString(conf) || isString(conf.path) || isFunction(conf); - module.exports = { verifyConditions: { default: ['@semantic-release/npm', '@semantic-release/github'], - configValidator: conf => !conf || (isArray(conf) ? conf : [conf]).every(conf => validatePluginConfig(conf)), + multiple: true, + required: false, pipelineConfig: () => ({settleAll: true}), }, analyzeCommits: { default: '@semantic-release/commit-analyzer', - configValidator: conf => Boolean(conf) && validatePluginConfig(conf), + multiple: false, + required: true, outputValidator: output => !output || RELEASE_TYPE.includes(output), preprocess: ({commits, ...inputs}) => ({ ...inputs, @@ -22,12 +22,14 @@ module.exports = { }, verifyRelease: { default: false, - configValidator: conf => !conf || (isArray(conf) ? conf : [conf]).every(conf => validatePluginConfig(conf)), + multiple: true, + required: false, pipelineConfig: () => ({settleAll: true}), }, generateNotes: { default: ['@semantic-release/release-notes-generator'], - configValidator: conf => !conf || (isArray(conf) ? conf : [conf]).every(conf => validatePluginConfig(conf)), + multiple: true, + required: false, outputValidator: output => !output || isString(output), pipelineConfig: () => ({ getNextInput: ({nextRelease, ...context}, notes) => ({ @@ -42,7 +44,8 @@ module.exports = { }, prepare: { default: ['@semantic-release/npm'], - configValidator: conf => !conf || (isArray(conf) ? conf : [conf]).every(conf => validatePluginConfig(conf)), + multiple: true, + required: false, pipelineConfig: ({generateNotes}, logger) => ({ getNextInput: async context => { const newGitHead = await gitHead({cwd: context.cwd}); @@ -60,7 +63,8 @@ module.exports = { }, publish: { default: ['@semantic-release/npm', '@semantic-release/github'], - configValidator: conf => !conf || (isArray(conf) ? conf : [conf]).every(conf => validatePluginConfig(conf)), + multiple: true, + required: false, outputValidator: output => !output || isPlainObject(output), pipelineConfig: () => ({ // Add `nextRelease` and plugin properties to published release @@ -73,12 +77,14 @@ module.exports = { }, success: { default: ['@semantic-release/github'], - configValidator: conf => !conf || (isArray(conf) ? conf : [conf]).every(conf => validatePluginConfig(conf)), + multiple: true, + required: false, pipelineConfig: () => ({settleAll: true}), }, fail: { default: ['@semantic-release/github'], - configValidator: conf => !conf || (isArray(conf) ? conf : [conf]).every(conf => validatePluginConfig(conf)), + multiple: true, + required: false, pipelineConfig: () => ({settleAll: true}), }, }; diff --git a/lib/plugins/index.js b/lib/plugins/index.js index a1838a92..9abd0581 100644 --- a/lib/plugins/index.js +++ b/lib/plugins/index.js @@ -2,6 +2,7 @@ const {identity, isPlainObject, omit, castArray, isUndefined} = require('lodash' const AggregateError = require('aggregate-error'); const getError = require('../get-error'); const PLUGINS_DEFINITIONS = require('../definitions/plugins'); +const {validateConfig} = require('./utils'); const pipeline = require('./pipeline'); const normalize = require('./normalize'); @@ -11,7 +12,7 @@ module.exports = (context, pluginsPath) => { const plugins = Object.entries(PLUGINS_DEFINITIONS).reduce( ( plugins, - [type, {configValidator, default: def, pipelineConfig, postprocess = identity, preprocess = identity}] + [type, {multiple, required, default: def, pipelineConfig, postprocess = identity, preprocess = identity}] ) => { let pluginOpts; @@ -23,8 +24,8 @@ module.exports = (context, pluginsPath) => { if (isPlainObject(options[type]) && !options[type].path && defaultPaths.length === 1) { [options[type].path] = defaultPaths; } - if (configValidator && !configValidator(options[type])) { - errors.push(getError('EPLUGINCONF', {type, pluginConf: options[type]})); + if (!validateConfig({multiple, required}, options[type])) { + errors.push(getError('EPLUGINCONF', {type, multiple, required, pluginConf: options[type]})); return plugins; } pluginOpts = options[type]; diff --git a/lib/plugins/utils.js b/lib/plugins/utils.js new file mode 100644 index 00000000..1ece5627 --- /dev/null +++ b/lib/plugins/utils.js @@ -0,0 +1,18 @@ +const {isString, isFunction, castArray} = require('lodash'); + +const validateSingleConfig = conf => { + conf = castArray(conf); + return conf.length === 1 && (isString(conf[0]) || isString(conf[0].path) || isFunction(conf[0])); +}; + +const validateMultipleConfig = conf => castArray(conf).every(conf => validateSingleConfig(conf)); + +const validateConfig = ({multiple, required}, conf) => { + conf = castArray(conf).filter(Boolean); + if (required) { + return Boolean(conf) && conf.length >= 1 && (multiple ? validateMultipleConfig : validateSingleConfig)(conf); + } + return conf.length === 0 || (multiple ? validateMultipleConfig : validateSingleConfig)(conf); +}; + +module.exports = {validateConfig}; diff --git a/test/definitions/plugins.test.js b/test/definitions/plugins.test.js index eedda95e..1ca6b3cc 100644 --- a/test/definitions/plugins.test.js +++ b/test/definitions/plugins.test.js @@ -2,94 +2,6 @@ import test from 'ava'; import plugins from '../../lib/definitions/plugins'; import {RELEASE_NOTES_SEPARATOR} from '../../lib/definitions/constants'; -test('The "verifyConditions" plugin, if defined, must be a single or an array of plugins definition', t => { - t.false(plugins.verifyConditions.configValidator({})); - t.false(plugins.verifyConditions.configValidator({path: null})); - - t.true(plugins.verifyConditions.configValidator({path: 'plugin-path.js'})); - t.true(plugins.verifyConditions.configValidator()); - t.true(plugins.verifyConditions.configValidator('plugin-path.js')); - t.true(plugins.verifyConditions.configValidator(() => {})); - t.true(plugins.verifyConditions.configValidator([{path: 'plugin-path.js'}, 'plugin-path.js', () => {}])); -}); - -test('The "analyzeCommits" plugin is mandatory, and must be a single plugin definition', t => { - t.false(plugins.analyzeCommits.configValidator({})); - t.false(plugins.analyzeCommits.configValidator({path: null})); - t.false(plugins.analyzeCommits.configValidator([])); - t.false(plugins.analyzeCommits.configValidator()); - - t.true(plugins.analyzeCommits.configValidator({path: 'plugin-path.js'})); - t.true(plugins.analyzeCommits.configValidator('plugin-path.js')); - t.true(plugins.analyzeCommits.configValidator(() => {})); -}); - -test('The "verifyRelease" plugin, if defined, must be a single or an array of plugins definition', t => { - t.false(plugins.verifyRelease.configValidator({})); - t.false(plugins.verifyRelease.configValidator({path: null})); - - t.true(plugins.verifyRelease.configValidator({path: 'plugin-path.js'})); - t.true(plugins.verifyRelease.configValidator()); - t.true(plugins.verifyRelease.configValidator('plugin-path.js')); - t.true(plugins.verifyRelease.configValidator(() => {})); - t.true(plugins.verifyRelease.configValidator([{path: 'plugin-path.js'}, 'plugin-path.js', () => {}])); -}); - -test('The "generateNotes" plugin, if defined, must be a single or an array of plugins definition', t => { - t.false(plugins.generateNotes.configValidator({})); - t.false(plugins.generateNotes.configValidator({path: null})); - - t.true(plugins.generateNotes.configValidator({path: 'plugin-path.js'})); - t.true(plugins.generateNotes.configValidator()); - t.true(plugins.generateNotes.configValidator('plugin-path.js')); - t.true(plugins.generateNotes.configValidator(() => {})); - t.true(plugins.generateNotes.configValidator([{path: 'plugin-path.js'}, 'plugin-path.js', () => {}])); -}); - -test('The "prepare" plugin, if defined, must be a single or an array of plugins definition', t => { - t.false(plugins.verifyRelease.configValidator({})); - t.false(plugins.verifyRelease.configValidator({path: null})); - - t.true(plugins.verifyRelease.configValidator({path: 'plugin-path.js'})); - t.true(plugins.verifyRelease.configValidator()); - t.true(plugins.verifyRelease.configValidator('plugin-path.js')); - t.true(plugins.verifyRelease.configValidator(() => {})); - t.true(plugins.verifyRelease.configValidator([{path: 'plugin-path.js'}, 'plugin-path.js', () => {}])); -}); - -test('The "publish" plugin is mandatory, and must be a single or an array of plugins definition', t => { - t.false(plugins.publish.configValidator({})); - t.false(plugins.publish.configValidator({path: null})); - - t.true(plugins.publish.configValidator({path: 'plugin-path.js'})); - t.true(plugins.publish.configValidator()); - t.true(plugins.publish.configValidator('plugin-path.js')); - t.true(plugins.publish.configValidator(() => {})); - t.true(plugins.publish.configValidator([{path: 'plugin-path.js'}, 'plugin-path.js', () => {}])); -}); - -test('The "success" plugin, if defined, must be a single or an array of plugins definition', t => { - t.false(plugins.success.configValidator({})); - t.false(plugins.success.configValidator({path: null})); - - t.true(plugins.success.configValidator({path: 'plugin-path.js'})); - t.true(plugins.success.configValidator()); - t.true(plugins.success.configValidator('plugin-path.js')); - t.true(plugins.success.configValidator(() => {})); - t.true(plugins.success.configValidator([{path: 'plugin-path.js'}, 'plugin-path.js', () => {}])); -}); - -test('The "fail" plugin, if defined, must be a single or an array of plugins definition', t => { - t.false(plugins.fail.configValidator({})); - t.false(plugins.fail.configValidator({path: null})); - - t.true(plugins.fail.configValidator({path: 'plugin-path.js'})); - t.true(plugins.fail.configValidator()); - t.true(plugins.fail.configValidator('plugin-path.js')); - t.true(plugins.fail.configValidator(() => {})); - t.true(plugins.fail.configValidator([{path: 'plugin-path.js'}, 'plugin-path.js', () => {}])); -}); - test('The "analyzeCommits" plugin output must be either undefined or a valid semver release type', t => { t.false(plugins.analyzeCommits.outputValidator('invalid')); t.false(plugins.analyzeCommits.outputValidator(1)); diff --git a/test/plugins/plugins.test.js b/test/plugins/plugins.test.js index eac70576..90059645 100644 --- a/test/plugins/plugins.test.js +++ b/test/plugins/plugins.test.js @@ -159,18 +159,15 @@ test('Merge global options with plugin options', async t => { t.deepEqual(result.pluginConfig, {localOpt: 'local', globalOpt: 'global', otherOpt: 'locally-defined'}); }); -test('Throw an error if plugins configuration are missing a path for plugin pipeline', t => { - const errors = [...t.throws(() => getPlugins({cwd, logger: t.context.logger, options: {verifyConditions: {}}}, {}))]; - - t.is(errors[0].name, 'SemanticReleaseError'); - t.is(errors[0].code, 'EPLUGINCONF'); -}); - -test('Throw an error if an array of plugin configuration is missing a path for plugin pipeline', t => { +test('Throw an error if plugins configuration are invalid', t => { const errors = [ ...t.throws(() => getPlugins( - {cwd, logger: t.context.logger, options: {verifyConditions: [{path: '@semantic-release/npm'}, {}]}}, + { + cwd, + logger: t.context.logger, + options: {verifyConditions: {}, analyzeCommits: [], verifyRelease: [{}], generateNotes: [{path: null}]}, + }, {} ) ), @@ -178,4 +175,10 @@ test('Throw an error if an array of plugin configuration is missing a path for p t.is(errors[0].name, 'SemanticReleaseError'); t.is(errors[0].code, 'EPLUGINCONF'); + t.is(errors[1].name, 'SemanticReleaseError'); + t.is(errors[1].code, 'EPLUGINCONF'); + t.is(errors[2].name, 'SemanticReleaseError'); + t.is(errors[2].code, 'EPLUGINCONF'); + t.is(errors[3].name, 'SemanticReleaseError'); + t.is(errors[3].code, 'EPLUGINCONF'); }); diff --git a/test/plugins/utils.test.js b/test/plugins/utils.test.js new file mode 100644 index 00000000..9a1bf350 --- /dev/null +++ b/test/plugins/utils.test.js @@ -0,0 +1,58 @@ +import test from 'ava'; +import {validateConfig} from '../../lib/plugins/utils'; + +test('Validate multiple/optional plugin configuration', t => { + const type = {multiple: true, required: false}; + t.false(validateConfig(type, {})); + t.false(validateConfig(type, {path: null})); + + t.true(validateConfig(type, {path: 'plugin-path.js'})); + t.true(validateConfig(type)); + t.true(validateConfig(type, 'plugin-path.js')); + t.true(validateConfig(type, ['plugin-path.js'])); + t.true(validateConfig(type, () => {})); + t.true(validateConfig(type, [{path: 'plugin-path.js'}, 'plugin-path.js', () => {}])); +}); + +test('Validate multiple/required plugin configuration', t => { + const type = {multiple: true, required: true}; + t.false(validateConfig(type, {})); + t.false(validateConfig(type, {path: null})); + t.false(validateConfig(type)); + + t.true(validateConfig(type, {path: 'plugin-path.js'})); + t.true(validateConfig(type, 'plugin-path.js')); + t.true(validateConfig(type, ['plugin-path.js'])); + t.true(validateConfig(type, () => {})); + t.true(validateConfig(type, [{path: 'plugin-path.js'}, 'plugin-path.js', () => {}])); +}); + +test('Validate single/required plugin configuration', t => { + const type = {multiple: false, required: true}; + + t.false(validateConfig(type, {})); + t.false(validateConfig(type, {path: null})); + t.false(validateConfig(type, [])); + t.false(validateConfig(type)); + t.false(validateConfig(type, [{path: 'plugin-path.js'}, 'plugin-path.js', () => {}])); + + t.true(validateConfig(type, {path: 'plugin-path.js'})); + t.true(validateConfig(type, 'plugin-path.js')); + t.true(validateConfig(type, ['plugin-path.js'])); + t.true(validateConfig(type, () => {})); +}); + +test('Validate single/optional plugin configuration', t => { + const type = {multiple: false, required: false}; + + t.false(validateConfig(type, {})); + t.false(validateConfig(type, {path: null})); + t.false(validateConfig(type, [{path: 'plugin-path.js'}, 'plugin-path.js', () => {}])); + + t.true(validateConfig(type)); + t.true(validateConfig(type, [])); + t.true(validateConfig(type, {path: 'plugin-path.js'})); + t.true(validateConfig(type, 'plugin-path.js')); + t.true(validateConfig(type, ['plugin-path.js'])); + t.true(validateConfig(type, () => {})); +});