Skip to content

fix(check): fix check via API not passing results to the callback #563

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 30, 2018
Merged

fix(check): fix check via API not passing results to the callback #563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lib/commands/on-complete.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
var assert = require('assert');
var log = require('db-migrate-shared').log;

module.exports = function (migrator, internals, callback, originalErr) {
module.exports = function (migrator, internals, callback, originalErr, results) {
if (typeof callback !== 'function') {
originalErr = originalErr || callback;
results = results || originalErr;
Copy link
Author

Choose a reason for hiding this comment

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

@wzrdtales I removed the change from line 6 as you requested.

That said, I don't understand under what circumstances callback is provided yet is not a function.

I'm concerned about leaving line 6 in its original state because if callback is not provided and results are, then originalErr will contain the results and not an error.

Copy link
Author

Choose a reason for hiding this comment

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

LMK if that doesn't make sense to you, not sure if I worded that as clearly as I could have...

Copy link
Member

Choose a reason for hiding this comment

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

If there is an error, there will be no results though :)

Copy link
Member

Choose a reason for hiding this comment

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

And yeah ok, the callback thing, that would need to fit with the condition if the callback is there or not, which is the only unique circumstance here.

Copy link
Member

Choose a reason for hiding this comment

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

But if no callback is passed, that should be valid anyway, since you do not need the results even if they're existent, since you do not deliver them.

Copy link
Author

Choose a reason for hiding this comment

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

Hah! That is true. Still seems a bit fragile to me but I'm OK with it if you are :)

}

migrator.driver.close(function (err) {
Expand All @@ -22,7 +23,7 @@ module.exports = function (migrator, internals, callback, originalErr) {
}

if (typeof callback === 'function') {
callback();
callback(null, results);
}
});
};
27 changes: 27 additions & 0 deletions test/on_complete_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
var Code = require('code');
var Lab = require('lab');
var lab = (exports.lab = Lab.script());
var onComplete = require('../lib/commands/on-complete');

lab.experiment('on-complete', function () {
lab.test('should invoke the callback with the results', function (done) {
var migratorMock = {
driver: {
close: function (cb) {
cb();
}
}
};
var internalsMock = {
argv: { }
};

var resultsPassedToCallback = 'callback should be invoked with results';
var testCallback = function (err, res) {
Code.expect(err).to.equal(null);
Code.expect(res).to.equal(resultsPassedToCallback);
done();
};
onComplete(migratorMock, internalsMock, testCallback, null, resultsPassedToCallback);
});
});