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

exports.clean method erring out when arg is undefined #2256

Closed
devangnegandhi opened this issue May 15, 2016 · 4 comments
Closed

exports.clean method erring out when arg is undefined #2256

devangnegandhi opened this issue May 15, 2016 · 4 comments

Comments

@devangnegandhi
Copy link

Hi,
I am using mocha along with karma (with client reporting turned on) and I have a global afterEach method that is called after every tests. I do some checks within this afterEach method, but since the actual test passed before reaching this afterEach, the following line in mocha.js errors out:

https://github.com/mochajs/mocha/blob/master/mocha.js#L2518

The above line is passing in test.body to the clean method. But since my afterEach is outside of any tests, the test.body variable is undefined causing the clean method to error out with the following error:

mocha.js:5937 Uncaught TypeError: Cannot read property 'replace' of undefined

The fix is pretty simple - within the clean method, just check if the str is defined, else ignore it. Let me know if you want me to create a PR for this. I will be happy to do it

@ScottFreeCode
Copy link
Contributor

Arguably making clean a no-op for undefined parameters is only an indirect, incidental solution where the real problem is either that hooks are missing the body property or else that non-test errors are being put through the same kind of error reporting processing as test failures (I mean, obviously we want both to be reported, but if there are differences between them that affect how they can be reported then those need to be respected).

That being said, I think this may already be addressed, by #2115. Can you try running a test that gets this issue against master instead of the current release and see if it's resolved? Note that, since the in-browser mocha.js file is only updated with each release, you'll also have to delete the main mocha.js file and rebuild it with make to get the updates in the browser, or else come up with a test that reproduces the issue on the commandline and use that -- in the latter case you may even be able to install master from GitHub with npm using the format npm i mochajs/mocha. (It's also possible the failure, once it is reported rather than throwing this exception, may be incorrectly labelled in the report due to #2220... although I think that won't apply to errors in hooks, which is what this case sounds like, so it's unlikely to be an issue.)

@devangnegandhi
Copy link
Author

you are right, #2115 does fix this issue. But is there a timeline of when this would be released to npm?

@ScottFreeCode
Copy link
Contributor

I think it's just one of those things where there's only one blocking issue (specifically, in this case, getting in-browser testing for the library that should ensure changes don't break in-browser use) but it's impossible to predict how long it will take to resolve given that it's more about managing the unforeseeable complications than about designing and implementing.

@boneskull
Copy link
Contributor

this sounds OK to close. we're nearing merging a solution to #2079. please reopen if not

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