jest: Breaking change in --testPathPattern

Since Jest 22 the fix of https://github.com/facebook/jest/issues/3793 via https://github.com/facebook/jest/pull/5054 breaks all tooling that already passed escaped path separator in the argument (e.g. IntelliJ IDEA / WebStorm Jest test runner).

The documentation states "--testPathPattern=<regex>", so the previous behaviour was imho correct. The argument accepts regex and it’s up to the user to correctly escape the string (including windows path separator).

If the change of behaviour was intentional, it should be at the very least mentioned in the version 22 release notes / migration guide from previous version.

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 1
Comments: 19 (11 by maintainers)

Most upvoted comments

@butchyyyy @seanpoulter Great work! Thanks for the heads up, I’m maintaining IntelliJ IDEA / WebStorm Jest test runner. Yes, it’s was reported as https://youtrack.jetbrains.com/issue/WEB-30377. Fixed in 2017.3.3 by replacing path separators (\, /) with . (any char) on Windows. E.g. the following pattern will be passed for the above-mentioned example: --testPathPattern ^C:.develop.SuchCoolProject.src.__tests__.component.verify.CoolComponent\.react-test\.tsx$

@seanpoulter You’re right that there is no need to escape - as it’s enough to escape [ and ] (https://github.com/lodash/lodash/blob/master/escapeRegExp.js). Thanks, will update IJ escaping logic. Unfortunately, IJ/WebStorm Jest test runner source code is closed, but I’m here to assist if you have any questions regarding Jest integration.

segrey on Jan 8, 2018

I meant remove the call to replacePathSepForRegex in the CLI arg only. I’d leave the watch usage as is.

Fine by me as that means going back to way it worked in Jest 21.

Also ok with Introducing / for \\ replacement in the --testPathPattern on Windows

butchyyyy on Jan 5, 2018

changing that in the replacePathSepForRegex would affect the watch usage right?

Ugh, sorry @butchyyyy. I meant remove the call to replacePathSepForRegex in the CLI arg only. I’d leave the watch usage as is.

seanpoulter on Jan 5, 2018

Many edge cases will fail becuase replacePathSepForRegex will mangle valid regexp

The watch usage file pattern will continue to mangle regular expressions. 😞

The idea of being able to correctly escape windows path separator in a regexp is fundamentally wrong.

Yes, there will always be cases when the \ is ambiguous. Can we improve cross-platform support and makes it friendlier for new Windows users despite this?

I’m game to revert replacePathSepForRegex and only swap swap POSIX path separators on Windows. That’d let us use / as a cross-platform path separator. We can update the docs accordingly to say “regexp preferring /”.

If we don’t escape Windows separators, how about displaying a warning to coach Windows users to use / or \\ when:

your pattern uses \ only
there are no matches
the directory exists

I don’t really see the point of dragging IJ into this

They’ll get a heads up on the issue, can check if the - is properly escaped, may have time to contribute to the issue, and they can comment as they see fit.

seanpoulter on Jan 5, 2018