From 00567a84c83d168a169970319081c8a15db5737e Mon Sep 17 00:00:00 2001 From: Christophe Porteneuve Date: Thu, 26 Jul 2018 15:24:49 +0200 Subject: [PATCH] refactor: rely on ValidationError for watch plugin key conflicts --- CHANGELOG.md | 2 +- docs/WatchPlugins.md | 8 +- packages/jest-cli/src/__tests__/watch.test.js | 77 +++++++------------ packages/jest-cli/src/watch.js | 27 ++++--- 4 files changed, 50 insertions(+), 64 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d0594d86167..30a2689a990f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ### Features -- `[jest-cli]` Watch plugin key registrations are now checked for conflicts. Some built-in keys, such as `p` and `t` for test limitation, are overridable, but most built-in keys are reserved (e.g. `q`, `a` or `o`), and any key registered by a third-party plugin cannot be overriden by another third-party plugin. Explicit error messages are logged to the console and Jest exits with code 64 (`EX_USAGE`). +- `[jest-cli]` Watch plugin key registrations are now checked for conflicts. Some built-in keys, such as `p` and `t` for test limitation, are overridable, but most built-in keys are reserved (e.g. `q`, `a` or `o`), and any key registered by a third-party plugin cannot be overriden by another third-party plugin. Explicit validation errors are then triggered. ### Fixes diff --git a/docs/WatchPlugins.md b/docs/WatchPlugins.md index aa08ba7faea4..026404e1200c 100644 --- a/docs/WatchPlugins.md +++ b/docs/WatchPlugins.md @@ -222,10 +222,10 @@ Any key not used by built-in functionality can be claimed, as you would expect. ### When a conflict happens -Should your plugin attempt to overwrite a reserved key, Jest will error out with exit code 64 (`EX_USAGE`) and a descriptive message, something like: +Should your plugin attempt to overwrite a reserved key, Jest will error out with a descriptive message, something like: -> Jest configuration error: watch plugin YourFaultyPlugin attempted to register key , that is reserved internally for quitting watch mode. Please change the configuration key for this plugin. +> Watch plugin YourFaultyPlugin attempted to register key , that is reserved internally for quitting watch mode. Please change the configuration key for this plugin. -Third-party plugins are also forbidden to overwrite a key reserved already by another third-party plugin present earlier in the configured plugins list (`watchPlugins` array setting). When this happens, you’ll also get a code-64 exit with an error message that tries to help you fix that: +Third-party plugins are also forbidden to overwrite a key reserved already by another third-party plugin present earlier in the configured plugins list (`watchPlugins` array setting). When this happens, you’ll also get an error message that tries to help you fix that: -> Jest configuration error: watch plugins YourFaultyPlugin and TheirFaultyPlugin both attempted to register key . Please change the key configuration for one of the conflicting plugins to avoid overlap. +> Watch plugins YourFaultyPlugin and TheirFaultyPlugin both attempted to register key . Please change the key configuration for one of the conflicting plugins to avoid overlap. diff --git a/packages/jest-cli/src/__tests__/watch.test.js b/packages/jest-cli/src/__tests__/watch.test.js index e0c9960bf401..6253c5f98106 100644 --- a/packages/jest-cli/src/__tests__/watch.test.js +++ b/packages/jest-cli/src/__tests__/watch.test.js @@ -12,7 +12,6 @@ import chalk from 'chalk'; import TestWatcher from '../TestWatcher'; import {JestHook, KEYS} from 'jest-watcher'; -const exitMock = jest.fn(); const runJestMock = jest.fn(); const watchPluginPath = `${__dirname}/__fixtures__/watch_plugin`; const watchPlugin2Path = `${__dirname}/__fixtures__/watch_plugin2`; @@ -44,7 +43,6 @@ jest.mock( ); jest.doMock('chalk', () => new chalk.constructor({enabled: false})); -jest.doMock('exit', () => exitMock); jest.doMock( '../runJest', () => @@ -98,7 +96,6 @@ const watch = require('../watch').default; const nextTick = () => new Promise(res => process.nextTick(res)); afterEach(runJestMock.mockReset); -afterEach(exitMock.mockReset); describe('Watch mode flows', () => { let pipe; @@ -368,14 +365,6 @@ describe('Watch mode flows', () => { }); describe('when dealing with potential watch plugin key conflicts', () => { - beforeEach(() => { - jest.spyOn(console, 'error'); - console.error.mockImplementation(() => {}); - }); - - afterEach(() => { - console.error.mockRestore(); - }); it.each` key | plugin @@ -404,27 +393,23 @@ describe('Watch mode flows', () => { {virtual: true}, ); - watch( - Object.assign({}, globalConfig, { - rootDir: __dirname, - watchPlugins: [{config: {}, path: pluginPath}], - }), - contexts, - pipe, - hasteMapInstances, - stdin, - ); - - expect(console.error).toHaveBeenLastCalledWith( - expect.stringMatching( - new RegExp( - `Jest configuration error: watch plugin OffendingWatchPlugin attempted to register key <${key}>, that is reserved internally for .+\\.\\s+Please change the configuration key for this plugin\\.`, - 'm', - ), + expect(() => { + watch( + Object.assign({}, globalConfig, { + rootDir: __dirname, + watchPlugins: [{config: {}, path: pluginPath}], + }), + contexts, + pipe, + hasteMapInstances, + stdin, + ); + }).toThrowError( + new RegExp( + `Watch plugin OffendingWatchPlugin attempted to register key <${key}>,\\s+that is reserved internally for .+\\.\\s+Please change the configuration key for this plugin\\.`, + 'm', ), ); - - expect(exitMock).toHaveBeenCalled(); }, ); @@ -466,8 +451,6 @@ describe('Watch mode flows', () => { hasteMapInstances, stdin, ); - - expect(exitMock).not.toHaveBeenCalled(); }, ); @@ -497,24 +480,20 @@ describe('Watch mode flows', () => { return pluginPath; }); - watch( - Object.assign({}, globalConfig, { - rootDir: __dirname, - watchPlugins: pluginPaths.map(path => ({config: {}, path})), - }), - contexts, - pipe, - hasteMapInstances, - stdin, - ); - - expect(console.error).toHaveBeenLastCalledWith( - expect.stringMatching( - /Jest configuration error: watch plugins OffendingFooThirdPartyWatchPlugin and OffendingBarThirdPartyWatchPlugin both attempted to register key \.\s+Please change the key configuration for one of the conflicting plugins to avoid overlap\./m, - ), + expect(() => { + watch( + Object.assign({}, globalConfig, { + rootDir: __dirname, + watchPlugins: pluginPaths.map(path => ({config: {}, path})), + }), + contexts, + pipe, + hasteMapInstances, + stdin, + ); + }).toThrowError( + /Watch plugins OffendingFooThirdPartyWatchPlugin and OffendingBarThirdPartyWatchPlugin both attempted to register key \.\s+Please change the key configuration for one of the conflicting plugins to avoid overlap\./m, ); - - expect(exitMock).toHaveBeenCalled(); }); }); diff --git a/packages/jest-cli/src/watch.js b/packages/jest-cli/src/watch.js index 2964ec832964..90e9f1212f83 100644 --- a/packages/jest-cli/src/watch.js +++ b/packages/jest-cli/src/watch.js @@ -37,6 +37,7 @@ import { getSortedUsageRows, filterInteractivePlugins, } from './lib/watch_plugins_helpers'; +import {ValidationError} from 'jest-validate'; import activeFilters from './lib/active_filters_message'; let hasExitListener = false; @@ -411,7 +412,6 @@ const checkForConflicts = (watchPluginKeys, plugin, globalConfig) => { } const conflictor = watchPluginKeys.get(key); - if (!conflictor || conflictor.overwritable) { watchPluginKeys.set(key, { overwritable: false, @@ -422,19 +422,26 @@ const checkForConflicts = (watchPluginKeys, plugin, globalConfig) => { let error; if (conflictor.forbiddenOverwriteMessage) { - error = `Jest configuration error: watch plugin ${getPluginIdentifier( - plugin, - )} attempted to register key <${key}>, that is reserved internally for ${ - conflictor.forbiddenOverwriteMessage - }. Please change the configuration key for this plugin.`; + error = ` + Watch plugin ${chalk.bold.red( + getPluginIdentifier(plugin), + )} attempted to register key ${chalk.bold.red(`<${key}>`)}, + that is reserved internally for ${chalk.bold.red( + conflictor.forbiddenOverwriteMessage, + )}. + Please change the configuration key for this plugin.`.trim(); } else { const plugins = [conflictor.plugin, plugin] - .map(getPluginIdentifier) + .map(p => chalk.bold.red(getPluginIdentifier(p))) .join(' and '); - error = `Jest configuration error: watch plugins ${plugins} both attempted to register key <${key}>. Please change the key configuration for one of the conflicting plugins to avoid overlap.`; + error = ` + Watch plugins ${plugins} both attempted to register key ${chalk.bold.red( + `<${key}>`, + )}. + Please change the key configuration for one of the conflicting plugins to avoid overlap.`.trim(); } - console.error('\n\n' + chalk.red(error)); - exit(64); // EX_USAGE + + throw new ValidationError('Watch plugin configuration error', error); }; const getPluginIdentifier = plugin =>