Fix --testPathPattern escaping for '\\' on Windows#5230
Fix --testPathPattern escaping for '\\' on Windows#5230cpojer merged 1 commit intojestjs:masterfrom seanpoulter:issue-5216
Conversation
|
As a heads up, here's the manual testing results with this PR.
|
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.
Codecov Report
@@ Coverage Diff @@
## master #5230 +/- ##
==========================================
+ Coverage 60.91% 61.13% +0.22%
==========================================
Files 202 202
Lines 6731 6760 +29
Branches 4 4
==========================================
+ Hits 4100 4133 +33
+ Misses 2630 2626 -4
Partials 1 1
Continue to review full report at Codecov.
|
|
Thank you for the thorough investigation and fix, this is really awesome and I greatly appreciate it. I merged this PR and let you consider the posix difference separately. |
All good, I just forgot to escape the
|
PR #5054 added a call to `replacePathSepForRegex` to escape values of the `--testPathPattern` and `<regexForTestFiles>` CLI options. Since the Windows path separator and the regular expression special character delimeter are the same character, this can lead to ambiguous patterns (e.g.: `app\book\d*\`). This commit: - Removes escaping CLI args with `replacePathSepForRegex` to leave them as is unless it's a POSIX path separator on Windows - Changes the tests in `normalize.test.js` to run the same test suite for `--testPathPattern` and `<regexForTestFiles>` - Reverts the changes to `replacePathSepForRegex` from #5230 but keeps the tests for the intended behavior. It will be complicated to escape the "safe" cases when `\` is a path separator and not a regular expression delimeter. Instead of getting fancy, we can urge Windows users to use `/` or `\\` as a path separator.
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
4 participants
Summary
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 escapespath\\.*fileaspath\\\.*file.This PR fixes the common regular expression escaping used in the "Watch Usage" prompt and the
--testPathPatternCLI argument with unit tests.Test plan
New unit tests have been added to confirm the regular expression escaping behavior. I've also added a snapshot to test the 8 test cases documented in #5216. I'm happy to record GIFs from a Linux and Windows system if you feel it's necessary.
Lessons learned
Sorry folks. I hastily submitted #5054 without adding enough tests to be confident in the regular expression escaping. I was wrong to assume that the "Watch Usage" prompt would behave the same way.