Skip to content

Commit

Permalink
Fix --testPathPattern escaping for '\\' on Windows (#5230)
Browse files Browse the repository at this point in the history
PR #5054 introduced a regression when handling escaped Windows path
separators in the `--testPathPattern`. The PR applied the same escaping
as the "Watch Usage" prompt, which incorrectly escapes `path\\.*file` as
`path\\\.*file`.

This commit fixes the regular expression used in "Watch Usage" and the
`--testPathPattern` CLI argument with unit tests.
  • Loading branch information
seanpoulter authored and cpojer committed Jan 4, 2018
1 parent 6896ee6 commit f3fce7b
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

* `[jest-config]` fix unexpected condition to avoid infinite recursion in
Windows platform. ([#5161](https://github.com/facebook/jest/pull/5161))
* `[jest-regex-util]` Fix breaking change in `--testPathPattern`
([#5230](https://github.com/facebook/jest/pull/5230))

### Features

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`replacePathSepForRegex() posix should match the expected output from #5216 1`] = `
Array [
"jest-config\\\\/.*normalize",
"jest-config/.*normalize",
"jest-config\\\\.*normalize",
"jest-config\\\\\\\\.*normalize",
]
`;

exports[`replacePathSepForRegex() win32 should match the expected output from #5216 1`] = `
Array [
"jest-config\\\\\\\\\\\\\\\\.*normalize",
"jest-config\\\\\\\\.*normalize",
"jest-config\\\\.*normalize",
"jest-config\\\\\\\\.*normalize",
]
`;
69 changes: 69 additions & 0 deletions packages/jest-regex-util/src/__tests__/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
jest.mock('path');

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');
});

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

// 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');
});

// Bonus: Test cases from https://github.com/facebook/jest#5216
it('should match the expected output from #5216', () => {
expect(
testPatternsFrom5216.map(replacePathSepForRegex),
).toMatchSnapshot();
});
});
});
54 changes: 51 additions & 3 deletions packages/jest-regex-util/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,56 @@ export const escapeStrForRegex = (string: string) =>
string.replace(/[[\]{}()*+?.\\^$|]/g, '\\$&');

export const replacePathSepForRegex = (string: string) => {
if (path.sep === '\\') {
return string.replace(/(\/|\\(?!\.))/g, '\\\\');
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;
}
return string;

return result;
};

0 comments on commit f3fce7b

Please sign in to comment.