Skip to content

Commit

Permalink
refactor: rely on ValidationError for watch plugin key conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
tdd committed Jul 26, 2018
1 parent 3699e4e commit 00567a8
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 64 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
8 changes: 4 additions & 4 deletions docs/WatchPlugins.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <q>, that is reserved internally for quitting watch mode. Please change the configuration key for this plugin.
> Watch plugin YourFaultyPlugin attempted to register key <q>, 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 <x>. Please change the key configuration for one of the conflicting plugins to avoid overlap.
> Watch plugins YourFaultyPlugin and TheirFaultyPlugin both attempted to register key <x>. Please change the key configuration for one of the conflicting plugins to avoid overlap.
77 changes: 28 additions & 49 deletions packages/jest-cli/src/__tests__/watch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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`;
Expand Down Expand Up @@ -44,7 +43,6 @@ jest.mock(
);

jest.doMock('chalk', () => new chalk.constructor({enabled: false}));
jest.doMock('exit', () => exitMock);
jest.doMock(
'../runJest',
() =>
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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();
},
);

Expand Down Expand Up @@ -466,8 +451,6 @@ describe('Watch mode flows', () => {
hasteMapInstances,
stdin,
);

expect(exitMock).not.toHaveBeenCalled();
},
);

Expand Down Expand Up @@ -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();
});
});

Expand Down
27 changes: 17 additions & 10 deletions packages/jest-cli/src/watch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -411,7 +412,6 @@ const checkForConflicts = (watchPluginKeys, plugin, globalConfig) => {
}

const conflictor = watchPluginKeys.get(key);

if (!conflictor || conflictor.overwritable) {
watchPluginKeys.set(key, {
overwritable: false,
Expand All @@ -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 =>
Expand Down

0 comments on commit 00567a8

Please sign in to comment.