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

Resolve #5216 #5269

Merged
merged 1 commit into from
Jan 10, 2018
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
124 changes: 69 additions & 55 deletions packages/jest-config/src/__tests__/normalize.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1031,15 +1031,6 @@ describe('testPathPattern', () => {
const initialOptions = {rootDir: '/root'};
const consoleLog = console.log;

function testWindowsPathSeparator(argv, expected) {
jest.resetModules();
jest.mock('path', () => require.requireActual('path').win32);
require('jest-resolve').findNodeModule = findNodeModule;

const {options} = require('../normalize').default(initialOptions, argv);
expect(options.testPathPattern).toBe(expected);
}

beforeEach(() => {
console.log = jest.fn();
});
Expand All @@ -1053,62 +1044,85 @@ describe('testPathPattern', () => {
expect(options.testPathPattern).toBe('');
});

describe('--testPathPattern', () => {
it('uses testPathPattern if set', () => {
const {options} = normalize(initialOptions, {testPathPattern: ['a/b']});
expect(options.testPathPattern).toBe('a/b');
});
const cliOptions = [
{name: '--testPathPattern', property: 'testPathPattern'},
{name: '<regexForTestFiles>', property: '_'},
];
for (const opt of cliOptions) {
describe(opt.name, () => {
it('uses ' + opt.name + ' if set', () => {
const argv = {[opt.property]: ['a/b']};
const {options} = normalize(initialOptions, argv);

it('ignores invalid regular expressions and logs a warning', () => {
const {options} = normalize(initialOptions, {testPathPattern: ['a(']});
expect(options.testPathPattern).toBe('');
expect(console.log.mock.calls[0][0]).toMatchSnapshot();
});
expect(options.testPathPattern).toBe('a/b');
});

it('escapes Windows path separators', () => {
testWindowsPathSeparator({testPathPattern: ['a\\b']}, 'a\\\\b');
});
it('ignores invalid regular expressions and logs a warning', () => {
const argv = {[opt.property]: ['a(']};
const {options} = normalize(initialOptions, argv);

it('joins multiple --testPathPatterns if set', () => {
const {options} = normalize(initialOptions, {
testPathPattern: ['a/b', 'c/d'],
expect(options.testPathPattern).toBe('');
expect(console.log.mock.calls[0][0]).toMatchSnapshot();
});
expect(options.testPathPattern).toBe('a/b|c/d');
});

it('escapes Windows path separators in multiple args', () => {
testWindowsPathSeparator(
{testPathPattern: ['a\\b', 'c\\d']},
'a\\\\b|c\\\\d',
);
});
});
it('joins multiple ' + opt.name + ' if set', () => {
const argv = {testPathPattern: ['a/b', 'c/d']};
const {options} = normalize(initialOptions, argv);

describe('<regexForTestFiles>', () => {
it('uses <regexForTestFiles> if set', () => {
const {options} = normalize(initialOptions, {_: ['a/b']});
expect(options.testPathPattern).toBe('a/b');
});

it('ignores invalid regular expressions and logs a warning', () => {
const {options} = normalize(initialOptions, {_: ['a(']});
expect(options.testPathPattern).toBe('');
expect(console.log.mock.calls[0][0]).toMatchSnapshot();
});
expect(options.testPathPattern).toBe('a/b|c/d');
});

it('escapes Windows path separators', () => {
testWindowsPathSeparator({_: ['a\\b']}, 'a\\\\b');
});
describe('posix', () => {
it('should not escape the pattern', () => {
const argv = {[opt.property]: ['a\\/b', 'a/b', 'a\\b', 'a\\\\b']};
const {options} = normalize(initialOptions, argv);

it('joins multiple <regexForTestFiles> if set', () => {
const {options} = normalize(initialOptions, {_: ['a/b', 'c/d']});
expect(options.testPathPattern).toBe('a/b|c/d');
});
expect(options.testPathPattern).toBe('a\\/b|a/b|a\\b|a\\\\b');
});
});

it('escapes Windows path separators in multiple args', () => {
testWindowsPathSeparator({_: ['a\\b', 'c\\d']}, 'a\\\\b|c\\\\d');
describe('win32', () => {
beforeEach(() => {
jest.mock('path', () => require.requireActual('path').win32);
require('jest-resolve').findNodeModule = findNodeModule;
});

afterEach(() => {
jest.resetModules();
});

it('preserves any use of "\\"', () => {
const argv = {[opt.property]: ['a\\b', 'c\\\\d']};
const {options} = require('../normalize').default(
initialOptions,
argv,
);

expect(options.testPathPattern).toBe('a\\b|c\\\\d');
});

it('replaces POSIX path separators', () => {
const argv = {[opt.property]: ['a/b']};
const {options} = require('../normalize').default(
initialOptions,
argv,
);

expect(options.testPathPattern).toBe('a\\\\b');
});

it('replaces POSIX paths in multiple args', () => {
const argv = {[opt.property]: ['a/b', 'c/d']};
const {options} = require('../normalize').default(
initialOptions,
argv,
);

expect(options.testPathPattern).toBe('a\\\\b|c\\\\d');
});
});
});
});
}

it('joins multiple --testPathPatterns and <regexForTestFiles>', () => {
const {options} = normalize(initialOptions, {
Expand Down
9 changes: 8 additions & 1 deletion packages/jest-config/src/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,14 @@ const buildTestPathPattern = (argv: Argv): string => {
patterns.push(...argv.testPathPattern);
}

const testPathPattern = patterns.map(replacePathSepForRegex).join('|');
const replacePosixSep = (pattern: string) => {
if (path.sep === '/') {
return pattern;
}
return pattern.replace(/\//g, '\\\\');
};

const testPathPattern = patterns.map(replacePosixSep).join('|');
if (validatePattern(testPathPattern)) {
return testPathPattern;
} else {
Expand Down

This file was deleted.

54 changes: 14 additions & 40 deletions packages/jest-regex-util/src/__tests__/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,66 +4,40 @@ import {replacePathSepForRegex} from '../index';
import path from 'path';

describe('replacePathSepForRegex()', () => {
const testPatternsFrom5216 = [
'jest-config\\/.*normalize',
'jest-config/.*normalize',
'jest-config\\.*normalize',
'jest-config\\\\.*normalize',
];

describe('posix', () => {
beforeEach(() => (path.sep = '/'));

it('should return the path', () => {
const expected = {};
expect(replacePathSepForRegex(expected)).toBe(expected);
});

// Confirming existing behavior; could be changed to improve cross-platform support
it('should not replace Windows path separators', () => {
expect(replacePathSepForRegex('a\\.*b')).toBe('a\\.*b');
expect(replacePathSepForRegex('a\\\\.*b')).toBe('a\\\\.*b');
});

// Bonus: Test cases from https://github.com/facebook/jest#5216
it('should match the expected output from #5216', () => {
expect(
testPatternsFrom5216.map(replacePathSepForRegex),
).toMatchSnapshot();
});
});

describe('win32', () => {
beforeEach(() => (path.sep = '\\'));

it('should escape Windows path separators', () => {
expect(replacePathSepForRegex('a\\b\\c')).toBe('a\\\\b\\\\c');
});

it('should replace POSIX path separators', () => {
expect(replacePathSepForRegex('a/b/c')).toBe('a\\\\b\\\\c');
});

it('should not escape an escaped period', () => {
expect(replacePathSepForRegex('a\\.dotfile')).toBe('a\\.dotfile');
expect(replacePathSepForRegex('a\\\\\\.dotfile')).toBe('a\\\\\\.dotfile');
// When a regular expression is created with a string, not enclosing
// slashes like "/<pattern>/", the "/" character does not need to be
// escaped as "\/". The result is the double path separator: "\\"
expect(replacePathSepForRegex('a\\/b')).toBe('a\\\\\\\\b');
});

it('should not escape an escaped Windows path separator', () => {
expect(replacePathSepForRegex('a\\\\b')).toBe('a\\\\b');
expect(replacePathSepForRegex('a\\\\.dotfile')).toBe('a\\\\.dotfile');
it('should escape Windows path separators', () => {
expect(replacePathSepForRegex('a\\b\\c')).toBe('a\\\\b\\\\c');
});

// Confirming existing behavior; could be changed to improve cross-platform support
it('should not replace escaped POSIX separators', () => {
expect(replacePathSepForRegex('a\\/b')).toBe('a\\\\\\\\b');
});
it('should not escape an escaped dot', () => {
expect(replacePathSepForRegex('a\\.dotfile')).toBe('a\\.dotfile');

// Bonus: Test cases from https://github.com/facebook/jest#5216
it('should match the expected output from #5216', () => {
expect(
testPatternsFrom5216.map(replacePathSepForRegex),
).toMatchSnapshot();
// If we expect Windows path separators to be escaped, one would expect
// the regular expression "\\\." to be unescaped as "\.". This is not the
// current behavior.
expect(replacePathSepForRegex('a\\\\\\.dotfile')).toBe(
'a\\\\\\\\\\.dotfile',
);
});
});
});
54 changes: 3 additions & 51 deletions packages/jest-regex-util/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,56 +22,8 @@ export const escapeStrForRegex = (string: string) =>
string.replace(/[[\]{}()*+?.\\^$|]/g, '\\$&');

export const replacePathSepForRegex = (string: string) => {
if (!string || path.sep !== '\\') {
return string;
}

let result = '';
for (let i = 0; i < string.length; i += 1) {
const char = string[i];
if (char === '\\') {
const nextChar = string[i + 1];
/* Case: \/ -- recreate legacy behavior */
if (nextChar === '/') {
i += 1;
result += '\\\\\\\\';
continue;
}

/* Case: \. */
if (nextChar === '.') {
i += 1;
result += '\\.';
continue;
}

/* Case: \\. */
if (nextChar === '\\' && string[i + 2] === '.') {
i += 2;
result += '\\\\.';
continue;
}

/* Case: \\ */
if (nextChar === '\\') {
i += 1;
result += '\\\\';
continue;
}

/* Case: \<other> */
result += '\\\\';
continue;
}

/* Case: / */
if (char === '/') {
result += '\\\\';
continue;
}

result += char;
if (path.sep === '\\') {
return string.replace(/(\/|\\(?!\.))/g, '\\\\');
}

return result;
return string;
};