-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: allow selecting tests and files containing regexp special characters #32
fix: allow selecting tests and files containing regexp special characters #32
Conversation
Is this an issue in jest itself? Also, just for test name not file name? |
I'm going to answer based on my limited knowledge about jest
I don't think so, from what I understand what gets passed to that jest's callback could actually be a regexp~ pattern and would be interpreted as regexp by jest. We call it in here after selecting from the list though and this is already "resolved" string which we don't want to actually be interpreted as regexp (but it will, so we need to provide a regexp pattern with special characters escaped).
When preparing this I thought about this briefly but concluded that file patterns contain seperator ( |
… special character
1506d0f
to
72c9e62
Compare
Ok, I've found out that path separator is indeed special-cased by jest here - https://github.com/facebook/jest/blob/70bed722d6b68456ffcb7df70d4a9b59bcea3d8a/packages/jest-core/src/lib/update_global_config.ts#L34-L37 I've implemented this fix for selecting file names as well. It's past midnight right now and I'm not sure if I will have time to write tests for this tomorrow, but I'd appreciate a code review on the logic when you get time. Maybe something else will come to your mind while I will be working on tests. |
72c9e62
to
0c50cc5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wonderful, thank you! Just a test and this should be good. Thanks for taking the time to investigate 👍
const stdout = { columns: 80, write: jest.fn() }; | ||
const pluginTester = (Plugin, options = {}) => { | ||
const stdout = { | ||
columns: (options.stdout || {}).columns || 80, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
optional chaining plz 😉
|
||
expect(updateConfigAndRun).toHaveBeenCalledWith({ | ||
mode: 'watch', | ||
testPathPattern: 'ing\\.js', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this should have included more - width is set to 40 after all. But this is a separate issue that could be investigated.
@SimenB I've added more tests and made some minor changes - so a re-review would be appreciated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wonderful, thank you! I'd like @rogeliog to take a look as well
Sure, no problem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this
Published as 0.4.1 |
This issue was quite annoying :p Jest interprets pattern as regexp so if we select a test name containing a special character this gets (currently) interpreted in a wrong way by jest. This fixes the problem