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

Allow array of regexp strings for testRegex #7209

Merged
merged 6 commits into from
Oct 20, 2018
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 3 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{
"editor.rulers": [80],
"editor.rulers": [
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the most annoying thing.... so when I fixed this, then saved, of course it incorrectly reformatted the fixed file itself!! I keel you vscode

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git checkout master .vscode/settings.json

80
],
"files.exclude": {
"**/.git": true,
"**/node_modules": true,
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- `[jest-haste-map]` Make `ignorePattern` optional ([#7166](https://github.com/facebook/jest/pull/7166))
- `[jest-runtime]` Remove `cacheDirectory` from `ignorePattern` for `HasteMap` if not necessary ([#7166](https://github.com/facebook/jest/pull/7166))
- `[jest-validate]` Add syntax to validate multiple permitted types ([#7207](https://github.com/facebook/jest/pull/7207))
- `[jest-config]` Accept an array as as well as a string for `testRegex`([#7209]https://github.com/facebook/jest/pull/7209))

### Fixes

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
6 changes: 3 additions & 3 deletions docs/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ The glob patterns Jest uses to detect test files. By default it looks for `.js`

See the [micromatch](https://github.com/jonschlinkert/micromatch) package for details of the patterns you can specify.

See also [`testRegex` [string]](#testregex-string), but note that you cannot specify both options.
See also [`testRegex` [string | Array<string>]](#testregex-string), but note that you cannot specify both options.

### `testPathIgnorePatterns` [array<string>]

Expand All @@ -814,11 +814,11 @@ An array of regexp pattern strings that are matched against all test paths befor

These pattern strings match against the full path. Use the `<rootDir>` string token to include the path to your project's root directory to prevent it from accidentally ignoring all of your files in different environments that may have different root directories. Example: `["<rootDir>/build/", "<rootDir>/node_modules/"]`.

### `testRegex` [string]
### `testRegex` [string | Array<string>]

Default: `(/__tests__/.*|(\\.|/)(test|spec))\\.jsx?$`

The pattern Jest uses to detect test files. By default it looks for `.js` and `.jsx` files inside of `__tests__` folders, as well as any files with a suffix of `.test` or `.spec` (e.g. `Component.test.js` or `Component.spec.js`). It will also find files called `test.js` or `spec.js`. See also [`testMatch` [array<string>]](#testmatch-array-string), but note that you cannot specify both options.
The pattern or patterns Jest uses to detect test files. By default it looks for `.js` and `.jsx` files inside of `__tests__` folders, as well as any files with a suffix of `.test` or `.spec` (e.g. `Component.test.js` or `Component.spec.js`). It will also find files called `test.js` or `spec.js`. See also [`testMatch` [array<string>]](#testmatch-array-string), but note that you cannot specify both options.

The following is a visualization of the default regex:

Expand Down
2 changes: 1 addition & 1 deletion e2e/__tests__/__snapshots__/show_config.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ exports[`--showConfig outputs config info and exits 1`] = `
\\"testPathIgnorePatterns\\": [
\\"/node_modules/\\"
],
\\"testRegex\\": \\"\\",
\\"testRegex\\": [],
\\"testRunner\\": \\"<<REPLACED_JEST_PACKAGES_DIR>>/jest-jasmine2/build/index.js\\",
\\"testURL\\": \\"http://localhost\\",
\\"timers\\": \\"real\\",
Expand Down
7 changes: 3 additions & 4 deletions packages/jest-cli/src/SearchSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,12 @@ const globsToMatcher = (globs: ?Array<Glob>) => {
return path => micromatch([path], globs, {dot: true}).length > 0;
};

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

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

const toTests = (context, tests) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jest.mock(
},
);

const config = {roots: [], testPathIgnorePatterns: [], testRegex: ''};
const config = {roots: [], testPathIgnorePatterns: [], testRegex: []};
let globalConfig;
const defaults = {
changedFilesPromise: Promise.resolve({
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-cli/src/__tests__/watch.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ describe('Watch mode flows', () => {
let stdin;

beforeEach(() => {
const config = {roots: [], testPathIgnorePatterns: [], testRegex: ''};
const config = {roots: [], testPathIgnorePatterns: [], testRegex: []};
pipe = {write: jest.fn()};
globalConfig = {watch: true};
hasteMapInstances = [{on: () => {}}];
Expand Down
5 changes: 3 additions & 2 deletions packages/jest-cli/src/cli/args.js
Original file line number Diff line number Diff line change
Expand Up @@ -576,8 +576,9 @@ export const options = {
type: 'array',
},
testRegex: {
description: 'The regexp pattern Jest uses to detect test files.',
type: 'string',
description:
'A string or array of string regexp patterns that Jest uses to detect test files.',
type: 'array',
},
testResultsProcessor: {
description:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ module.exports = {
// \\"/node_modules/\\"
// ],

// The regexp pattern Jest uses to detect test files
// testRegex: \\"\\",
// The regexp pattern or array of patterns that Jest uses to detect test files
// testRegex: [],

// This option allows the use of a custom results processor
// testResultsProcessor: null,
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-config/src/Defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export default ({
testLocationInResults: false,
testMatch: ['**/__tests__/**/*.js?(x)', '**/?(*.)+(spec|test).js?(x)'],
testPathIgnorePatterns: [NODE_MODULES_REGEXP],
testRegex: '',
testRegex: [],
testResultsProcessor: null,
testRunner: 'jasmine2',
testURL: 'http://localhost',
Expand Down
3 changes: 2 additions & 1 deletion packages/jest-config/src/Descriptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,8 @@ export default ({
testMatch: 'The glob patterns Jest uses to detect test files',
testPathIgnorePatterns:
'An array of regexp pattern strings that are matched against all test paths, matched tests are skipped',
testRegex: 'The regexp pattern Jest uses to detect test files',
testRegex:
'The regexp pattern or array of patterns that Jest uses to detect test files',
testResultsProcessor:
'This option allows the use of a custom results processor',
testRunner: 'This option allows use of a custom test runner',
Expand Down
6 changes: 5 additions & 1 deletion packages/jest-config/src/ValidConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import type {InitialOptions} from 'types/Config';

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

const NODE_MODULES_REGEXP = replacePathSepForRegex(NODE_MODULES);
Expand Down Expand Up @@ -100,7 +101,10 @@ export default ({
testMatch: ['**/__tests__/**/*.js?(x)', '**/?(*.)+(spec|test).js?(x)'],
testNamePattern: 'test signature',
testPathIgnorePatterns: [NODE_MODULES_REGEXP],
testRegex: '(/__tests__/.*|(\\.|/)(test|spec))\\.jsx?$',
testRegex: MultipleValidOptions(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also, what do you think about naming this just multiple for simplicity? I'm good with both, so pick whatever you like :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel strongly, but I guess I'd lean towards leaving it more verbose to provide clear context, since Mutliple could potentially mean more than one thing.

'(/__tests__/.*|(\\.|/)(test|spec))\\.jsx?$',
['/__tests__/\\.test\\.jsx?$', '/__tests__/\\.spec\\.jsx?$'],
),
testResultsProcessor: 'processor-node-module',
testRunner: 'jasmine2',
testURL: 'http://localhost',
Expand Down
38 changes: 37 additions & 1 deletion packages/jest-config/src/__tests__/normalize.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,42 @@ describe('Upgrade help', () => {
});
});

describe('testRegex', () => {
it('testRegex empty string is mapped to empty array', () => {
const {options} = normalize(
{
rootDir: '/root',
testRegex: '',
},
{},
);

expect(options.testRegex).toEqual([]);
});
it('testRegex string is mapped to array of RegExp objects', () => {
const {options} = normalize(
{
rootDir: '/root',
testRegex: '.*',
},
{},
);

expect(options.testRegex).toEqual([/.*/]);
});
it('testRegex array is mapped to array of RegExp objects', () => {
const {options} = normalize(
{
rootDir: '/root',
testRegex: ['.*', 'foo\\.bar'],
},
{},
);

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

describe('testMatch', () => {
it('testMatch default not applied if testRegex is set', () => {
const {options} = normalize(
Expand All @@ -826,7 +862,7 @@ describe('testMatch', () => {
{},
);

expect(options.testRegex).toBe('');
expect(options.testRegex).toEqual([]);
});

it('throws if testRegex and testMatch are both specified', () => {
Expand Down
10 changes: 7 additions & 3 deletions packages/jest-config/src/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,11 @@ export default function normalize(options: InitialOptions, argv: Argv) {
);
break;
case 'testRegex':
value = options[key] && replacePathSepForRegex(options[key]);
const valueOrEmptyArray = options[key] || [];
const valueArray = Array.isArray(valueOrEmptyArray)
? valueOrEmptyArray
: [valueOrEmptyArray];
value = valueArray.map(replacePathSepForRegex).map(e => new RegExp(e));
jamietre marked this conversation as resolved.
Show resolved Hide resolved
break;
case 'moduleFileExtensions': {
value = options[key];
Expand Down Expand Up @@ -701,14 +705,14 @@ export default function normalize(options: InitialOptions, argv: Argv) {
}
}

if (options.testRegex && options.testMatch) {
if (newOptions.testRegex.length && options.testMatch) {
throw createConfigError(
` Configuration options ${chalk.bold('testMatch')} and` +
` ${chalk.bold('testRegex')} cannot be used together.`,
);
}

if (options.testRegex && !options.testMatch) {
if (newOptions.testRegex.length && !options.testMatch) {
// Prevent the default testMatch conflicting with any explicitly
// configured `testRegex` value
newOptions.testMatch = [];
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-editor-support/src/Settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {createProcess} from './Process';
type Glob = string;

type ConfigRepresentation = {
testRegex: string,
testRegex: string | Array<string>,
testMatch: Array<Glob>,
};

Expand Down Expand Up @@ -59,7 +59,7 @@ export default class Settings extends EventEmitter {
// Defaults for a Jest project
this.settings = {
testMatch: ['**/__tests__/**/*.js?(x)', '**/?(*.)+(spec|test).js?(x)'],
testRegex: '(/__tests__/.*|\\.(test|spec))\\.jsx?$',
testRegex: ['(/__tests__/.*|\\.(test|spec))\\.jsx?$'],
};

this.configs = [this.settings];
Expand Down
18 changes: 15 additions & 3 deletions packages/jest-runtime/src/__tests__/should_instrument.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +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/],
});
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that we only test happy path in this test file. It would be great to add at least two smoke tests (1 for testRegex, 1 for testMatch) when the file should not be instrumented (e.g. add 4th expected argument to testShouldInstrument with default value of true).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are already two describe sections in this file, the 2nd -- "should return false" -- tests for scenarios that don't match, the unhappy path. I did add a test there already for testRegex.

I do see that the description of the spec was incorrect, I have fixed that, as well as a couple other existing ones that were also incorrect. Maybe this was throwing you off - either way if there's still some additional coverage you'd like to see let me know.

I have also added a test of error condition to normalize.test.js.

Copy link
Collaborator

@thymikee thymikee Oct 19, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my, I totally missed second testShouldInstrument. Should be renamed to testShouldNotInstrument 😛

});

Expand Down Expand Up @@ -88,7 +94,7 @@ describe('should_instrument', () => {
testShouldInstrument('do/collect/sum.coverage.test.js', defaultOptions, {
forceCoverageMatch: ['**/*.(coverage).(test).js'],
rootDir: '/',
testRegex: '.*\\.(test)\\.(js)$',
testRegex: ['.*\\.(test)\\.(js)$'],
});
});
});
Expand All @@ -115,7 +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/],
});
});

Expand Down
5 changes: 4 additions & 1 deletion packages/jest-runtime/src/should_instrument.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,10 @@ export default function shouldInstrument(
return true;
}

if (config.testRegex && filename.match(config.testRegex)) {
if (
config.testRegex &&
config.testRegex.some(regex => regex.test(filename))
) {
return false;
}

Expand Down
2 changes: 1 addition & 1 deletion types/Argv.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ export type Argv = {|
testNamePattern: string,
testPathIgnorePatterns: Array<string>,
testPathPattern: Array<string>,
testRegex: string,
testRegex: string | Array<string>,
testResultsProcessor: ?string,
testRunner: string,
testURL: string,
Expand Down
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: string,
testRegex: Array<RegExp>,
testResultsProcessor: ?string,
testRunner: ?string,
testURL: string,
Expand Down Expand Up @@ -165,7 +165,7 @@ export type InitialOptions = {
testNamePattern?: string,
testPathDirs?: Array<Path>,
testPathIgnorePatterns?: Array<string>,
testRegex?: string,
testRegex?: string | Array<any>,
testResultsProcessor?: ?string,
testRunner?: string,
testURL?: string,
Expand Down Expand Up @@ -282,7 +282,7 @@ export type ProjectConfig = {|
testMatch: Array<Glob>,
testLocationInResults: boolean,
testPathIgnorePatterns: Array<string>,
testRegex: string,
testRegex: Array<RegExp>,
testRunner: string,
testURL: string,
timers: 'real' | 'fake',
Expand Down