Skip to content
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

Test file path not found when description contains escaped quotes #12

Closed
xfh opened this issue May 1, 2019 · 9 comments
Closed

Test file path not found when description contains escaped quotes #12

xfh opened this issue May 1, 2019 · 9 comments

Comments

@xfh
Copy link
Contributor

xfh commented May 1, 2019

When you have a description containing escaped strings as bellow, the test file path cannot be found:

it('should return false for "\'\'"', () => expect(isNotNullOrUndefined('')).toBe(true));
@maurycyg
Copy link

If you looks at the regex:
https://github.com/fadc80/karma-sonarqube-reporter/blob/master/lib/path-finder.js#L63

the escaped characters are working fine and output of executing this regex against your it with or without escaped strings is the same

/((describe)|(it))\s*\(\s*((?<![\\])[\`\'\"])((?:.(?!(?<![\\])\4))*.?)\4/.exec(`it('should return false for "\'\'"', () => expect(isNotNullOrUndefined('')).toBe(true));`)

["it('should return false for "'", "it", undefined, "it", "'", "should return false for "", index: 0, input: "it('should return false for "''"', () => expect(isNotNullOrUndefined('')).toBe(true));", groups: undefined]
/((describe)|(it))\s*\(\s*((?<![\\])[\`\'\"])((?:.(?!(?<![\\])\4))*.?)\4/.exec(`it('should return false for "''"', () => expect(isNotNullOrUndefined('')).toBe(true));`)

["it('should return false for "'", "it", undefined, "it", "'", "should return false for "", index: 0, input: "it('should return false for "''"', () => expect(isNotNullOrUndefined('')).toBe(true));", groups: undefined]

@xfh
Copy link
Contributor Author

xfh commented May 22, 2019

The problem is that the parameter it, given to the function testFile, is receiving the string should return false for "''", while the name of the saved spec is should return false for "\'\'".

There is some logic in existIt to escape quotes of the parameter it, but this results in yet another non-matching string: should return false for \"\'\'\".

I've created a minimal reproduction repo: https://github.com/xfh/karma-sonarqube-reporter-string-escaping

When you run npm test you will see the output:
Test file path not found! {"src/escapes-spec.js":{"describe":["escaped quotes"],"it":["should return false for \"\\'\\'\""]}} | escaped quotes | should return false for "''"

@fadc80
Copy link
Owner

fadc80 commented May 30, 2019

Thank you. I'm going to take a look at it during the next weekend. If you have found a solution, feel free to send a PR.

@fadc80
Copy link
Owner

fadc80 commented Jun 2, 2019

@xfh I created a new branch with a correction. Could you verify if it was effective, please? I think you can check it out using the following commands:

git clone https://github.com/fadc80/karma-sonarqube-reporter.git
cd karma-sonarqube-reporter
git checkout fix-issue-12
npm install
cd ..
git clone https://github.com/xfh/karma-sonarqube-reporter-string-escaping.git
cd karma-sonarqube-reporter-string-escaping
npm install
npm link ../karma-sonarqube-reporter
npm run test

Expected result:

START:
escaped quotes
✔ should return false for "''"
Finished in 0.003 secs / 0.001 secs @ 23:16:15 GMT-0300 (Brasilia Standard Time)
SUMMARY:
✔ 1 test completed

Thanks!

@xfh
Copy link
Contributor Author

xfh commented Jun 3, 2019

Thanks, that fixed the problem!

I've run your new version also through my original project with 900 unit tests and nothing unexpected was reported. Looks good to me.

@fadc80 fadc80 closed this as completed in 6cfedd6 Jun 4, 2019
@fadc80
Copy link
Owner

fadc80 commented Nov 30, 2019

@all-contributors please add @maurycyg as a contributor for bug.

@allcontributors
Copy link
Contributor

@fadc80

I've put up a pull request to add @maurycyg! 🎉

@fadc80
Copy link
Owner

fadc80 commented Nov 30, 2019

@all-contributors please add @xfh as a contributor for bug.

@allcontributors
Copy link
Contributor

@fadc80

I've put up a pull request to add @xfh! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants