Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(jest-config): Throw an error instead of showing a warning if multiple configs are used #12510

Merged
merged 3 commits into from
Feb 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- `[jest-config, @jest/types]` Add `ci` to `GlobalConfig` ([#12378](https://github.com/facebook/jest/pull/12378))
- `[jest-config]` [**BREAKING**] Rename `moduleLoader` to `runtime` ([#10817](https://github.com/facebook/jest/pull/10817))
- `[jest-config]` [**BREAKING**] Rename `extraGlobals` to `sandboxInjectedGlobals` ([#10817](https://github.com/facebook/jest/pull/10817))
- `[jest-config]` [**BREAKING**] Throw an error instead of showing a warning if multiple configs are used ([#12510](https://github.com/facebook/jest/pull/12510))
- `[jest-core]` Pass project config to `globalSetup`/`globalTeardown` function as second argument ([#12440](https://github.com/facebook/jest/pull/12440))
- `[jest-environment-jsdom]` [**BREAKING**] Upgrade jsdom to 19.0.0 ([#12290](https://github.com/facebook/jest/pull/12290))
- `[jest-environment-jsdom]` [**BREAKING**] Add default `browser` condition to `exportConditions` for `jsdom` environment ([#11924](https://github.com/facebook/jest/pull/11924))
Expand Down
17 changes: 4 additions & 13 deletions e2e/__tests__/__snapshots__/multipleConfigs.ts.snap
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`multiple configs will warn 1`] = `
exports[`multiple configs will throw error 1`] = `
"● Multiple configurations found:

* <rootDir>/e2e/multiple-configs/jest.config.js
* <rootDir>/e2e/multiple-configs/jest.config.json
* \`jest\` key in <rootDir>/e2e/multiple-configs/package.json
Expand All @@ -10,16 +11,6 @@ exports[`multiple configs will warn 1`] = `
Either remove unused config files or select one explicitly with \`--config\`.

Configuration Documentation:
https://jestjs.io/docs/configuration.html

PASS Config from js file __tests__/test.js
✓ dummy test"
`;

exports[`multiple configs will warn 2`] = `
"Test Suites: 1 passed, 1 total
Tests: 1 passed, 1 total
Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites."
https://jestjs.io/docs/configuration
"
`;
16 changes: 5 additions & 11 deletions e2e/__tests__/multipleConfigs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,30 @@

import * as path from 'path';
import slash = require('slash');
import {extractSummary} from '../Utils';
import runJest from '../runJest';

const MULTIPLE_CONFIGS_WARNING_TEXT = 'Multiple configurations found';

test('multiple configs will warn', () => {
test('multiple configs will throw error', () => {
const rootDir = slash(path.resolve(__dirname, '../..'));
const {exitCode, stderr} = runJest('multiple-configs', [], {
skipPkgJsonCheck: true,
});

expect(exitCode).toBe(0);
expect(exitCode).toBe(1);
expect(stderr).toContain(MULTIPLE_CONFIGS_WARNING_TEXT);

const cleanStdErr = stderr.replace(new RegExp(rootDir, 'g'), '<rootDir>');
const {rest, summary} = extractSummary(cleanStdErr);

expect(rest).toMatchSnapshot();
expect(summary).toMatchSnapshot();
expect(cleanStdErr).toMatchSnapshot();
});

test('multiple configs warning can be suppressed by using --config', () => {
const {exitCode, stderr} = runJest(
test('multiple configs error can be suppressed by using --config', () => {
const {exitCode} = runJest(
'multiple-configs',
['--config', 'jest.config.json'],
{
skipPkgJsonCheck: true,
},
);

expect(exitCode).toBe(0);
expect(stderr).not.toContain(MULTIPLE_CONFIGS_WARNING_TEXT);
});
53 changes: 9 additions & 44 deletions packages/jest-config/src/__tests__/resolveConfigPath.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,6 @@ const ERROR_PATTERN = /Could not find a config file based on provided values/;
const NO_ROOT_DIR_ERROR_PATTERN = /Can't find a root directory/;
const MULTIPLE_CONFIGS_ERROR_PATTERN = /Multiple configurations found/;

const mockConsoleWarn = () => {
jest.spyOn(console, 'warn');
const mockedConsoleWarn = console.warn as jest.Mock<void, Array<any>>;

// We will mock console.warn because it would produce a lot of noise in the tests
mockedConsoleWarn.mockImplementation(() => {});

return mockedConsoleWarn;
};

beforeEach(() => cleanup(DIR));
afterEach(() => cleanup(DIR));

Expand Down Expand Up @@ -56,8 +46,6 @@ describe.each(JEST_CONFIG_EXT_ORDER.slice(0))(
});

test(`directory path with "${extension}"`, () => {
const mockedConsoleWarn = mockConsoleWarn();

const relativePackageJsonPath = 'a/b/c/package.json';
const absolutePackageJsonPath = path.resolve(
DIR,
Expand All @@ -81,7 +69,6 @@ describe.each(JEST_CONFIG_EXT_ORDER.slice(0))(

writeFiles(DIR, {[relativePackageJsonPath]: ''});

mockedConsoleWarn.mockClear();
// absolute
expect(
resolveConfigPath(path.dirname(absolutePackageJsonPath), DIR),
Expand All @@ -91,12 +78,10 @@ describe.each(JEST_CONFIG_EXT_ORDER.slice(0))(
expect(
resolveConfigPath(path.dirname(relativePackageJsonPath), DIR),
).toBe(absolutePackageJsonPath);
expect(mockedConsoleWarn).not.toBeCalled();

// jest.config.js takes precedence
writeFiles(DIR, {[relativeJestConfigPath]: ''});

mockedConsoleWarn.mockClear();
// absolute
expect(
resolveConfigPath(path.dirname(absolutePackageJsonPath), DIR),
Expand All @@ -106,30 +91,19 @@ describe.each(JEST_CONFIG_EXT_ORDER.slice(0))(
expect(
resolveConfigPath(path.dirname(relativePackageJsonPath), DIR),
).toBe(absoluteJestConfigPath);
expect(mockedConsoleWarn).not.toBeCalled();

// jest.config.js and package.json with 'jest' cannot be used together
writeFiles(DIR, {[relativePackageJsonPath]: JSON.stringify({jest: {}})});

// absolute
mockedConsoleWarn.mockClear();
expect(
expect(() =>
resolveConfigPath(path.dirname(absolutePackageJsonPath), DIR),
).toBe(absoluteJestConfigPath);
expect(mockedConsoleWarn).toBeCalledTimes(1);
expect(mockedConsoleWarn.mock.calls[0].join()).toMatch(
MULTIPLE_CONFIGS_ERROR_PATTERN,
);
).toThrowError(MULTIPLE_CONFIGS_ERROR_PATTERN);

// relative
mockedConsoleWarn.mockClear();
expect(
expect(() =>
resolveConfigPath(path.dirname(relativePackageJsonPath), DIR),
).toBe(absoluteJestConfigPath);
expect(mockedConsoleWarn).toBeCalledTimes(1);
expect(mockedConsoleWarn.mock.calls[0].join()).toMatch(
MULTIPLE_CONFIGS_ERROR_PATTERN,
);
).toThrowError(MULTIPLE_CONFIGS_ERROR_PATTERN);

expect(() => {
resolveConfigPath(
Expand All @@ -146,8 +120,7 @@ const pickPairsWithSameOrder = <T>(array: ReadonlyArray<T>) =>
.map((value1, idx, arr) =>
arr.slice(idx + 1).map(value2 => [value1, value2]),
)
// TODO: use .flat() when we drop Node 10
.reduce((acc, val) => acc.concat(val), []);
.flat();

test('pickPairsWithSameOrder', () => {
expect(pickPairsWithSameOrder([1, 2, 3])).toStrictEqual([
Expand All @@ -158,11 +131,9 @@ test('pickPairsWithSameOrder', () => {
});

describe.each(pickPairsWithSameOrder(JEST_CONFIG_EXT_ORDER))(
'Using multiple configs shows warning',
'Using multiple configs shows error',
(extension1, extension2) => {
test(`Using jest.config${extension1} and jest.config${extension2} shows warning`, () => {
const mockedConsoleWarn = mockConsoleWarn();

test(`Using jest.config${extension1} and jest.config${extension2} shows error`, () => {
const relativeJestConfigPaths = [
`a/b/c/jest.config${extension1}`,
`a/b/c/jest.config${extension2}`,
Expand All @@ -173,15 +144,9 @@ describe.each(pickPairsWithSameOrder(JEST_CONFIG_EXT_ORDER))(
[relativeJestConfigPaths[1]]: '',
});

// multiple configs here, should print warning
mockedConsoleWarn.mockClear();
expect(
expect(() =>
resolveConfigPath(path.dirname(relativeJestConfigPaths[0]), DIR),
).toBe(path.resolve(DIR, relativeJestConfigPaths[0]));
expect(mockedConsoleWarn).toBeCalledTimes(1);
expect(mockedConsoleWarn.mock.calls[0].join()).toMatch(
MULTIPLE_CONFIGS_ERROR_PATTERN,
);
).toThrowError(MULTIPLE_CONFIGS_ERROR_PATTERN);
});
},
);
6 changes: 3 additions & 3 deletions packages/jest-config/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export async function readConfig(
skipArgvConfigOption?: boolean,
parentConfigDirname?: string | null,
projectIndex = Infinity,
skipMultipleConfigWarning = false,
skipMultipleConfigError = false,
): Promise<ReadConfig> {
let rawOptions: Config.InitialOptions;
let configPath = null;
Expand Down Expand Up @@ -78,15 +78,15 @@ export async function readConfig(
configPath = resolveConfigPath(
argv.config,
process.cwd(),
skipMultipleConfigWarning,
skipMultipleConfigError,
);
rawOptions = await readConfigFileAndSetRootDir(configPath);
} else {
// Otherwise just try to find config in the current rootDir.
configPath = resolveConfigPath(
packageRootOrConfig,
process.cwd(),
skipMultipleConfigWarning,
skipMultipleConfigError,
);
rawOptions = await readConfigFileAndSetRootDir(configPath);
}
Expand Down
46 changes: 23 additions & 23 deletions packages/jest-config/src/resolveConfigPath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import * as path from 'path';
import chalk = require('chalk');
import * as fs from 'graceful-fs';
import slash = require('slash');
import {ValidationError} from 'jest-validate';
import {
JEST_CONFIG_BASE_NAME,
JEST_CONFIG_EXT_ORDER,
PACKAGE_JSON,
} from './constants';
import {BULLET, DOCUMENTATION_NOTE} from './utils';

const isFile = (filePath: string) =>
fs.existsSync(filePath) && !fs.lstatSync(filePath).isDirectory();
Expand All @@ -23,7 +25,7 @@ const getConfigFilename = (ext: string) => JEST_CONFIG_BASE_NAME + ext;
export default function resolveConfigPath(
pathToResolve: string,
cwd: string,
skipMultipleConfigWarning = false,
skipMultipleConfigError = false,
): string {
if (!path.isAbsolute(cwd)) {
throw new Error(`"cwd" must be an absolute path. cwd: ${cwd}`);
Expand Down Expand Up @@ -58,15 +60,15 @@ export default function resolveConfigPath(
absolutePath,
pathToResolve,
cwd,
skipMultipleConfigWarning,
skipMultipleConfigError,
);
}

const resolveConfigPathByTraversing = (
pathToResolve: string,
initialPath: string,
cwd: string,
skipMultipleConfigWarning: boolean,
skipMultipleConfigError: boolean,
): string => {
const configFiles = JEST_CONFIG_EXT_ORDER.map(ext =>
path.resolve(pathToResolve, getConfigFilename(ext)),
Expand All @@ -77,8 +79,8 @@ const resolveConfigPathByTraversing = (
configFiles.push(packageJson);
}

if (!skipMultipleConfigWarning && configFiles.length > 1) {
console.warn(makeMultipleConfigsWarning(configFiles));
if (!skipMultipleConfigError && configFiles.length > 1) {
throw new ValidationError(...makeMultipleConfigsErrorMessage(configFiles));
}

if (configFiles.length > 0 || packageJson) {
Expand All @@ -96,7 +98,7 @@ const resolveConfigPathByTraversing = (
path.dirname(pathToResolve),
initialPath,
cwd,
skipMultipleConfigWarning,
skipMultipleConfigError,
);
};

Expand Down Expand Up @@ -137,20 +139,18 @@ function extraIfPackageJson(configPath: string) {
return '';
}

const makeMultipleConfigsWarning = (configPaths: Array<string>) =>
chalk.yellow(
[
chalk.bold('\u25cf Multiple configurations found:'),
...configPaths.map(
configPath =>
` * ${extraIfPackageJson(configPath)}${slash(configPath)}`,
),
'',
' Implicit config resolution does not allow multiple configuration files.',
' Either remove unused config files or select one explicitly with `--config`.',
'',
' Configuration Documentation:',
' https://jestjs.io/docs/configuration.html',
'',
].join('\n'),
);
const makeMultipleConfigsErrorMessage = (
configPaths: Array<string>,
): [string, string, string] => [
`${BULLET}${chalk.bold('Multiple configurations found')}`,
[
...configPaths.map(
configPath =>
` * ${extraIfPackageJson(configPath)}${slash(configPath)}`,
),
'',
' Implicit config resolution does not allow multiple configuration files.',
' Either remove unused config files or select one explicitly with `--config`.',
].join('\n'),
DOCUMENTATION_NOTE,
];