Skip to content
This repository has been archived by the owner on Aug 30, 2021. It is now read-only.

#450 Fix for hanging gulp on mongoose connections being left open #453

Merged
merged 5 commits into from
Mar 9, 2015

Conversation

reblace
Copy link
Contributor

@reblace reblace commented Mar 6, 2015

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.

reporter: 'spec'
}))
.on('error', function (err) {
error = new Error('Mocha tests failed');
Copy link
Member

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?

Copy link
Contributor Author

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.

@ilanbiala ilanbiala self-assigned this Mar 6, 2015
@ilanbiala
Copy link
Member

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?

@reblace
Copy link
Contributor Author

reblace commented Mar 6, 2015

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.

error = err;
}).on('end', function() {
// When the tests are done, disconnect mongoose and pass the error state back to gulp
mongoose.disconnect(function(){
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix spacing.

@ilanbiala
Copy link
Member

Can you make sure that the code is formatted according to our editorconfig and jshintrc? Spacing and tabs are biggest priorities.

@ilanbiala
Copy link
Member

LGTM.

ilanbiala added a commit that referenced this pull request Mar 9, 2015
Fix hanging gulp because mongoose connections are left open. Fixes #450.
@ilanbiala ilanbiala merged commit cabb4c4 into meanjs:0.4.0 Mar 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants