diff --git a/packages/desktop-gui/src/styles/components/_alerts.scss b/packages/desktop-gui/src/styles/components/_alerts.scss index 27832af46d..5b7f75ae5f 100644 --- a/packages/desktop-gui/src/styles/components/_alerts.scss +++ b/packages/desktop-gui/src/styles/components/_alerts.scss @@ -56,6 +56,10 @@ .alert-content { text-align: left; + + p { + line-height: 1.5; + } } code, pre { diff --git a/packages/server/__snapshots__/3_plugins_spec.js b/packages/server/__snapshots__/3_plugins_spec.js index 03e4a60844..41b2bf1aaa 100644 --- a/packages/server/__snapshots__/3_plugins_spec.js +++ b/packages/server/__snapshots__/3_plugins_spec.js @@ -461,3 +461,21 @@ The following error was thrown by a plugin. We stopped running your tests becaus ` + +exports['e2e plugins projectRoot and configFile passes projectRoot and default configFile to plugins function 1'] = ` +The following validation error was thrown by your plugins file (\`/foo/bar/.projects/plugin-validation-error/cypress/plugins/index.js\`). + + Error: You must pass a valid event name when registering a plugin. + +You passed: \`invalid:event\` + +The following are valid events: +- file:preprocessor +- before:browser:launch +- task +- after:screenshot + + [stack trace lines] + + +` diff --git a/packages/server/lib/errors.coffee b/packages/server/lib/errors.coffee index fb54062aac..d5f22cb086 100644 --- a/packages/server/lib/errors.coffee +++ b/packages/server/lib/errors.coffee @@ -592,6 +592,11 @@ getMsgByType = (type, arg1 = {}, arg2, arg3) -> The following error was thrown by a plugin. We stopped running your tests because a plugin crashed. Please check your plugins file (`#{arg1}`) """.trim() return {msg: msg, details: arg2} + when "PLUGINS_VALIDATION_ERROR" + msg = """ + The following validation error was thrown by your plugins file (`#{arg1}`). + """.trim() + return {msg: msg, details: arg2} when "BUNDLE_ERROR" ## IF YOU MODIFY THIS MAKE SURE TO UPDATE ## THE ERROR MESSAGE IN THE RUNNER TOO diff --git a/packages/server/lib/plugins/child/run_plugins.js b/packages/server/lib/plugins/child/run_plugins.js index e3e3fcb020..e859d5ab02 100644 --- a/packages/server/lib/plugins/child/run_plugins.js +++ b/packages/server/lib/plugins/child/run_plugins.js @@ -4,10 +4,12 @@ const _ = require('lodash') const debug = require('debug')('cypress:server:plugins:child') const Promise = require('bluebird') + +const errors = require('../../errors') const preprocessor = require('./preprocessor') const task = require('./task') const util = require('../util') -const errors = require('../../errors') +const validateEvent = require('./validate_event') const ARRAY_METHODS = ['concat', 'push', 'unshift', 'slice', 'pop', 'shift', 'slice', 'splice', 'filter', 'map', 'forEach', 'reduce', 'reverse', 'splice', 'includes'] @@ -16,23 +18,9 @@ const registeredEvents = {} const invoke = (eventId, args = []) => { const event = registeredEvents[eventId] - if (!event) { - sendError(new Error(`No handler registered for event id ${eventId}`)) - - return - } - return event.handler(...args) } -const sendError = (ipc, err) => { - ipc.send('error', util.serializeError(err)) -} - -const sendWarning = (ipc, warningErr) => { - ipc.send('warning', util.serializeError(warningErr)) -} - let plugins const load = (ipc, config, pluginsFile) => { @@ -44,6 +32,14 @@ const load = (ipc, config, pluginsFile) => { // we track the register calls and then send them all at once // to the parent process const register = (event, handler) => { + const { isValid, error } = validateEvent(event, handler) + + if (!isValid) { + ipc.send('load:error', 'PLUGINS_VALIDATION_ERROR', pluginsFile, error.stack) + + return + } + if (event === 'task') { const existingEventId = _.findKey(registeredEvents, { event: 'task' }) @@ -74,12 +70,16 @@ const load = (ipc, config, pluginsFile) => { Promise .try(() => { + debug('run plugins function') + return plugins(register, config) }) .then((modifiedCfg) => { + debug('plugins file successfully loaded') ipc.send('loaded', modifiedCfg, registrations) }) .catch((err) => { + debug('plugins file errored:', err && err.stack) ipc.send('load:error', 'PLUGINS_FUNCTION_ERROR', pluginsFile, err.stack) }) } @@ -113,10 +113,9 @@ const execute = (ipc, event, ids, args = []) => { hasEmittedWarning = true - sendWarning(ipc, - errors.get( - 'DEPRECATED_BEFORE_BROWSER_LAUNCH_ARGS', - )) + const warning = errors.get('DEPRECATED_BEFORE_BROWSER_LAUNCH_ARGS') + + ipc.send('warning', util.serializeError(warning)) // eslint-disable-next-line prefer-rest-params return boundFn.apply(this, arguments) diff --git a/packages/server/lib/plugins/child/validate_event.js b/packages/server/lib/plugins/child/validate_event.js new file mode 100644 index 0000000000..4ad4caf075 --- /dev/null +++ b/packages/server/lib/plugins/child/validate_event.js @@ -0,0 +1,45 @@ +const _ = require('lodash') + +const createErrorResult = (errorMessage) => ({ isValid: false, error: new Error(errorMessage) }) +const createSuccessResult = () => ({ isValid: true }) + +const validate = (func, arg, errorMessage) => { + return func(arg) ? createSuccessResult() : createErrorResult(errorMessage) +} + +const isFunction = (event, handler) => { + return validate(_.isFunction, handler, `The handler for the event \`${event}\` must be a function`) +} + +const isObject = (event, handler) => { + return validate(_.isPlainObject, handler, `The handler for the event \`${event}\` must be an object`) +} + +const eventValidators = { + 'file:preprocessor': isFunction, + 'before:browser:launch': isFunction, + 'task': isObject, + 'after:screenshot': isFunction, + '_get:task:keys': isFunction, + '_get:task:body': isFunction, +} + +const validateEvent = (event, handler) => { + const validator = eventValidators[event] + + if (!validator) { + const userEvents = _.reject(_.keys(eventValidators), (event) => event.startsWith('_')) + + return createErrorResult(`You must pass a valid event name when registering a plugin. + +You passed: \`${event}\` + +The following are valid events: +- ${userEvents.join('\n- ')} +`) + } + + return validator(event, handler) +} + +module.exports = validateEvent diff --git a/packages/server/lib/server.coffee b/packages/server/lib/server.coffee index 4e859fee39..338a5c0c01 100644 --- a/packages/server/lib/server.coffee +++ b/packages/server/lib/server.coffee @@ -164,6 +164,7 @@ class Server open: (config = {}, project, onError, onWarning) -> debug("server open") + la(_.isPlainObject(config), "expected plain config object", config) Promise.try => diff --git a/packages/server/test/e2e/3_plugins_spec.js b/packages/server/test/e2e/3_plugins_spec.js index 727813810e..9628c92e0e 100644 --- a/packages/server/test/e2e/3_plugins_spec.js +++ b/packages/server/test/e2e/3_plugins_spec.js @@ -15,6 +15,7 @@ const pluginAfterScreenshot = Fixtures.projectPath('plugin-after-screenshot') const pluginReturnsBadConfig = Fixtures.projectPath('plugin-returns-bad-config') const pluginReturnsEmptyBrowsersList = Fixtures.projectPath('plugin-returns-empty-browsers-list') const pluginReturnsInvalidBrowser = Fixtures.projectPath('plugin-returns-invalid-browser') +const pluginValidationError = Fixtures.projectPath('plugin-validation-error') describe('e2e plugins', function () { e2e.setup() @@ -134,6 +135,16 @@ describe('e2e plugins', function () { }) }) + it('fails when invalid event is registered', function () { + e2e.exec(this, { + spec: 'app_spec.js', + project: pluginValidationError, + sanitizeScreenshotDimensions: true, + snapshot: true, + expectedExitCode: 1, + }) + }) + describe('projectRoot and configFile', function () { it('passes projectRoot and default configFile to plugins function', function () { return e2e.exec(this, { diff --git a/packages/server/test/support/fixtures/projects/plugin-before-browser-launch-deprecation/cypress/plugins/index.js b/packages/server/test/support/fixtures/projects/plugin-before-browser-launch-deprecation/cypress/plugins/index.js index 96ea549772..a52576c6c3 100644 --- a/packages/server/test/support/fixtures/projects/plugin-before-browser-launch-deprecation/cypress/plugins/index.js +++ b/packages/server/test/support/fixtures/projects/plugin-before-browser-launch-deprecation/cypress/plugins/index.js @@ -44,7 +44,6 @@ const getHandlersByType = (type) => { return launchOptions }, onTask: { assertPsOutput: assertPsOutput('--foo') }, - } case 'return-launch-options-mutate-only-args-property': @@ -57,7 +56,6 @@ const getHandlersByType = (type) => { return launchOptions }, onTask: { assertPsOutput: assertPsOutput(['--foo', '--bar']) }, - } case 'return-undefined-mutate-array': @@ -70,7 +68,6 @@ const getHandlersByType = (type) => { return }, onTask: { assertPsOutput: assertPsOutput([]) }, - } case 'return-unknown-properties': @@ -83,6 +80,7 @@ const getHandlersByType = (type) => { return launchOptions }, + onTask: {}, } case 'throw-explicit-error': @@ -90,6 +88,7 @@ const getHandlersByType = (type) => { onBeforeBrowserLaunch (browser, launchOptions) { throw new Error('Error thrown from plugins handler') }, + onTask: {}, } case 'reject-promise': @@ -101,6 +100,7 @@ const getHandlersByType = (type) => { throw new Error('Promise rejected from plugins handler') }) }, + onTask: {}, } default: () => { diff --git a/packages/server/test/support/fixtures/projects/plugin-validation-error/cypress.json b/packages/server/test/support/fixtures/projects/plugin-validation-error/cypress.json new file mode 100644 index 0000000000..0967ef424b --- /dev/null +++ b/packages/server/test/support/fixtures/projects/plugin-validation-error/cypress.json @@ -0,0 +1 @@ +{} diff --git a/packages/server/test/support/fixtures/projects/plugin-validation-error/cypress/integration/app_spec.js b/packages/server/test/support/fixtures/projects/plugin-validation-error/cypress/integration/app_spec.js new file mode 100644 index 0000000000..99a13400ed --- /dev/null +++ b/packages/server/test/support/fixtures/projects/plugin-validation-error/cypress/integration/app_spec.js @@ -0,0 +1,3 @@ +it('passes', () => { + expect(true).to.be.true +}) diff --git a/packages/server/test/support/fixtures/projects/plugin-validation-error/cypress/plugins/index.js b/packages/server/test/support/fixtures/projects/plugin-validation-error/cypress/plugins/index.js new file mode 100644 index 0000000000..5777196b75 --- /dev/null +++ b/packages/server/test/support/fixtures/projects/plugin-validation-error/cypress/plugins/index.js @@ -0,0 +1,3 @@ +module.exports = (on) => { + on('invalid:event', () => {}) +} diff --git a/packages/server/test/unit/plugins/child/validate_event_spec.js b/packages/server/test/unit/plugins/child/validate_event_spec.js new file mode 100644 index 0000000000..86d7e7142e --- /dev/null +++ b/packages/server/test/unit/plugins/child/validate_event_spec.js @@ -0,0 +1,69 @@ +require('../../../spec_helper') + +const _ = require('lodash') +const validateEvent = require('../../../../lib/plugins/child/validate_event') + +const events = [ + ['file:preprocessor', 'a function', () => {}], + ['before:browser:launch', 'a function', () => {}], + ['after:screenshot', 'a function', () => {}], + ['task', 'an object', {}], +] + +describe('lib/plugins/child/validate_event', () => { + it('returns error when called with no event name', () => { + const { isValid, error } = validateEvent() + + expect(isValid).to.be.false + expect(error.message).to.equal(`You must pass a valid event name when registering a plugin. + +You passed: \`undefined\` + +The following are valid events: +- file:preprocessor +- before:browser:launch +- task +- after:screenshot +`) + }) + + it('returns error when called with no event handler', () => { + const { isValid, error } = validateEvent('file:preprocessor') + + expect(isValid).to.be.false + expect(error.message).to.equal('The handler for the event `file:preprocessor` must be a function') + }) + + it('returns error when called with unsupported event name', () => { + const { isValid, error } = validateEvent('invalid:event:name', {}) + + expect(isValid).to.be.false + expect(error.message).to.equal(`You must pass a valid event name when registering a plugin. + +You passed: \`invalid:event:name\` + +The following are valid events: +- file:preprocessor +- before:browser:launch +- task +- after:screenshot +`) + }) + + _.each(events, ([event, type]) => { + it(`returns error when event handler of ${event} is not ${type}`, () => { + const { isValid, error } = validateEvent(event, 'invalid type') + + expect(isValid).to.be.false + expect(error.message).to.equal(`The handler for the event \`${event}\` must be ${type}`) + }) + }) + + _.each(events, ([event, type, validValue]) => { + it(`returns success when event handler of ${event} is ${type}`, () => { + const { isValid } = validateEvent(event, validValue) + + expect(isValid).to.be.true + }) + }) +})