Normalize --findRelatedTests paths on win32 platforms #8961
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8961 +/- ##
==========================================
- Coverage 64.77% 64.74% -0.03%
==========================================
Files 280 280
Lines 11987 11994 +7
Branches 2956 2957 +1
==========================================
+ Hits 7764 7766 +2
- Misses 3591 3596 +5
Partials 632 632
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
I don't think this is the right way to fix this, since it makes FS calls to find out what the right casing for the path is first, so that it will later match when compared with the HasteMap that already has the whole dependency tree without having to make additional calls. I believe the right solution would be to make the matching of paths itself case insensitive (if the FS is) - just like the FS itself doesn't care about casing when looking for entries.
packages/jest-core/package.json
Outdated
| @@ -13,6 +13,7 @@ | |||
| "ansi-escapes": "^4.2.1", | |||
| "chalk": "^2.0.1", | |||
| "exit": "^0.1.2", | |||
| "glob": "^7.1.3", | |||
There was a problem hiding this comment.
we already depend on micromatch here, shouldn't need another glob module
There was a problem hiding this comment.
The difference is that glob doesn't just match strings but matches files in the FS - that's what I was referring to as extra FS calls
There was a problem hiding this comment.
I've reimplemented this with micromatch. Could you review please?
|
|
||
| let paths = globalConfig.nonFlagArgs; | ||
|
|
||
| if (globalConfig.findRelatedTests && 'win32' === os.platform()) { |
There was a problem hiding this comment.
mac by default is also case insensitive, but I'm not sure if it's possible to detect it somehow beyond checking the os?
There was a problem hiding this comment.
Unfortunately I don't know any api that could be used to check it
SimenB
changed the title
|
Thanks! |
|
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. |
Summary
Hey, there is a fix for #8900 however I'm not sure this is the right place to normalize non flag arguments.
Test plan
I've made a few runs with "wrong" cased path on my machine. For
packages\jest-core\src\FailedTestsCache.tsfile I've runjest --findRelatedTests packages/jest-core/SRC/FailedTestsCache.tsand that works well.In addition I'll cover it with some test, just before do that I'm interested in your opinion about a correct place for paths normalization.