-
Notifications
You must be signed in to change notification settings - Fork 2k
#450 Fix for hanging gulp on mongoose connections being left open #453
Conversation
reporter: 'spec' | ||
})) | ||
.on('error', function (err) { | ||
error = new Error('Mocha tests failed'); |
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.
How come we aren't using err
to create the error
variable?
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.
Actually, it's better to use the err value. I'll update the PR.
I'm fine with the change, it's actually what I've been doing in my projects before MEAN.js added Gulp support. What happens when you don't have the error listener? |
You want to report the error still so that a CI server will be able to see that the build failed if some of the tests failed. Karma already reports the error because in the karma task you return a handle to the stream. The mocha task needs to manually send the error back up to gulp since you are not returning the reference to the stream. |
…ining what's going on in the mocha task.
error = err; | ||
}).on('end', function() { | ||
// When the tests are done, disconnect mongoose and pass the error state back to gulp | ||
mongoose.disconnect(function(){ |
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 spacing.
Can you make sure that the code is formatted according to our editorconfig and jshintrc? Spacing and tabs are biggest priorities. |
LGTM. |
Fix hanging gulp because mongoose connections are left open. Fixes #450.
I updated the gulp build so that the mongoose connect/disconnect take place in the mocha task based on the lifecycle of the stream. This way, we can guarantee that the disconnect function is always called regardless of if the mocha tests fail or not. I'm also sending an error message to the task callback in the event that an error is generated by the tests. This way, gulp will finish in an error state if the tests fail and will finish in an ok state if they pass. I've tested this with passing and failing tests and the gulp build closes correctly now.