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

Remove uncaught exception errors #94

Merged
merged 1 commit into from
Nov 23, 2016

Conversation

inukshuk
Copy link
Collaborator

Mocha registers exception handlers (work in main and renderer) to handle async test failures. If we handle uncaught exceptions (and exit) the test suite will not run to completion. See #93

I think the best approach is actually for us to not handle uncaught errors at all since Mocha will handle them anyway. I have not had time to add tests for this (would be nice, but requires some effort, because it would have to be an integration test to be useful really). But I tested this with some of my own projects and made sure that if I throw errors in a test or hook they're picked up, so I hope this is good to merge.

Mocha registers exception handlers (work in main and renderer) to
handle async test failures.

Fix #93
@jprichardson jprichardson merged commit 6cebd71 into master Nov 23, 2016
@jprichardson
Copy link
Owner

LGTM. Possible breaking change?

@inukshuk inukshuk deleted the fix-exit-on-uncaught-exceptions branch November 23, 2016 10:00
@inukshuk
Copy link
Collaborator Author

I just realized that we should retain the error handling for supporting modules (--require and --preload) -- once we bring those back I think it'll be good to go with minimal impact.

inukshuk added a commit that referenced this pull request Nov 23, 2016
Catch errors thrown synchronously by --require, --require-main
or --preload scripts. Mocha.run will listen for uncaught errors
so we ignore them here.

See #93 #94
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

Successfully merging this pull request may close these issues.

2 participants