-
Notifications
You must be signed in to change notification settings - Fork 83
fix (unit tests): Fix event dispatcher missing in unit test #256
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
Conversation
tests on unhandled promise rejections.
@@ -0,0 +1,7 @@ | |||
process.on('unhandledRejection', function(err) { |
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.
Mind adding a comment in this explaining why this is here.
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.
Fix looks good, just a suggestion on placement and also licensing.
@@ -0,0 +1,11 @@ | |||
/* |
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.
Maybe we should move this into the lib
dir and further, into the tests
dir since this is only used for tests.
Also, please include the license header for this code file :)
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.
lgtm
Summary
Optimizely
was constructed without an event dispatcher, causing an error to be thrown asynchronously. This fixes the test, and updates thenpm test
command to make it easier to catch such mistakes - adding a listener for'unhandledRejection'
onprocess
that callsprocess.exit
.Test plan
Run unit tests, verify log output.