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

0.4.0 'gulp test' hangs because mongoose connections are staying open #450

Closed
reblace opened this issue Mar 6, 2015 · 14 comments
Closed
Assignees

Comments

@reblace
Copy link
Contributor

reblace commented Mar 6, 2015

The mongoose connection opened as part of the 'test' task in the gulp build is never closed. This causes gulp to hang at the completion of the tests.

@ilanbiala ilanbiala self-assigned this Mar 6, 2015
ilanbiala added a commit that referenced this issue Mar 6, 2015
@ilanbiala
Copy link
Member

Does the referenced commit and changes made in gulpfile.js and config/lib/mongoose.js fix the issue?.

@reblace
Copy link
Contributor Author

reblace commented Mar 6, 2015

Your commit only references the gulpfile.

As a modification to your commit, I think you want to pass a callback into the disconnect method on the mongoose connection so you can call the gulp done method when the disconnect is complete. Eg:

mongoose.disconnect(function(err) {
done(err);
});

Then, you can have the config/lib/mongoose.js file do something like this:

module.exports.disconnect = function(cb) {
db.disconnect(function(err) {
cb(err);
});
};

But otherwise, yes this is the same thing I did to fix the issue.

@ilanbiala
Copy link
Member

Fixed gulp tracking in the latest commit. Mongoose lib disconnect method was already there before.

@reblace
Copy link
Contributor Author

reblace commented Mar 6, 2015

You can't do this:

mongoose.connect(done);

because gulp interprets any parameter to the callback as an error. So, in this case, the build immedaitely fails on the openMongoose task when the mongoose.connect callback function passes the db as a parameter.

So, you need to do this instead on the openMongoose task:

mongoose.connect(function(db) {
done();
});

@ilanbiala
Copy link
Member

The build passes.

@reblace
Copy link
Contributor Author

reblace commented Mar 6, 2015

[email protected] test /home/travis/build/meanjs/mean
grunt test

It isn't running the gulp build.

@ilanbiala
Copy link
Member

Out of curiosity, what error does Gulp throw because of that?

@reblace
Copy link
Contributor Author

reblace commented Mar 6, 2015

Gulp fails because the task is reporting an error:

[22:53:28] 'openMongoose' errored after 247 ms
[22:53:28] Error: [object Object]
at formatError (/usr/local/lib/node_modules/gulp/bin/gulp.js:161:10)
at Gulp. (/usr/local/lib/node_modules/gulp/bin/gulp.js:187:15)
at Gulp.emit (events.js:129:20)

The '[object Object]' being referenced in the error is the db object returned by the mongoose connect function. Gulp's docs specify that calling the callback function with any parameter is interpreted as an error.

gulp.task('one', function(cb) {
// do stuff -- async or otherwise
cb(err); // if err is not null and not undefined, the run will stop, and note that it failed
});

Since you are effectively telling gulp that task has failed, it doesn't even both to run the tests. Since the Mongoose db connection is still open at that point, it still hangs.

You can check out the branch and do "gulp test" to reproduce.

@ilanbiala
Copy link
Member

Fixed in the latest commit.

@reblace
Copy link
Contributor Author

reblace commented Mar 6, 2015

Fixed.

@reblace reblace closed this as completed Mar 6, 2015
@reblace reblace reopened this Mar 6, 2015
@reblace
Copy link
Contributor Author

reblace commented Mar 6, 2015

Just discovered that this issue isn't entirely fixed. The remaining issue is that when the tests fail, the 'closeMongoose' task isn't called, so the build still hangs.

One solution is to suppress errors on both the mocha and karma build tasks... eg:

// Mocha tests task
gulp.task('mocha', function () {
    return gulp.src(testAssets.tests.server)
        .pipe(plugins.mocha({
            reporter: 'spec'
        }))
        .on('error', function (err) {
            // Make sure failed tests don't cause gulp to exit, otherwise Mongoose won't cleanup
            this.emit('end');
        });
});

// Karma test runner task
gulp.task('karma', function (done) {
    return gulp.src([])
        .pipe(plugins.karma({
            configFile: 'karma.conf.js',
            action: 'run',
            singleRun: true
        }))
        .on('error', function (err) {
            // Make sure failed tests cause gulp to exit non-zero
            this.emit('end');
        });
});

If you do that, then the gulp tasks will run to completion and the 'closeMongoose' task will run.

But, the issue with this approach is I don't think a CI server will know that the tests failed when they do.

Other possible solutions include making the tests themselves create and close the mongoose connections in the pre/post hooks. Or, to add some code that manually invokes the closeMongoose task in the event of an error and then fails out of the build.

@ilanbiala
Copy link
Member

I think a big issue is using runSequence because that doesn't seem to handle error propagation properly. Can you please format your code with syntax highlighting and proper indentation using markdown?

reblace added a commit to reblace/mean that referenced this issue Mar 6, 2015
@reblace
Copy link
Contributor Author

reblace commented Mar 6, 2015

Submitted a pull request for fixing this: #453

I changed the gulpfile so the mocha task calls mongoose.disconnect regardless of if the mocha tests pass or fail and passes the error back to the done callback if it exists. I should note, I've tested with both passing and failing mocha tests and karma tests.

reblace added a commit to reblace/mean that referenced this issue Mar 6, 2015
reblace added a commit to reblace/mean that referenced this issue Mar 7, 2015
reblace added a commit to reblace/mean that referenced this issue Mar 7, 2015
ilanbiala added a commit that referenced this issue Mar 9, 2015
Fix hanging gulp because mongoose connections are left open. Fixes #450.
@ilanbiala
Copy link
Member

Just merged #453 in, so this should be fixed.

pdfowler pushed a commit to pdfowler/mean that referenced this issue Jan 19, 2016
* commit 'a0495eabbd773ea642468ad77cfd28de0cfe3dab':
  Bump glob to version 5.0
  Correctly encode and decode password salt
  meanjs#450 minor formatting fixes.
  meanjs#450 Fixing unrelated jshint warnings
  removed unused gulp-watch dependency
  meanjs#450 Use the error reported by mocha. Added some comments explaining what's going on in the mocha task.
  meanjs#450 Now the mocha task synchronously calls mongoose connect and disconnect.
  Fix Gulp throwing errors
  Cleanly track mongoose connection in test task sequence
  Properly track DB disconnect
  Gulp now closes the mongoose connection
  Disconnect method to close DB connection
  update gulp-sass to ensure node-0.12 compatibility

Conflicts:
	config/config.js
	gulpfile.js
	package.json
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants