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

Replacing current working directory in stack trace message not always working as intended. #3093

Closed
zrbecker opened this issue Nov 3, 2017 · 3 comments
Labels
good first issue new contributors should look here! status: accepting prs Mocha can use your help with this one!

Comments

@zrbecker
Copy link

zrbecker commented Nov 3, 2017

I had the following test fail when running npm test in a docker container where I had mounting the mocha source at /local.
https://github.com/mochajs/mocha/blob/master/test/unit/runner.spec.js#L406

The issue is here.

line = line.replace(cwd, '');

The replace call, causes '/usr/local/dev' to be changed to '/usrdev/, and the string comparison fails in the assert.

I imagine you would have to do a fancier regex replacement to get this to work correctly. In practice this might not be a huge issue since users tend to be in /home/username/path which should be reasonably unique.

However, I can imagine a situation where some files being stored at /tmp/<random_string>/home/username/path/filename or a similar location would occur in a stack trace message and be replaced to /tmp/<random_string>/filename.

@ScottFreeCode
Copy link
Contributor

Hmm... I can't think of a reason other than temporary files to be running JS outside the nice little project-local stack managed by NPM, and I am having a hard time imagining a temporary file reusing the project's path (wherever the project is) inside the temporary files directory, but...

If there's a standard way for a JS regex built from strings to escape one particular part of the regex, we'd just need to escape cwd that way and then prepend "^" to it, I believe, so that seems like it would be easy enough provided we aren't writing custom regex voodoo.

@ScottFreeCode ScottFreeCode added good first issue new contributors should look here! status: accepting prs Mocha can use your help with this one! labels Nov 5, 2017
@ScottFreeCode
Copy link
Contributor

Followup -- I don't know about a standard way, but elsewhere in Mocha (e.g. the fgrep function) we use the var escapeRe = require('escape-string-regexp'); package, like so: new RegExp(escapeRe(string)) -- presumably line = line.replace(escapeRe(cwd), ''); would do? (It'd be nice to get an integration test for the filepath formatting in this type of case, and thereby confirm that the fix works.)

@boneskull
Copy link
Contributor

closed via #3244

This was referenced Sep 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue new contributors should look here! status: accepting prs Mocha can use your help with this one!
Projects
None yet
Development

No branches or pull requests

3 participants