Skip to content

Commit

Permalink
Fix serialization issues with RegExp instances in the normalized conf…
Browse files Browse the repository at this point in the history
…ig (#7251)
  • Loading branch information
rubennorte authored Oct 24, 2018
1 parent 319329f commit d5c74a2
Show file tree
Hide file tree
Showing 17 changed files with 64 additions and 72 deletions.
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);
});
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

0 comments on commit d5c74a2

Please sign in to comment.