WIP: haste ignores using testPathIgnorePatterns, not all node_modules #2318
Conversation
|
I've tried to work on this a bit but I'm not completely sure how this one is supposed to look? Is this supposed to work 'out of the box' or do we require some configuration? @gaearon was pushing to make the use of |
|
@cpojer can you or someone else knowledgeable about haste help me out on this? I don't really understand how to get the tests passing here. |
| @@ -672,10 +672,7 @@ class HasteMap extends EventEmitter { | |||
| * Helpers | |||
| */ | |||
| _ignore(filePath: Path): boolean { | |||
| return ( | |||
| this._options.ignorePattern.test(filePath) || | |||
| (!this._options.retainAllFiles && this._isNodeModulesDir(filePath)) | |||
There was a problem hiding this comment.
we cannot remove this option as we are using it at FB.
There was a problem hiding this comment.
Instead maybe we could add config.testPathIgnorePatterns to this._whitelist?
|
@cpojer could you reply to my comment? (#2318 (comment)) I'd love to work on this pull request but I'm not sure if you want this to work out of the box or should be configurable. |
|
I think making retainAllFiles configurable through the I honestly don't know yet what the right solution should really be as I haven't had time to think about it beyond pointing out that the solution in this PR is not appropriate. It would be best to pick this up sometime after New Years, as I'm on vacation right now. |
|
IMHO, only Another approach could be to use |
|
Did anybody else have thoughts on this? As is, this PR isn't a good solution. |
|
I agree with @julien-f that adding should be enough to make sure Jest runs the tests inside any |
|
@julien-f and @rovansteen are you saying this is already working? |
|
@cpojer No it's not, it's how it should work IMHO :) |
|
But the more I think about it, the more I think it should by default uses |
|
I'm happy if somebody makes a PR that makes that work. This one doesn't yet, though. |
|
I don't think that is appropriate, @julien-f. Not every project uses git, for example we use |
|
Fair enough :) |
|
@cpojer Is this fixed? |
|
No it isn't yet and we haven't made any progress on this in a while. It doesn't seem like this PR isn't being worked on right now. |
|
Ok, thank you. |
|
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
An attempt to fix the issue raised in #2145, which is blocking facebook/create-react-app#1081.
Test plan
This PR is incomplete because this test is failing; furthermore, I should introduce a test that fails in master but works in this PR to illustrate the feature it solves.
I need some help with this.