fix: clarify EPLUGINCONF error message

The message now specify if the step is required and if it allows to configure multiple plugins.
2018-07-29 23:28:33 -04:00 · 2018-07-29 23:28:33 -04:00 · d8c84a0e0b
commit d8c84a0e0b
parent 3cc62f0318
8 changed files with 116 additions and 116 deletions
--- 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).
--- 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)}\`.`,
  }),
--- 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}),
  },
 };
--- 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];
--- a/lib/plugins/utils.js
+++ 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};
--- 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));
--- 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');
 });
--- a/test/plugins/utils.test.js
+++ 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, () => {}));
+});