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

Fix serialization issues with RegExp instances in the normalized config #7251

Merged
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 @@ -55,6 +55,7 @@
- `[jest-runtime]` Fix transform cache invalidation when requiring a test file from multiple projects ([#7186](https://github.com/facebook/jest/pull/7186))
- `[jest-changed-files]` Return correctly the changed files when using `lastCommit=true` on Mercurial repositories ([#7228](https://github.com/facebook/jest/pull/7228))
- `[babel-jest]` Cache includes babel environment variables ([#7239](https://github.com/facebook/jest/pull/7239))
- `[jest-config]` Use strings instead of `RegExp` instances in normalized configuration ([#7251](https://github.com/facebook/jest/pull/7251))

### Chore & Maintenance

Expand Down
2 changes: 1 addition & 1 deletion TestUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ const DEFAULT_PROJECT_CONFIG: ProjectConfig = {
testLocationInResults: false,
testMatch: [],
testPathIgnorePatterns: [],
testRegex: [/\.test\.js$/],
testRegex: ['\\.test\\.js$'],
testRunner: 'jest-jasmine2',
testURL: '',
timers: 'real',
Expand Down
17 changes: 17 additions & 0 deletions e2e/__tests__/__snapshots__/coverage_report.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,23 @@ All files | 85.71 | 100 | 50 | 100 | |

exports[`does not output coverage report when html is requested 1`] = `""`;

exports[`generates coverage when using the testRegex config param 1`] = `
"-------------------------------------|----------|----------|----------|----------|-------------------|
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s |
-------------------------------------|----------|----------|----------|----------|-------------------|
All files </> | 56.52</> | 0</> | 50</> | 55.56</> | </> |
coverage-report </> | 41.18</> | 0</> | 25</> | 42.86</> | </> |
OtherFile.js </> | 100</> | 100</> | 100</> | 100</> | </> |
Sum.js </> | 85.71</> | 100</> | 50</> | 100</> | </> |
SumDependency.js </> | 0</> | 0</> | 0</> | 0</> | 8,10,12</> |
notRequiredInTestSuite.js </> | 0</> | 0</> | 0</> | 0</> | 8,15,16,17,19</> |
coverage-report/cached-duplicates/a</> | 100</> | 100</> | 100</> | 100</> | </> |
Identical.js </> | 100</> | 100</> | 100</> | 100</> | </> |
coverage-report/cached-duplicates/b</> | 100</> | 100</> | 100</> | 100</> | </> |
Identical.js </> | 100</> | 100</> | 100</> | 100</> | </> |
-------------------------------------|----------|----------|----------|----------|-------------------|"
`;

exports[`json reporter printing with --coverage 1`] = `
"Test Suites: 1 failed, 1 total
Tests: 1 failed, 2 passed, 3 total
Expand Down
10 changes: 10 additions & 0 deletions e2e/__tests__/coverage_report.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -185,3 +185,13 @@ test('collects coverage from duplicate files avoiding shared cache', () => {
expect(stdout).toMatchSnapshot();
expect(status).toBe(0);
});

test('generates coverage when using the testRegex config param ', () => {
const {stdout, status} = runJest(DIR, [
'--no-cache',
'--testRegex=__tests__',
'--coverage',
]);
expect(stdout).toMatchSnapshot();
expect(status).toBe(0);
});
3 changes: 0 additions & 3 deletions packages/babel-jest/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,11 @@ const createTransformer = (options: any): Transformer => {
}
let configJsFilePath = path.join(directory, BABELRC_JS_FILENAME);
if (fs.existsSync(configJsFilePath)) {
// $FlowFixMe
cache[directory] = JSON.stringify(require(configJsFilePath));
break;
}
configJsFilePath = path.join(directory, BABEL_CONFIG_JS_FILENAME);
if (fs.existsSync(configJsFilePath)) {
// $FlowFixMe
cache[directory] = JSON.stringify(require(configJsFilePath));
break;
}
Expand All @@ -64,7 +62,6 @@ const createTransformer = (options: any): Transformer => {
? path.resolve(directory, PACKAGE_JSON)
: resolvedJsonFilePath;
if (fs.existsSync(packageJsonFilePath)) {
// $FlowFixMe
const packageJsonFileContents = require(packageJsonFilePath);
if (packageJsonFileContents[BABEL_CONFIG_KEY]) {
cache[directory] = JSON.stringify(
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-cli/src/SearchSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,12 @@ const globsToMatcher = (globs: ?Array<Glob>) => {
return path => micromatch([path], globs, {dot: true}).length > 0;
};

const regexToMatcher = (testRegex: Array<RegExp>) => {
const regexToMatcher = (testRegex: Array<string>) => {
if (!testRegex.length) {
return () => true;
}

return path => testRegex.some(e => e.test(path));
return path => testRegex.some(testRegex => new RegExp(testRegex).test(path));
};

const toTests = (context, tests) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const os = require('os');
const {makeGlobalConfig, makeProjectConfig} = require('../../../../TestUtils');

jest.mock('jest-runtime', () => {
// $FlowFixMe requireActual
const realRuntime = jest.requireActual('jest-runtime');
realRuntime.shouldInstrument = () => true;
return realRuntime;
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-config/src/ValidConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import type {InitialOptions} from 'types/Config';

import {replacePathSepForRegex} from 'jest-regex-util';
import {MultipleValidOptions} from 'jest-validate';
import {multipleValidOptions} from 'jest-validate';
import {NODE_MODULES} from './constants';

const NODE_MODULES_REGEXP = replacePathSepForRegex(NODE_MODULES);
Expand Down Expand Up @@ -101,7 +101,7 @@ export default ({
testMatch: ['**/__tests__/**/*.js?(x)', '**/?(*.)+(spec|test).js?(x)'],
testNamePattern: 'test signature',
testPathIgnorePatterns: [NODE_MODULES_REGEXP],
testRegex: MultipleValidOptions(
testRegex: multipleValidOptions(
'(/__tests__/.*|(\\.|/)(test|spec))\\.jsx?$',
['/__tests__/\\.test\\.jsx?$', '/__tests__/\\.spec\\.jsx?$'],
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,6 @@ exports[`testMatch throws if testRegex and testMatch are both specified 1`] = `
<red></>"
`;

exports[`testMatch throws if testRegex is provided an invalid regex string 1`] = `
"<red><bold><bold>● <bold>Validation Error</>:</>
<red></>
<red>Error parsing configuration for <bold>testRegex</>: \\"foo(bar\\" could not be parsed.</>
<red>Error: Invalid regular expression: /foo(bar/: Unterminated group</>
<red></>
<red> <bold>Configuration Documentation:</></>
<red> https://jestjs.io/docs/configuration.html</>
<red></>"
`;

exports[`testPathPattern <regexForTestFiles> ignores invalid regular expressions and logs a warning 1`] = `"<red> Invalid testPattern a( supplied. Running all tests instead.</>"`;

exports[`testPathPattern --testPathPattern ignores invalid regular expressions and logs a warning 1`] = `"<red> Invalid testPattern a( supplied. Running all tests instead.</>"`;
20 changes: 4 additions & 16 deletions packages/jest-config/src/__tests__/normalize.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ describe('testRegex', () => {

expect(options.testRegex).toEqual([]);
});
it('testRegex string is mapped to array of RegExp objects', () => {
it('testRegex string is mapped to an array', () => {
const {options} = normalize(
{
rootDir: '/root',
Expand All @@ -869,9 +869,9 @@ describe('testRegex', () => {
{},
);

expect(options.testRegex).toEqual([/.*/]);
expect(options.testRegex).toEqual(['.*']);
});
it('testRegex array is mapped to array of RegExp objects', () => {
it('testRegex array is preserved', () => {
const {options} = normalize(
{
rootDir: '/root',
Expand All @@ -880,7 +880,7 @@ describe('testRegex', () => {
{},
);

expect(options.testRegex).toEqual([/.*/, /foo\.bar/]);
expect(options.testRegex).toEqual(['.*', 'foo\\.bar']);
});
});

Expand Down Expand Up @@ -922,18 +922,6 @@ describe('testMatch', () => {
}).toThrowErrorMatchingSnapshot();
});

it('throws if testRegex is provided an invalid regex string', () => {
expect(() => {
normalize(
{
rootDir: '/root',
testRegex: 'foo(bar',
},
{},
);
}).toThrowErrorMatchingSnapshot();
});

it('normalizes testMatch', () => {
const {options} = normalize(
{
Expand Down
20 changes: 3 additions & 17 deletions packages/jest-config/src/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -569,23 +569,9 @@ export default function normalize(options: InitialOptions, argv: Argv) {
);
break;
case 'testRegex':
const valueOrEmptyArray = options[key] || [];
const valueArray = Array.isArray(valueOrEmptyArray)
? valueOrEmptyArray
: [valueOrEmptyArray];

value = valueArray.map(replacePathSepForRegex).map(pattern => {
try {
return new RegExp(pattern);
} catch (err) {
throw createConfigError(
`Error parsing configuration for ${chalk.bold(
key,
)}: "${pattern}" could not be parsed.\n` +
`Error: ${err.message}`,
);
}
});
value = options[key]
? [].concat(options[key]).map(replacePathSepForRegex)
: [];
break;
case 'moduleFileExtensions': {
value = options[key];
Expand Down
8 changes: 4 additions & 4 deletions packages/jest-runtime/src/__tests__/should_instrument.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ describe('should_instrument', () => {

it('when testRegex provided and file is not a test file', () => {
testShouldInstrument('source_file.js', defaultOptions, {
testRegex: [/.*\.(test)\\.(js)$'/],
testRegex: ['.*\\.(test)\\.(js)$'],
});
});

it('when more than one testRegex is provided and filename is not a test file', () => {
testShouldInstrument('source_file.js', defaultOptions, {
testRegex: [/.*\_(test)\.(js)$/, /.*\.(test)\.(js)$/, /never/],
testRegex: ['.*\\_(test)\\.(js)$', '.*\\.(test)\\.(js)$', 'never'],
});
});

Expand Down Expand Up @@ -121,13 +121,13 @@ describe('should_instrument', () => {

it('when testRegex provided and filename is a test file', () => {
testShouldInstrument(defaultFilename, defaultOptions, {
testRegex: [/.*\.(test)\.(js)$/],
testRegex: ['.*\\.(test)\\.(js)$'],
});
});

it('when more than one testRegex is provided and filename matches one of the patterns', () => {
testShouldInstrument(defaultFilename, defaultOptions, {
testRegex: [/.*\_(test)\.(js)$/, /.*\.(test)\.(js)$/, /never/],
testRegex: ['.*\\_(test)\\.(js)$', '.*\\.(test)\\.(js)$', 'never'],
});
});

Expand Down
2 changes: 1 addition & 1 deletion packages/jest-runtime/src/should_instrument.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export default function shouldInstrument(

if (
config.testRegex &&
config.testRegex.some(regex => regex.test(filename))
config.testRegex.some(regex => new RegExp(regex).test(filename))
) {
return false;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-validate/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -141,11 +141,11 @@ This will output:
## Example validating multiple types

```js
import {MultipleValidOptions} from 'jest-validate';
import {multipleValidOptions} from 'jest-validate';

validate(config, {
// `bar` will accept either a string or a number
bar: MultipleValidOptions('string is ok', 2),
bar: multipleValidOptions('string is ok', 2),
});
```

Expand Down
8 changes: 4 additions & 4 deletions packages/jest-validate/src/__tests__/validate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
'use strict';

import validate from '../validate';
import {MultipleValidOptions} from '../condition';
import {multipleValidOptions} from '../condition';
import jestValidateExampleConfig from '../example_config';
import jestValidateDefaultConfig from '../default_config';

Expand Down Expand Up @@ -210,7 +210,7 @@ test('works with custom deprecations', () => {

test('works with multiple valid types', () => {
const exampleConfig = {
foo: MultipleValidOptions('text', ['text']),
foo: multipleValidOptions('text', ['text']),
};

expect(
Expand Down Expand Up @@ -239,7 +239,7 @@ test('works with multiple valid types', () => {

test('reports errors nicely when failing with multiple valid options', () => {
const exampleConfig = {
foo: MultipleValidOptions('text', ['text']),
foo: multipleValidOptions('text', ['text']),
};

expect(() =>
Expand All @@ -254,7 +254,7 @@ test('reports errors nicely when failing with multiple valid options', () => {

test('Repeated types within multiple valid examples are coalesced in error report', () => {
const exampleConfig = {
foo: MultipleValidOptions('foo', 'bar', 2),
foo: multipleValidOptions('foo', 'bar', 2),
};

expect(() =>
Expand Down
14 changes: 8 additions & 6 deletions packages/jest-validate/src/condition.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

const toString = Object.prototype.toString;

const MultipleValidOptionsSymbol = Symbol('JEST_MULTIPLE_VALID_OPTIONS');
const MULTIPLE_VALID_OPTIONS_SYMBOL = Symbol('JEST_MULTIPLE_VALID_OPTIONS');

function validationConditionSingle(option: any, validOption: any): boolean {
return (
Expand All @@ -22,10 +22,9 @@ function validationConditionSingle(option: any, validOption: any): boolean {
export function getValues(validOption: any) {
if (
Array.isArray(validOption) &&
validOption.length &&
validOption[0] === MultipleValidOptionsSymbol
validOption[MULTIPLE_VALID_OPTIONS_SYMBOL]
) {
return validOption.slice(1);
return validOption;
}
return [validOption];
}
Expand All @@ -34,6 +33,9 @@ export function validationCondition(option: any, validOption: any): boolean {
return getValues(validOption).some(e => validationConditionSingle(option, e));
}

export function MultipleValidOptions(...args: Array<any>) {
return [MultipleValidOptionsSymbol, ...args];
export function multipleValidOptions(...args: Array<any>) {
const options = [...args];
// $FlowFixMe
options[MULTIPLE_VALID_OPTIONS_SYMBOL] = true;
return options;
}
4 changes: 2 additions & 2 deletions packages/jest-validate/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@ import {
} from './utils';
import validate from './validate';
import validateCLIOptions from './validate_cli_options';
import {MultipleValidOptions} from './condition';
import {multipleValidOptions} from './condition';

module.exports = {
MultipleValidOptions,
ValidationError,
createDidYouMeanMessage,
format,
logValidationWarning,
multipleValidOptions,
validate,
validateCLIOptions,
};
6 changes: 3 additions & 3 deletions types/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export type DefaultOptions = {|
testLocationInResults: boolean,
testMatch: Array<Glob>,
testPathIgnorePatterns: Array<string>,
testRegex: Array<RegExp>,
testRegex: Array<string>,
testResultsProcessor: ?string,
testRunner: ?string,
testURL: string,
Expand Down Expand Up @@ -166,7 +166,7 @@ export type InitialOptions = {
testNamePattern?: string,
testPathDirs?: Array<Path>,
testPathIgnorePatterns?: Array<string>,
testRegex?: string | Array<any>,
testRegex?: string | Array<string>,
testResultsProcessor?: ?string,
testRunner?: string,
testURL?: string,
Expand Down Expand Up @@ -283,7 +283,7 @@ export type ProjectConfig = {|
testMatch: Array<Glob>,
testLocationInResults: boolean,
testPathIgnorePatterns: Array<string>,
testRegex: Array<RegExp>,
testRegex: Array<string>,
testRunner: string,
testURL: string,
timers: 'real' | 'fake',
Expand Down