From 681e9f9f51f99fb849816a8b8acb7dafe1b6b0ff Mon Sep 17 00:00:00 2001 From: Hannah Wolfe Date: Wed, 23 Mar 2016 15:02:40 +0000 Subject: [PATCH] Data & fixture migration cleanup refs #6621, #6622 - remove unneeded `return new Promise.resolve()` lines - reduce code in tests - improve quality of tests checking that all task functions are executed - add missing test coverage --- .../fixtures/004/01-move-jquery-with-alert.js | 2 +- .../004/02-update-private-setting-type.js | 2 - .../004/03-update-password-setting-type.js | 2 - .../004/04-update-ghost-admin-client.js | 2 - .../004/05-add-ghost-frontend-client.js | 2 - .../fixtures/004/06-clean-broken-tags.js | 1 - core/test/unit/migration_fixture_spec.js | 455 ++++++++---------- core/test/unit/migration_spec.js | 245 ++++++---- 8 files changed, 369 insertions(+), 342 deletions(-) diff --git a/core/server/data/migration/fixtures/004/01-move-jquery-with-alert.js b/core/server/data/migration/fixtures/004/01-move-jquery-with-alert.js index 9248ad5e51..87bda1ed17 100644 --- a/core/server/data/migration/fixtures/004/01-move-jquery-with-alert.js +++ b/core/server/data/migration/fixtures/004/01-move-jquery-with-alert.js @@ -24,7 +24,7 @@ module.exports = function moveJQuery(options, logger) { return models.Settings.findOne('ghost_foot').then(function (setting) { if (setting) { - value = setting.attributes.value; + value = setting.get('value'); // Only add jQuery if it's not already in there if (value.indexOf(jquery.join('')) === -1) { logger.info(message); diff --git a/core/server/data/migration/fixtures/004/02-update-private-setting-type.js b/core/server/data/migration/fixtures/004/02-update-private-setting-type.js index 19b7d0723d..43a153edb1 100644 --- a/core/server/data/migration/fixtures/004/02-update-private-setting-type.js +++ b/core/server/data/migration/fixtures/004/02-update-private-setting-type.js @@ -1,6 +1,5 @@ // Update the `isPrivate` setting, so that it has a type of `private` rather than `blog` var models = require('../../../../models'), - Promise = require('bluebird'), message = 'Update isPrivate setting'; @@ -12,6 +11,5 @@ module.exports = function updatePrivateSetting(options, logger) { } else { logger.warn(message); } - return Promise.resolve(); }); }; diff --git a/core/server/data/migration/fixtures/004/03-update-password-setting-type.js b/core/server/data/migration/fixtures/004/03-update-password-setting-type.js index b08c77064e..f650a47d1b 100644 --- a/core/server/data/migration/fixtures/004/03-update-password-setting-type.js +++ b/core/server/data/migration/fixtures/004/03-update-password-setting-type.js @@ -1,6 +1,5 @@ // Update the `password` setting, so that it has a type of `private` rather than `blog` var models = require('../../../../models'), - Promise = require('bluebird'), message = 'Update password setting'; @@ -12,6 +11,5 @@ module.exports = function updatePasswordSetting(options, logger) { } else { logger.warn(message); } - return Promise.resolve(); }); }; diff --git a/core/server/data/migration/fixtures/004/04-update-ghost-admin-client.js b/core/server/data/migration/fixtures/004/04-update-ghost-admin-client.js index 23b2e15798..ae473ecb5b 100644 --- a/core/server/data/migration/fixtures/004/04-update-ghost-admin-client.js +++ b/core/server/data/migration/fixtures/004/04-update-ghost-admin-client.js @@ -1,7 +1,6 @@ // Update the `ghost-admin` client so that it has a proper secret var models = require('../../../../models'), _ = require('lodash'), - Promise = require('bluebird'), crypto = require('crypto'), adminClient = require('../fixtures').models.Client[0], @@ -19,6 +18,5 @@ module.exports = function updateGhostAdminClient(options, logger) { } else { logger.warn(message); } - return Promise.resolve(); }); }; diff --git a/core/server/data/migration/fixtures/004/05-add-ghost-frontend-client.js b/core/server/data/migration/fixtures/004/05-add-ghost-frontend-client.js index 14eb211be8..5efccdd972 100644 --- a/core/server/data/migration/fixtures/004/05-add-ghost-frontend-client.js +++ b/core/server/data/migration/fixtures/004/05-add-ghost-frontend-client.js @@ -1,6 +1,5 @@ // Create a new `ghost-frontend` client for use in themes var models = require('../../../../models'), - Promise = require('bluebird'), frontendClient = require('../fixtures').models.Client[1], message = 'Add ghost-frontend client fixture'; @@ -13,6 +12,5 @@ module.exports = function addGhostFrontendClient(options, logger) { } else { logger.warn(message); } - return Promise.resolve(); }); }; diff --git a/core/server/data/migration/fixtures/004/06-clean-broken-tags.js b/core/server/data/migration/fixtures/004/06-clean-broken-tags.js index 939933d712..ad65dfd170 100644 --- a/core/server/data/migration/fixtures/004/06-clean-broken-tags.js +++ b/core/server/data/migration/fixtures/004/06-clean-broken-tags.js @@ -27,6 +27,5 @@ module.exports = function cleanBrokenTags(options, logger) { } else { logger.warn(message); } - return Promise.resolve(); }); }; diff --git a/core/test/unit/migration_fixture_spec.js b/core/test/unit/migration_fixture_spec.js index c8120c7c6e..bece2ed484 100644 --- a/core/test/unit/migration_fixture_spec.js +++ b/core/test/unit/migration_fixture_spec.js @@ -72,57 +72,62 @@ describe('Fixtures', function () { }); describe('Update to 004', function () { - it('should call all the 004 fixture upgrades', function (done) { - // Stub all the model methods so that nothing happens - var settingsOneStub = sandbox.stub(models.Settings, 'findOne').returns(Promise.resolve()), - settingsEditStub = sandbox.stub(models.Settings, 'edit').returns(Promise.resolve()), - clientOneStub = sandbox.stub(models.Client, 'findOne'), - clientEditStub = sandbox.stub(models.Client, 'edit').returns(Promise.resolve()), - clientAddStub = sandbox.stub(models.Client, 'add').returns(Promise.resolve()), - tagAllStub = sandbox.stub(models.Tag, 'findAll').returns(Promise.resolve()), - postAllStub = sandbox.stub(models.Post, 'findAll').returns(Promise.resolve()), - postOneStub = sandbox.stub(models.Post, 'findOne').returns(Promise.resolve({})), - postAddStub = sandbox.stub(models.Post, 'add').returns(Promise.resolve()); + it('should call all the 004 fixture upgrade tasks', function (done) { + // Setup + // Create a new stub, this will replace sequence, so that db calls don't actually get run + var sequenceStub = sandbox.stub(), + sequenceReset = update.__set__('sequence', sequenceStub); - clientOneStub.withArgs({slug: 'ghost-admin'}).returns(Promise.resolve()); - clientOneStub.withArgs({slug: 'ghost-frontend'}).returns(Promise.resolve({})); + // The first time we call sequence, it should be to execute a top level version, e.g 004 + // yieldsTo('0') means this stub will execute the function at index 0 of the array passed as the + // first argument. In short the `runVersionTasks` function gets executed, and sequence gets called + // again with the array of tasks to execute for 004, which is what we want to check + sequenceStub.onFirstCall().yieldsTo('0').returns(Promise.resolve([])); update(['004'], loggerStub).then(function (result) { should.exist(result); - // Gets called 3 times to output peripheral info - loggerStub.info.called.should.be.true(); - loggerStub.info.callCount.should.eql(3); - // Should be called 8 times, once for each skipped migration - loggerStub.warn.called.should.be.true(); - loggerStub.warn.callCount.should.eql(8); + loggerStub.info.calledTwice.should.be.true(); + loggerStub.warn.called.should.be.false(); - settingsOneStub.calledThrice.should.be.true(); - settingsEditStub.called.should.be.false(); - clientOneStub.calledTwice.should.be.true(); - clientEditStub.called.should.be.false(); - clientAddStub.called.should.be.false(); - tagAllStub.calledOnce.should.be.true(); - postAllStub.calledOnce.should.be.true(); - postOneStub.calledOnce.should.be.true(); - postAddStub.called.should.be.false(); + sequenceStub.calledTwice.should.be.true(); + sequenceStub.firstCall.calledWith(sinon.match.array, sinon.match.object, loggerStub).should.be.true(); + sequenceStub.secondCall.calledWith(sinon.match.array, sinon.match.object, loggerStub).should.be.true(); - sinon.assert.callOrder( - settingsOneStub, settingsOneStub, settingsOneStub, clientOneStub, clientOneStub, tagAllStub, - postAllStub, postOneStub - ); + sequenceStub.firstCall.args[0].should.be.an.Array().with.lengthOf(1); + sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(8); + sequenceStub.firstCall.args[0][0].should.be.a.Function().with.property('name', 'runVersionTasks'); + + sequenceStub.secondCall.args[0][0].should.be.a.Function().with.property('name', 'moveJQuery'); + sequenceStub.secondCall.args[0][1].should.be.a.Function().with.property('name', 'updatePrivateSetting'); + sequenceStub.secondCall.args[0][2].should.be.a.Function().with.property('name', 'updatePasswordSetting'); + sequenceStub.secondCall.args[0][3].should.be.a.Function().with.property('name', 'updateGhostAdminClient'); + sequenceStub.secondCall.args[0][4].should.be.a.Function().with.property('name', 'addGhostFrontendClient'); + sequenceStub.secondCall.args[0][5].should.be.a.Function().with.property('name', 'cleanBrokenTags'); + sequenceStub.secondCall.args[0][6].should.be.a.Function().with.property('name', 'addPostTagOrder'); + sequenceStub.secondCall.args[0][7].should.be.a.Function().with.property('name', 'addNewPostFixture'); + + // Reset + sequenceReset(); done(); }).catch(done); }); describe('Tasks:', function () { + var getObjStub, settingsOneStub, settingsEditStub, clientOneStub, clientEditStub; + + beforeEach(function () { + getObjStub = {get: sandbox.stub()}; + settingsOneStub = sandbox.stub(models.Settings, 'findOne').returns(Promise.resolve(getObjStub)); + settingsEditStub = sandbox.stub(models.Settings, 'edit').returns(Promise.resolve()); + clientOneStub = sandbox.stub(models.Client, 'findOne').returns(Promise.resolve(getObjStub)); + clientEditStub = sandbox.stub(models.Client, 'edit').returns(Promise.resolve()); + }); + describe('01-move-jquery-with-alert', function () { it('tries to move jQuery to ghost_foot', function (done) { - var settingsOneStub = sandbox.stub(models.Settings, 'findOne').returns(Promise.resolve({ - attributes: {value: ''} - })), - settingsEditStub = sandbox.stub(models.Settings, 'edit').returns(Promise.resolve()); + getObjStub.get.returns(''); fixtures004[0]({}, loggerStub).then(function () { settingsOneStub.calledOnce.should.be.true(); @@ -136,13 +141,10 @@ describe('Fixtures', function () { }); it('does not move jQuery to ghost_foot if it is already there', function (done) { - var settingsOneStub = sandbox.stub(models.Settings, 'findOne').returns(Promise.resolve({ - attributes: { - value: '\n' - + '\n\n' - } - })), - settingsEditStub = sandbox.stub(models.Settings, 'edit').returns(Promise.resolve()); + getObjStub.get.returns( + '\n' + + '\n\n' + ); fixtures004[0]({}, loggerStub).then(function () { settingsOneStub.calledOnce.should.be.true(); @@ -155,13 +157,24 @@ describe('Fixtures', function () { }).catch(done); }); + it('does not move jQuery to ghost_foot if the setting is missing', function (done) { + settingsOneStub.returns(Promise.resolve()); + + fixtures004[0]({}, loggerStub).then(function () { + settingsOneStub.calledOnce.should.be.true(); + settingsOneStub.calledWith('ghost_foot').should.be.true(); + settingsEditStub.called.should.be.false(); + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + it('tried to move jQuery AND add a privacy message if any privacy settings are on', function (done) { + var notificationsAddStub = sandbox.stub(notifications, 'add').returns(Promise.resolve()); configUtils.set({privacy: {useGoogleFonts: false}}); - var settingsOneStub = sandbox.stub(models.Settings, 'findOne').returns(Promise.resolve({ - attributes: {value: ''} - })), - settingsEditStub = sandbox.stub(models.Settings, 'edit').returns(Promise.resolve()), - notificationsAddStub = sandbox.stub(notifications, 'add').returns(Promise.resolve()); + getObjStub.get.returns(''); fixtures004[0]({}, loggerStub).then(function () { settingsOneStub.calledOnce.should.be.true(); @@ -178,45 +191,35 @@ describe('Fixtures', function () { describe('02-update-private-setting-type', function () { it('tries to update setting type correctly', function (done) { - var settingObjStub = {get: sandbox.stub()}, - settingsOneStub = sandbox.stub(models.Settings, 'findOne').returns( - Promise.resolve(settingObjStub) - ), - settingsEditStub = sandbox.stub(models.Settings, 'edit').returns(Promise.resolve()); - fixtures004[1]({}, loggerStub).then(function () { settingsOneStub.calledOnce.should.be.true(); settingsOneStub.calledWith('isPrivate').should.be.true(); - settingObjStub.get.calledOnce.should.be.true(); - settingObjStub.get.calledWith('type').should.be.true(); + getObjStub.get.calledOnce.should.be.true(); + getObjStub.get.calledWith('type').should.be.true(); settingsEditStub.calledOnce.should.be.true(); settingsEditStub.calledWith({key: 'isPrivate', type: 'private'}).should.be.true(); loggerStub.info.calledOnce.should.be.true(); loggerStub.warn.called.should.be.false(); - sinon.assert.callOrder(settingsOneStub, settingObjStub.get, loggerStub.info, settingsEditStub); + sinon.assert.callOrder(settingsOneStub, getObjStub.get, loggerStub.info, settingsEditStub); done(); }).catch(done); }); it('does not try to update setting type if it is already set', function (done) { - var settingObjStub = {get: sandbox.stub().returns('private')}, - settingsOneStub = sandbox.stub(models.Settings, 'findOne').returns( - Promise.resolve(settingObjStub) - ), - settingsEditStub = sandbox.stub(models.Settings, 'edit').returns(Promise.resolve()); + getObjStub.get.returns('private'); fixtures004[1]({}, loggerStub).then(function () { settingsOneStub.calledOnce.should.be.true(); settingsOneStub.calledWith('isPrivate').should.be.true(); - settingObjStub.get.calledOnce.should.be.true(); - settingObjStub.get.calledWith('type').should.be.true(); + getObjStub.get.calledOnce.should.be.true(); + getObjStub.get.calledWith('type').should.be.true(); settingsEditStub.called.should.be.false(); loggerStub.info.called.should.be.false(); loggerStub.warn.calledOnce.should.be.true(); - sinon.assert.callOrder(settingsOneStub, settingObjStub.get, loggerStub.warn); + sinon.assert.callOrder(settingsOneStub, getObjStub.get, loggerStub.warn); done(); }).catch(done); @@ -225,12 +228,6 @@ describe('Fixtures', function () { describe('03-update-password-setting-type', function () { it('tries to update setting type correctly', function (done) { - var settingObjStub = {get: sandbox.stub()}, - settingsOneStub = sandbox.stub(models.Settings, 'findOne').returns( - Promise.resolve(settingObjStub) - ), - settingsEditStub = sandbox.stub(models.Settings, 'edit').returns(Promise.resolve()); - fixtures004[2]({}, loggerStub).then(function () { settingsOneStub.calledOnce.should.be.true(); settingsOneStub.calledWith('password').should.be.true(); @@ -245,23 +242,19 @@ describe('Fixtures', function () { }); it('does not try to update setting type if it is already set', function (done) { - var settingObjStub = {get: sandbox.stub().returns('private')}, - settingsOneStub = sandbox.stub(models.Settings, 'findOne').returns( - Promise.resolve(settingObjStub) - ), - settingsEditStub = sandbox.stub(models.Settings, 'edit').returns(Promise.resolve()); + getObjStub.get.returns('private'); fixtures004[2]({}, loggerStub).then(function () { settingsOneStub.calledOnce.should.be.true(); settingsOneStub.calledWith('password').should.be.true(); - settingObjStub.get.calledOnce.should.be.true(); - settingObjStub.get.calledWith('type').should.be.true(); + getObjStub.get.calledOnce.should.be.true(); + getObjStub.get.calledWith('type').should.be.true(); settingsEditStub.called.should.be.false(); loggerStub.info.called.should.be.false(); loggerStub.warn.calledOnce.should.be.true(); - sinon.assert.callOrder(settingsOneStub, settingObjStub.get); + sinon.assert.callOrder(settingsOneStub, getObjStub.get); done(); }).catch(done); @@ -270,21 +263,17 @@ describe('Fixtures', function () { describe('04-update-ghost-admin-client', function () { it('tries to update client correctly', function (done) { - var clientObjStub = {get: sandbox.stub()}, - clientOneStub = sandbox.stub(models.Client, 'findOne').returns(Promise.resolve(clientObjStub)), - clientEditStub = sandbox.stub(models.Client, 'edit').returns(Promise.resolve()); - fixtures004[3]({}, loggerStub).then(function () { clientOneStub.calledOnce.should.be.true(); clientOneStub.calledWith({slug: 'ghost-admin'}).should.be.true(); - clientObjStub.get.calledTwice.should.be.true(); - clientObjStub.get.calledWith('secret').should.be.true(); - clientObjStub.get.calledWith('status').should.be.true(); + getObjStub.get.calledTwice.should.be.true(); + getObjStub.get.calledWith('secret').should.be.true(); + getObjStub.get.calledWith('status').should.be.true(); clientEditStub.calledOnce.should.be.true(); loggerStub.info.calledOnce.should.be.true(); loggerStub.warn.called.should.be.false(); sinon.assert.callOrder( - clientOneStub, clientObjStub.get, clientObjStub.get, loggerStub.info, clientEditStub + clientOneStub, getObjStub.get, getObjStub.get, loggerStub.info, clientEditStub ); done(); @@ -292,48 +281,40 @@ describe('Fixtures', function () { }); it('does not try to update client if the secret and status are already correct', function (done) { - var clientObjStub = {get: sandbox.stub()}, - clientOneStub = sandbox.stub(models.Client, 'findOne').returns(Promise.resolve(clientObjStub)), - clientEditStub = sandbox.stub(models.Client, 'edit').returns(Promise.resolve()); - - clientObjStub.get.withArgs('secret').returns('abc'); - clientObjStub.get.withArgs('status').returns('enabled'); + getObjStub.get.withArgs('secret').returns('abc'); + getObjStub.get.withArgs('status').returns('enabled'); fixtures004[3]({}, loggerStub).then(function () { clientOneStub.calledOnce.should.be.true(); clientOneStub.calledWith({slug: 'ghost-admin'}).should.be.true(); - clientObjStub.get.calledTwice.should.be.true(); - clientObjStub.get.calledWith('secret').should.be.true(); - clientObjStub.get.calledWith('status').should.be.true(); + getObjStub.get.calledTwice.should.be.true(); + getObjStub.get.calledWith('secret').should.be.true(); + getObjStub.get.calledWith('status').should.be.true(); clientEditStub.called.should.be.false(); loggerStub.info.called.should.be.false(); loggerStub.warn.calledOnce.should.be.true(); - sinon.assert.callOrder(clientOneStub, clientObjStub.get, clientObjStub.get, loggerStub.warn); + sinon.assert.callOrder(clientOneStub, getObjStub.get, getObjStub.get, loggerStub.warn); done(); }).catch(done); }); it('tries to update client if secret is correct but status is wrong', function (done) { - var clientObjStub = {get: sandbox.stub()}, - clientOneStub = sandbox.stub(models.Client, 'findOne').returns(Promise.resolve(clientObjStub)), - clientEditStub = sandbox.stub(models.Client, 'edit').returns(Promise.resolve()); - - clientObjStub.get.withArgs('secret').returns('abc'); - clientObjStub.get.withArgs('status').returns('development'); + getObjStub.get.withArgs('secret').returns('abc'); + getObjStub.get.withArgs('status').returns('development'); fixtures004[3]({}, loggerStub).then(function () { clientOneStub.calledOnce.should.be.true(); clientOneStub.calledWith({slug: 'ghost-admin'}).should.be.true(); - clientObjStub.get.calledTwice.should.be.true(); - clientObjStub.get.calledWith('secret').should.be.true(); - clientObjStub.get.calledWith('status').should.be.true(); + getObjStub.get.calledTwice.should.be.true(); + getObjStub.get.calledWith('secret').should.be.true(); + getObjStub.get.calledWith('status').should.be.true(); clientEditStub.calledOnce.should.be.true(); loggerStub.info.calledOnce.should.be.true(); loggerStub.warn.called.should.be.false(); sinon.assert.callOrder( - clientOneStub, clientObjStub.get, clientObjStub.get, loggerStub.info, clientEditStub + clientOneStub, getObjStub.get, getObjStub.get, loggerStub.info, clientEditStub ); done(); @@ -341,24 +322,20 @@ describe('Fixtures', function () { }); it('tries to update client if status is correct but secret is wrong', function (done) { - var clientObjStub = {get: sandbox.stub()}, - clientOneStub = sandbox.stub(models.Client, 'findOne').returns(Promise.resolve(clientObjStub)), - clientEditStub = sandbox.stub(models.Client, 'edit').returns(Promise.resolve()); - - clientObjStub.get.withArgs('secret').returns('not_available'); - clientObjStub.get.withArgs('status').returns('enabled'); + getObjStub.get.withArgs('secret').returns('not_available'); + getObjStub.get.withArgs('status').returns('enabled'); fixtures004[3]({}, loggerStub).then(function () { clientOneStub.calledOnce.should.be.true(); clientOneStub.calledWith({slug: 'ghost-admin'}).should.be.true(); - clientObjStub.get.calledOnce.should.be.true(); - clientObjStub.get.calledWith('secret').should.be.true(); + getObjStub.get.calledOnce.should.be.true(); + getObjStub.get.calledWith('secret').should.be.true(); clientEditStub.calledOnce.should.be.true(); loggerStub.info.calledOnce.should.be.true(); loggerStub.warn.called.should.be.false(); sinon.assert.callOrder( - clientOneStub, clientObjStub.get, loggerStub.info, clientEditStub + clientOneStub, getObjStub.get, loggerStub.info, clientEditStub ); done(); @@ -368,8 +345,8 @@ describe('Fixtures', function () { describe('05-add-ghost-frontend-client', function () { it('tries to add client correctly', function (done) { - var clientOneStub = sandbox.stub(models.Client, 'findOne').returns(Promise.resolve()), - clientAddStub = sandbox.stub(models.Client, 'add').returns(Promise.resolve()); + var clientAddStub = sandbox.stub(models.Client, 'add').returns(Promise.resolve()); + clientOneStub.returns(Promise.resolve()); fixtures004[4]({}, loggerStub).then(function () { clientOneStub.calledOnce.should.be.true(); @@ -384,8 +361,7 @@ describe('Fixtures', function () { }); it('does not try to add client if it already exists', function (done) { - var clientOneStub = sandbox.stub(models.Client, 'findOne').returns(Promise.resolve({})), - clientAddStub = sandbox.stub(models.Client, 'add').returns(Promise.resolve()); + var clientAddStub = sandbox.stub(models.Client, 'add').returns(Promise.resolve()); fixtures004[4]({}, loggerStub).then(function () { clientOneStub.calledOnce.should.be.true(); @@ -400,13 +376,19 @@ describe('Fixtures', function () { }); describe('06-clean-broken-tags', function () { + var tagObjStub, tagCollStub, tagAllStub; + + beforeEach(function () { + tagObjStub = { + get: sandbox.stub(), + save: sandbox.stub().returns(Promise.resolve) + }; + tagCollStub = {each: sandbox.stub().callsArgWith(0, tagObjStub)}; + tagAllStub = sandbox.stub(models.Tag, 'findAll').returns(Promise.resolve(tagCollStub)); + }); + it('tries to clean broken tags correctly', function (done) { - var tagObjStub = { - get: sandbox.stub().returns(',hello'), - save: sandbox.stub().returns(Promise.resolve) - }, - tagCollStub = {each: sandbox.stub().callsArgWith(0, tagObjStub)}, - tagAllStub = sandbox.stub(models.Tag, 'findAll').returns(Promise.resolve(tagCollStub)); + tagObjStub.get.returns(',hello'); fixtures004[5]({}, loggerStub).then(function () { tagAllStub.calledOnce.should.be.true(); @@ -424,12 +406,7 @@ describe('Fixtures', function () { }); it('tries can handle tags which end up empty', function (done) { - var tagObjStub = { - get: sandbox.stub().returns(','), - save: sandbox.stub().returns(Promise.resolve) - }, - tagCollStub = {each: sandbox.stub().callsArgWith(0, tagObjStub)}, - tagAllStub = sandbox.stub(models.Tag, 'findAll').returns(Promise.resolve(tagCollStub)); + tagObjStub.get.returns(','); fixtures004[5]({}, loggerStub).then(function () { tagAllStub.calledOnce.should.be.true(); @@ -447,12 +424,7 @@ describe('Fixtures', function () { }); it('does not change tags if not necessary', function (done) { - var tagObjStub = { - get: sandbox.stub().returns('hello'), - save: sandbox.stub().returns(Promise.resolve) - }, - tagCollStub = {each: sandbox.stub().callsArgWith(0, tagObjStub)}, - tagAllStub = sandbox.stub(models.Tag, 'findAll').returns(Promise.resolve(tagCollStub)); + tagObjStub.get.returns('hello'); fixtures004[5]({}, loggerStub).then(function () { tagAllStub.calledOnce.should.be.true(); @@ -462,7 +434,23 @@ describe('Fixtures', function () { tagObjStub.save.called.should.be.false(); loggerStub.info.called.should.be.false(); loggerStub.warn.calledOnce.should.be.true(); - sinon.assert.callOrder(tagAllStub, tagCollStub.each, tagObjStub.get); + sinon.assert.callOrder(tagAllStub, tagCollStub.each, tagObjStub.get, loggerStub.warn); + + done(); + }).catch(done); + }); + + it('does nothing if there are no tags', function (done) { + tagAllStub.returns(Promise.resolve()); + + fixtures004[5]({}, loggerStub).then(function () { + tagAllStub.calledOnce.should.be.true(); + tagCollStub.each.called.should.be.false(); + tagObjStub.get.called.should.be.false(); + tagObjStub.save.called.should.be.false(); + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + sinon.assert.callOrder(tagAllStub, loggerStub.warn); done(); }).catch(done); @@ -470,13 +458,31 @@ describe('Fixtures', function () { }); describe('07-add-post-tag-order', function () { - it('calls load on each post', function (done) { - var postObjStub = { - load: sandbox.stub() - }, - postCollStub = {mapThen: sandbox.stub().callsArgWith(0, postObjStub).returns([])}, - postAllStub = sandbox.stub(models.Post, 'findAll').returns(Promise.resolve(postCollStub)); + var tagOp1Stub, tagOp2Stub, tagObjStub, postObjStub, postCollStub, postAllStub; + beforeEach(function () { + tagOp1Stub = sandbox.stub().returns(Promise.resolve()); + tagOp2Stub = sandbox.stub().returns(Promise.resolve()); + tagObjStub = { + pivot: {get: sandbox.stub()} + }; + postCollStub = {mapThen: sandbox.stub()}; + postAllStub = sandbox.stub(models.Post, 'findAll').returns(Promise.resolve(postCollStub)); + + postObjStub = { + load: sandbox.stub(), + reduce: sandbox.stub(), + // By returning an array from related, we can use native reduce to simulate a result + related: sandbox.stub().returns([tagObjStub]), + // Get called when executing sequence + tags: sandbox.stub().returnsThis(), + updatePivot: sandbox.stub().returns(Promise.resolve()) + }; + }); + + it('calls load on each post', function (done) { + // Fake mapThen behaviour + postCollStub.mapThen.callsArgWith(0, postObjStub).returns([]); fixtures004[6]({}, loggerStub).then(function () { postAllStub.calledOnce.should.be.true(); postCollStub.mapThen.calledOnce.should.be.true(); @@ -492,7 +498,9 @@ describe('Fixtures', function () { }); it('returns early, if no posts are found', function (done) { - var postAllStub = sandbox.stub(models.Post, 'findAll').returns(Promise.resolve()); + // Fake mapThen behaviour + postCollStub.mapThen.returns([]); + postAllStub.returns(Promise.resolve()); fixtures004[6]({}, loggerStub).then(function () { loggerStub.info.calledOnce.should.be.true(); @@ -506,25 +514,23 @@ describe('Fixtures', function () { it('executes sequence, if at least one tag is found', function (done) { var tagOpStub = sandbox.stub().returns(Promise.resolve()), - tagOpsArr = [tagOpStub], - // By stubbing reduce, we can return an array directly without pretending to process tags - postArrayReduceStub = { - reduce: sandbox.stub().returns(tagOpsArr) - }, - // By returning from mapThen, we can skip doing tag.load in this test - postCollStub = {mapThen: sandbox.stub().returns(postArrayReduceStub)}, - postAllStub = sandbox.stub(models.Post, 'findAll').returns(Promise.resolve(postCollStub)); + tagOpsArr = [tagOpStub]; + + // By stubbing reduce, we can return an array directly without pretending to process tags + postObjStub.reduce.returns(tagOpsArr); + // By returning from mapThen, we can skip doing tag.load in this test + postCollStub.mapThen.returns(postObjStub); fixtures004[6]({}, loggerStub).then(function () { loggerStub.info.calledThrice.should.be.true(); loggerStub.warn.called.should.be.false(); postAllStub.calledOnce.should.be.true(); postCollStub.mapThen.calledOnce.should.be.true(); - postArrayReduceStub.reduce.calledOnce.should.be.true(); + postObjStub.reduce.calledOnce.should.be.true(); tagOpStub.calledOnce.should.be.true(); sinon.assert.callOrder( - loggerStub.info, postAllStub, postCollStub.mapThen, postArrayReduceStub.reduce, + loggerStub.info, postAllStub, postCollStub.mapThen, postObjStub.reduce, loggerStub.info, tagOpStub, loggerStub.info ); @@ -533,28 +539,23 @@ describe('Fixtures', function () { }); it('executes sequence, if more than one tag is found', function (done) { - var tagOp1Stub = sandbox.stub().returns(Promise.resolve()), - tagOp2Stub = sandbox.stub().returns(Promise.resolve()), - tagOpsArr = [tagOp1Stub, tagOp2Stub], - // By stubbing reduce, we can return an array directly without pretending to process tags - postArrayReduceStub = { - reduce: sandbox.stub().returns(tagOpsArr) - }, - // By returning from mapThen, we can skip doing tag.load in this test - postCollStub = {mapThen: sandbox.stub().returns(postArrayReduceStub)}, - postAllStub = sandbox.stub(models.Post, 'findAll').returns(Promise.resolve(postCollStub)); + var tagOpsArr = [tagOp1Stub, tagOp2Stub]; + // By stubbing reduce, we can return an array directly without pretending to process tags + postObjStub.reduce.returns(tagOpsArr); + // By returning from mapThen, we can skip doing tag.load in this test + postCollStub.mapThen.returns(postObjStub); fixtures004[6]({}, loggerStub).then(function () { loggerStub.info.calledThrice.should.be.true(); loggerStub.warn.called.should.be.false(); postAllStub.calledOnce.should.be.true(); postCollStub.mapThen.calledOnce.should.be.true(); - postArrayReduceStub.reduce.calledOnce.should.be.true(); + postObjStub.reduce.calledOnce.should.be.true(); tagOp1Stub.calledOnce.should.be.true(); tagOp2Stub.calledOnce.should.be.true(); sinon.assert.callOrder( - loggerStub.info, postAllStub, postCollStub.mapThen, postArrayReduceStub.reduce, + loggerStub.info, postAllStub, postCollStub.mapThen, postObjStub.reduce, loggerStub.info, tagOp1Stub, tagOp2Stub, loggerStub.info ); @@ -563,19 +564,9 @@ describe('Fixtures', function () { }); it('does not execute sequence, if migrationHasRunFlag gets set to true', function (done) { - var tagObjStub = { - pivot: { - // If pivot gets a non-zero, migrationHasRunFlag gets set to true - get: sandbox.stub().returns(1) - } - }, - postObjStub = { - // By returning an array from related, we can use real reduce to simulate a result here - related: sandbox.stub().returns([tagObjStub]) - }, - // By returning from mapThen, we can skip doing tag.load in this test - postCollStub = {mapThen: sandbox.stub().returns([postObjStub])}, - postAllStub = sandbox.stub(models.Post, 'findAll').returns(Promise.resolve(postCollStub)); + tagObjStub.pivot.get.returns(1); + // By returning from mapThen, we can skip doing tag.load in this test + postCollStub.mapThen.returns([postObjStub]); fixtures004[6]({}, loggerStub).then(function () { loggerStub.info.calledOnce.should.be.true(); @@ -595,23 +586,10 @@ describe('Fixtures', function () { }); it('does execute sequence, if migrationHasRunFlag is false', function (done) { - var tagObjStub = { - pivot: { - // If pivot gets a non-zero, migrationHasRunFlag gets set to true - get: sandbox.stub().returns(0) - } - }, - postObjStub = { - // By returning an array from related, we can use real reduce to simulate a result here - related: sandbox.stub().returns([tagObjStub]), - - // Get called when executing the sequence - tags: sandbox.stub().returnsThis(), - updatePivot: sandbox.stub().returns(Promise.resolve()) - }, - // By returning from mapThen, we can skip doing tag.load in this test - postCollStub = {mapThen: sandbox.stub().returns([postObjStub])}, - postAllStub = sandbox.stub(models.Post, 'findAll').returns(Promise.resolve(postCollStub)); + // If pivot gets a non-zero, migrationHasRunFlag gets set to true + tagObjStub.pivot.get.returns(0); + // By returning from mapThen, we can skip doing tag.load in this test + postCollStub.mapThen.returns([postObjStub]); fixtures004[6]({}, loggerStub).then(function () { loggerStub.info.calledThrice.should.be.true(); @@ -634,23 +612,12 @@ describe('Fixtures', function () { }); it('tries to add incremental sort_order to posts_tags', function (done) { - var tagObjStub = { - pivot: { - // If pivot gets a non-zero, migrationHasRunFlag gets set to true - get: sandbox.stub().returns(0) - } - }, - postObjStub = { - // By returning an array from related, we can use real reduce to simulate a result here - related: sandbox.stub().returns([tagObjStub, tagObjStub, tagObjStub]), - - // Get called when executing the sequence - tags: sandbox.stub().returnsThis(), - updatePivot: sandbox.stub().returns(Promise.resolve()) - }, + // If pivot gets a non-zero, migrationHasRunFlag gets set to true + tagObjStub.pivot.get.returns(0); + // By returning an array from related, we can use real reduce to simulate a result here + postObjStub.related.returns([tagObjStub, tagObjStub, tagObjStub]); // By returning from mapThen, we can skip doing tag.load in this test - postCollStub = {mapThen: sandbox.stub().returns([postObjStub])}, - postAllStub = sandbox.stub(models.Post, 'findAll').returns(Promise.resolve(postCollStub)); + postCollStub.mapThen.returns([postObjStub]); fixtures004[6]({}, loggerStub).then(function () { loggerStub.info.calledThrice.should.be.true(); @@ -683,10 +650,14 @@ describe('Fixtures', function () { }); describe('08-add-post-fixture', function () { - it('tries to add a new post fixture correctly', function (done) { - var postOneStub = sandbox.stub(models.Post, 'findOne').returns(Promise.resolve()), - postAddStub = sandbox.stub(models.Post, 'add').returns(Promise.resolve()); + var postOneStub, postAddStub; + beforeEach(function () { + postOneStub = sandbox.stub(models.Post, 'findOne').returns(Promise.resolve()); + postAddStub = sandbox.stub(models.Post, 'add').returns(Promise.resolve()); + }); + + it('tries to add a new post fixture correctly', function (done) { fixtures004[7]({}, loggerStub).then(function () { postOneStub.calledOnce.should.be.true(); loggerStub.info.calledOnce.should.be.true(); @@ -699,8 +670,7 @@ describe('Fixtures', function () { }); it('does not try to add new post fixture if it already exists', function (done) { - var postOneStub = sandbox.stub(models.Post, 'findOne').returns(Promise.resolve({})), - postAddStub = sandbox.stub(models.Post, 'add').returns(Promise.resolve()); + postOneStub.returns(Promise.resolve({})); fixtures004[7]({}, loggerStub).then(function () { postOneStub.calledOnce.should.be.true(); @@ -813,10 +783,17 @@ describe('Fixtures', function () { }); describe('Create Owner', function () { + var createOwner = populate.__get__('createOwner'), + roleOneStub, userAddStub; + + beforeEach(function () { + roleOneStub = sandbox.stub(models.Role, 'findOne'); + userAddStub = sandbox.stub(models.User, 'add'); + }); + it('createOwner will add user if owner role is present', function (done) { - var createOwner = populate.__get__('createOwner'), - roleOneStub = sandbox.stub(models.Role, 'findOne').returns(Promise.resolve({id: 1})), - userAddStub = sandbox.stub(models.User, 'add').returns(Promise.resolve({})); + roleOneStub.returns(Promise.resolve({id: 1})); + userAddStub.returns(Promise.resolve({})); createOwner(loggerStub).then(function () { loggerStub.info.called.should.be.true(); @@ -829,9 +806,8 @@ describe('Fixtures', function () { }); it('createOwner does not add user if owner role is not present', function (done) { - var createOwner = populate.__get__('createOwner'), - roleOneStub = sandbox.stub(models.Role, 'findOne').returns(Promise.resolve()), - userAddStub = sandbox.stub(models.User, 'add').returns(Promise.resolve({})); + roleOneStub.returns(Promise.resolve()); + userAddStub.returns(Promise.resolve({})); createOwner().then(function () { roleOneStub.calledOnce.should.be.true(); @@ -843,20 +819,22 @@ describe('Fixtures', function () { }); describe('Match Func', function () { - var matchFunc = populate.__get__('matchFunc'); + var matchFunc = populate.__get__('matchFunc'), + getStub; + + beforeEach(function () { + getStub = sandbox.stub(); + getStub.withArgs('foo').returns('bar'); + getStub.withArgs('fun').returns('baz'); + }); it('should match undefined with no args', function () { - var getStub = sandbox.stub(); - matchFunc()({get: getStub}).should.be.true(); getStub.calledOnce.should.be.true(); getStub.calledWith(undefined).should.be.true(); }); it('should match key with match string', function () { - var getStub = sandbox.stub(); - getStub.withArgs('foo').returns('bar'); - matchFunc('foo', 'bar')({get: getStub}).should.be.true(); getStub.calledOnce.should.be.true(); getStub.calledWith('foo').should.be.true(); @@ -867,9 +845,6 @@ describe('Fixtures', function () { }); it('should match value when key is 0', function () { - var getStub = sandbox.stub(); - getStub.withArgs('foo').returns('bar'); - matchFunc('foo', 0, 'bar')({get: getStub}).should.be.true(); getStub.calledOnce.should.be.true(); getStub.calledWith('foo').should.be.true(); @@ -880,10 +855,6 @@ describe('Fixtures', function () { }); it('should match key & value when match is array', function () { - var getStub = sandbox.stub(); - getStub.withArgs('foo').returns('bar'); - getStub.withArgs('fun').returns('baz'); - matchFunc(['foo', 'fun'], 'bar', 'baz')({get: getStub}).should.be.true(); getStub.calledTwice.should.be.true(); getStub.getCall(0).calledWith('fun').should.be.true(); @@ -896,10 +867,6 @@ describe('Fixtures', function () { }); it('should match key only when match is array, but value is all', function () { - var getStub = sandbox.stub(); - getStub.withArgs('foo').returns('bar'); - getStub.withArgs('fun').returns('baz'); - matchFunc(['foo', 'fun'], 'bar', 'all')({get: getStub}).should.be.true(); getStub.calledOnce.should.be.true(); getStub.calledWith('foo').should.be.true(); @@ -911,10 +878,6 @@ describe('Fixtures', function () { }); it('should match key & value when match and value are arrays', function () { - var getStub = sandbox.stub(); - getStub.withArgs('foo').returns('bar'); - getStub.withArgs('fun').returns('baz'); - matchFunc(['foo', 'fun'], 'bar', ['baz', 'buz'])({get: getStub}).should.be.true(); getStub.calledTwice.should.be.true(); getStub.getCall(0).calledWith('fun').should.be.true(); diff --git a/core/test/unit/migration_spec.js b/core/test/unit/migration_spec.js index 4e0355d295..ff74398245 100644 --- a/core/test/unit/migration_spec.js +++ b/core/test/unit/migration_spec.js @@ -76,11 +76,15 @@ describe('Migrations', function () { }); describe('Backup', function () { - it('should create a backup JSON file', function (done) { - var exportStub = sandbox.stub(exporter, 'doExport').returns(new Promise.resolve()), - filenameStub = sandbox.stub(exporter, 'fileName').returns(new Promise.resolve('test')), - fsStub = sandbox.stub(fs, 'writeFile').yields(); + var exportStub, filenameStub, fsStub; + beforeEach(function () { + exportStub = sandbox.stub(exporter, 'doExport').returns(new Promise.resolve()); + filenameStub = sandbox.stub(exporter, 'fileName').returns(new Promise.resolve('test')); + fsStub = sandbox.stub(fs, 'writeFile').yields(); + }); + + it('should create a backup JSON file', function (done) { migration.backupDatabase(loggerStub).then(function () { exportStub.calledOnce.should.be.true(); filenameStub.calledOnce.should.be.true(); @@ -92,10 +96,7 @@ describe('Migrations', function () { }); it('should fall back to console.log if no logger provided', function (done) { - var exportStub = sandbox.stub(exporter, 'doExport').returns(new Promise.resolve()), - filenameStub = sandbox.stub(exporter, 'fileName').returns(new Promise.resolve('test')), - noopStub = sandbox.stub(_, 'noop'), - fsStub = sandbox.stub(fs, 'writeFile').yields(); + var noopStub = sandbox.stub(_, 'noop'); migration.backupDatabase().then(function () { exportStub.calledOnce.should.be.true(); @@ -111,11 +112,13 @@ describe('Migrations', function () { }); describe('Reset', function () { - it('should delete all tables in reverse order', function (done) { - // Setup - var deleteStub = sandbox.stub(schema.commands, 'deleteTable').returns(new Promise.resolve()); + var deleteStub; - // Execute + beforeEach(function () { + deleteStub = sandbox.stub(schema.commands, 'deleteTable').returns(new Promise.resolve()); + }); + + it('should delete all tables in reverse order', function (done) { migration.reset().then(function (result) { should.exist(result); result.should.be.an.Array().with.lengthOf(schemaTables.length); @@ -132,10 +135,6 @@ describe('Migrations', function () { }); it('should delete all tables in reverse order when called twice in a row', function (done) { - // Setup - var deleteStub = sandbox.stub(schema.commands, 'deleteTable').returns(new Promise.resolve()); - - // Execute migration.reset().then(function (result) { should.exist(result); result.should.be.an.Array().with.lengthOf(schemaTables.length); @@ -165,12 +164,15 @@ describe('Migrations', function () { }); describe('Populate', function () { - it('should create all tables, and populate fixtures', function (done) { - // Setup - var createStub = sandbox.stub(schema.commands, 'createTable').returns(new Promise.resolve()), - fixturesStub = sandbox.stub(fixtures, 'populate').returns(new Promise.resolve()), - settingsStub = sandbox.stub(fixtures, 'ensureDefaultSettings').returns(new Promise.resolve()); + var createStub, fixturesStub, settingsStub; + beforeEach(function () { + createStub = sandbox.stub(schema.commands, 'createTable').returns(new Promise.resolve()); + fixturesStub = sandbox.stub(fixtures, 'populate').returns(new Promise.resolve()); + settingsStub = sandbox.stub(fixtures, 'ensureDefaultSettings').returns(new Promise.resolve()); + }); + + it('should create all tables, and populate fixtures', function (done) { populate(loggerStub).then(function (result) { should.not.exist(result); @@ -190,11 +192,6 @@ describe('Migrations', function () { }); it('should should only create tables, with tablesOnly setting', function (done) { - // Setup - var createStub = sandbox.stub(schema.commands, 'createTable').returns(new Promise.resolve()), - fixturesStub = sandbox.stub(fixtures, 'populate').returns(new Promise.resolve()), - settingsStub = sandbox.stub(fixtures, 'ensureDefaultSettings').returns(new Promise.resolve()); - populate(loggerStub, true).then(function (result) { should.exist(result); result.should.be.an.Array().with.lengthOf(schemaTables.length); @@ -217,7 +214,7 @@ describe('Migrations', function () { describe('Update', function () { describe('Update function', function () { - var reset, backupStub, settingsStub, fixturesStub, setDbStub, errorStub, versionsSpy; + var reset, backupStub, settingsStub, fixturesStub, setDbStub, errorStub, versionsSpy, tasksSpy; beforeEach(function () { // Stubs @@ -228,6 +225,7 @@ describe('Migrations', function () { errorStub = sandbox.stub(schema.versioning, 'showCannotMigrateError').returns(new Promise.resolve()); // Spys versionsSpy = sandbox.spy(schema.versioning, 'getMigrationVersions'); + tasksSpy = sandbox.spy(schema.versioning, 'getUpdateDatabaseTasks'); // Internal overrides reset = update.__set__('backup', backupStub); @@ -335,29 +333,17 @@ describe('Migrations', function () { }); describe('Update to 004', function () { - var knexStub, knexMock; - - beforeEach(function () { - knexMock = sandbox.stub().returns({}); - knexMock.schema = { - hasTable: sandbox.stub(), - hasColumn: sandbox.stub() - }; - // this MUST use sinon, not sandbox, see sinonjs/sinon#781 - knexStub = sinon.stub(db, 'knex', {get: function () { return knexMock; }}); - }); - - afterEach(function () { - knexStub.restore(); - }); - - it('should call all the 004 database upgrades', function (done) { + it('should call all the 004 database upgrade tasks', function (done) { // Setup - // stub has table, so that the next action won't happen - knexMock.schema.hasTable.withArgs('users').returns(new Promise.resolve(false)); - knexMock.schema.hasTable.withArgs('posts_tags').returns(new Promise.resolve(false)); - knexMock.schema.hasTable.withArgs('clients').returns(new Promise.resolve(false)); - knexMock.schema.hasTable.withArgs('client_trusted_domains').returns(new Promise.resolve(true)); + // Create a new stub, this will replace sequence, so that db calls don't actually get run + var sequenceStub = sandbox.stub(), + sequenceReset = update.__set__('sequence', sequenceStub); + + // The first time we call sequence, it should be to execute a top level version, e.g 004 + // yieldsTo('0') means this stub will execute the function at index 0 of the array passed as the + // first argument. In short the `runVersionTasks` function gets executed, and sequence gets called + // again with the array of tasks to execute for 004, which is what we want to check + sequenceStub.onFirstCall().yieldsTo('0'); // Execute update('003', '004', loggerStub).then(function () { @@ -368,19 +354,77 @@ describe('Migrations', function () { versionsSpy.calledWith('003', '004').should.be.true(); versionsSpy.returned(['003', '004']).should.be.true(); - knexStub.get.callCount.should.eql(5); - knexMock.schema.hasTable.callCount.should.eql(5); - knexMock.schema.hasColumn.called.should.be.false(); + tasksSpy.calledOnce.should.be.true(); + tasksSpy.calledWith('004', loggerStub).should.be.true(); + tasksSpy.firstCall.returnValue.should.be.an.Array().with.lengthOf(5); + sequenceStub.calledTwice.should.be.true(); + sequenceStub.firstCall.calledWith(sinon.match.array, loggerStub).should.be.true(); + sequenceStub.secondCall.calledWith(sinon.match.array, loggerStub).should.be.true(); + + sequenceStub.firstCall.args[0].should.be.an.Array().with.lengthOf(1); + sequenceStub.secondCall.args[0].should.be.an.Array().with.lengthOf(5); + + sequenceStub.firstCall.args[0][0].should.be.a.Function().with.property('name', 'runVersionTasks'); + + sequenceStub.secondCall.args[0][0].should.be.a.Function().with.property('name', 'addTourColumnToUsers'); + sequenceStub.secondCall.args[0][1].should.be.a.Function().with.property('name', 'addSortOrderColumnToPostsTags'); + sequenceStub.secondCall.args[0][2].should.be.a.Function().with.property('name', 'addManyColumnsToClients'); + sequenceStub.secondCall.args[0][3].should.be.a.Function().with.property('name', 'addClientTrustedDomainsTable'); + sequenceStub.secondCall.args[0][4].should.be.a.Function().with.property('name', 'dropUniqueOnClientsSecret'); + + // Reset sequence + sequenceReset(); done(); }).catch(done); }); describe('Tasks:', function () { + var addColumnStub, createTableStub, getIndexesStub, dropUniqueStub, + knexStub, knexMock; + + beforeEach(function () { + knexMock = sandbox.stub().returns({}); + knexMock.schema = { + hasTable: sandbox.stub(), + hasColumn: sandbox.stub() + }; + // this MUST use sinon, not sandbox, see sinonjs/sinon#781 + knexStub = sinon.stub(db, 'knex', {get: function () { return knexMock; }}); + + addColumnStub = sandbox.stub(schema.commands, 'addColumn'); + createTableStub = sandbox.stub(schema.commands, 'createTable'); + getIndexesStub = sandbox.stub(schema.commands, 'getIndexes'); + dropUniqueStub = sandbox.stub(schema.commands, 'dropUnique'); + }); + + afterEach(function () { + knexStub.restore(); + }); + describe('01-add-tour-column-to-users', function () { + it('does not try to add a new column if the table does not exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('users').returns(new Promise.resolve(false)); + + // Execute + updates004[0](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('users').should.be.true(); + + knexMock.schema.hasColumn.called.should.be.false(); + + addColumnStub.called.should.be.false(); + + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + it('does not try to add a new column if the column already exists', function (done) { // Setup - var addColumnStub = sandbox.stub(schema.commands, 'addColumn'); knexMock.schema.hasTable.withArgs('users').returns(new Promise.resolve(true)); knexMock.schema.hasColumn.withArgs('users', 'tour').returns(new Promise.resolve(true)); @@ -403,7 +447,6 @@ describe('Migrations', function () { it('tries to add a new column if table is present but column is not', function (done) { // Setup - var addColumnStub = sandbox.stub(schema.commands, 'addColumn'); knexMock.schema.hasTable.withArgs('users').returns(new Promise.resolve(true)); knexMock.schema.hasColumn.withArgs('users', 'tour').returns(new Promise.resolve(false)); @@ -427,9 +470,28 @@ describe('Migrations', function () { }); describe('02-add-sortorder-column-to-poststags', function () { + it('does not try to add a new column if the table does not exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('posts_tags').returns(new Promise.resolve(false)); + + // Execute + updates004[1](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('posts_tags').should.be.true(); + + knexMock.schema.hasColumn.called.should.be.false(); + + addColumnStub.called.should.be.false(); + + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + it('does not try to add a new column if the column already exists', function (done) { // Setup - var addColumnStub = sandbox.stub(schema.commands, 'addColumn'); knexMock.schema.hasTable.withArgs('posts_tags').returns(new Promise.resolve(true)); knexMock.schema.hasColumn.withArgs('posts_tags', 'sort_order').returns(new Promise.resolve(true)); @@ -452,7 +514,6 @@ describe('Migrations', function () { it('tries to add a new column if table is present but column is not', function (done) { // Setup - var addColumnStub = sandbox.stub(schema.commands, 'addColumn'); knexMock.schema.hasTable.withArgs('posts_tags').returns(new Promise.resolve(true)); knexMock.schema.hasColumn.withArgs('posts_tags', 'sort_order').returns(new Promise.resolve(false)); @@ -476,9 +537,28 @@ describe('Migrations', function () { }); describe('03-add-many-columns-to-clients', function () { + it('does not try to add new columns if the table does not exist', function (done) { + // Setup + knexMock.schema.hasTable.withArgs('clients').returns(new Promise.resolve(false)); + + // Execute + updates004[2](loggerStub).then(function () { + knexMock.schema.hasTable.calledOnce.should.be.true(); + knexMock.schema.hasTable.calledWith('clients').should.be.true(); + + knexMock.schema.hasColumn.called.should.be.false(); + + addColumnStub.called.should.be.false(); + + loggerStub.info.called.should.be.false(); + loggerStub.warn.calledOnce.should.be.true(); + + done(); + }).catch(done); + }); + it('does not try to add new columns if the columns already exist', function (done) { // Setup - var addColumnStub = sandbox.stub(schema.commands, 'addColumn'); knexMock.schema.hasTable.withArgs('clients').returns(new Promise.resolve(true)); knexMock.schema.hasColumn.withArgs('clients', 'redirection_uri').returns(new Promise.resolve(true)); knexMock.schema.hasColumn.withArgs('clients', 'logo').returns(new Promise.resolve(true)); @@ -509,7 +589,6 @@ describe('Migrations', function () { it('tries to add new columns if table is present but columns are not', function (done) { // Setup - var addColumnStub = sandbox.stub(schema.commands, 'addColumn'); knexMock.schema.hasTable.withArgs('clients').returns(new Promise.resolve(true)); knexMock.schema.hasColumn.withArgs('clients', 'redirection_uri').returns(new Promise.resolve(false)); knexMock.schema.hasColumn.withArgs('clients', 'logo').returns(new Promise.resolve(false)); @@ -545,7 +624,6 @@ describe('Migrations', function () { it('will only try to add columns that do not exist', function (done) { // Setup - var addColumnStub = sandbox.stub(schema.commands, 'addColumn'); knexMock.schema.hasTable.withArgs('clients').returns(new Promise.resolve(true)); knexMock.schema.hasColumn.withArgs('clients', 'redirection_uri').returns(new Promise.resolve(true)); knexMock.schema.hasColumn.withArgs('clients', 'logo').returns(new Promise.resolve(false)); @@ -580,8 +658,6 @@ describe('Migrations', function () { describe('04-add-clienttrusteddomains-table', function () { it('does not try to add a new table if the table already exists', function (done) { // Setup - var createTableStub = sandbox.stub(schema.commands, 'createTable'); - knexMock.schema.hasTable.withArgs('client_trusted_domains').returns(new Promise.resolve(true)); // Execute @@ -600,8 +676,6 @@ describe('Migrations', function () { it('tries to add a new table if the table does not exist', function (done) { // Setup - var createTableStub = sandbox.stub(schema.commands, 'createTable'); - knexMock.schema.hasTable.withArgs('client_trusted_domains').returns(new Promise.resolve(false)); // Execute @@ -623,9 +697,6 @@ describe('Migrations', function () { describe('05-drop-unique-on-clients-secret', function () { it('does not try to drop unique if the table does not exist', function (done) { // Setup - var getIndexesStub = sandbox.stub(schema.commands, 'getIndexes'), - dropUniqueStub = sandbox.stub(schema.commands, 'dropUnique'); - getIndexesStub.withArgs('clients').returns(new Promise.resolve( ['clients_slug_unique', 'clients_name_unique', 'clients_secret_unique']) ); @@ -649,9 +720,6 @@ describe('Migrations', function () { it('does not try to drop unique if the index does not exist', function (done) { // Setup - var getIndexesStub = sandbox.stub(schema.commands, 'getIndexes'), - dropUniqueStub = sandbox.stub(schema.commands, 'dropUnique'); - knexMock.schema.hasTable.withArgs('clients').returns(new Promise.resolve(true)); getIndexesStub.withArgs('clients').returns(new Promise.resolve( @@ -677,9 +745,6 @@ describe('Migrations', function () { it('tries to add a drop unique if table and index both exist', function (done) { // Setup - var getIndexesStub = sandbox.stub(schema.commands, 'getIndexes'), - dropUniqueStub = sandbox.stub(schema.commands, 'dropUnique'); - knexMock.schema.hasTable.withArgs('clients').returns(new Promise.resolve(true)); getIndexesStub.withArgs('clients').returns(new Promise.resolve( @@ -709,9 +774,15 @@ describe('Migrations', function () { }); describe('Update Database Schema', function () { + var updateDatabaseSchema = update.__get__('updateDatabaseSchema'), + getVersionTasksStub; + + beforeEach(function () { + getVersionTasksStub = sandbox.stub(schema.versioning, 'getUpdateDatabaseTasks'); + }); + it('should not do anything if there are no tasks', function (done) { - var updateDatabaseSchema = update.__get__('updateDatabaseSchema'), - getVersionTasksStub = sandbox.stub(schema.versioning, 'getUpdateDatabaseTasks').returns([]); + getVersionTasksStub.returns([]); updateDatabaseSchema(['001'], loggerStub).then(function () { getVersionTasksStub.calledOnce.should.be.true(); @@ -722,10 +793,10 @@ describe('Migrations', function () { }); it('should call the tasks if they are provided', function (done) { - var updateDatabaseSchema = update.__get__('updateDatabaseSchema'), - task1Stub = sandbox.stub().returns(new Promise.resolve()), - task2Stub = sandbox.stub().returns(new Promise.resolve()), - getVersionTasksStub = sandbox.stub(schema.versioning, 'getUpdateDatabaseTasks').returns([task1Stub, task2Stub]); + var task1Stub = sandbox.stub().returns(new Promise.resolve()), + task2Stub = sandbox.stub().returns(new Promise.resolve()); + + getVersionTasksStub.returns([task1Stub, task2Stub]); updateDatabaseSchema(['001'], loggerStub).then(function () { getVersionTasksStub.calledOnce.should.be.true(); @@ -733,6 +804,8 @@ describe('Migrations', function () { task2Stub.calledOnce.should.be.true(); loggerStub.info.calledTwice.should.be.true(); + sinon.assert.callOrder(getVersionTasksStub, loggerStub.info, loggerStub.info, task1Stub, task2Stub); + done(); }).catch(done); }); @@ -947,11 +1020,15 @@ describe('Migrations', function () { }); describe('Logger', function () { - it('should output an info message prefixed with "Migrations"', function () { - var logger = migration.__get__('logger'), - errorsInfoStub = sandbox.stub(errors, 'logComponentInfo'), - errorsWarnStub = sandbox.stub(errors, 'logComponentWarn'); + var logger, errorsInfoStub, errorsWarnStub; + beforeEach(function () { + logger = migration.__get__('logger'); + errorsInfoStub = sandbox.stub(errors, 'logComponentInfo'); + errorsWarnStub = sandbox.stub(errors, 'logComponentWarn'); + }); + + it('should output an info message prefixed with "Migrations"', function () { logger.info('Stuff'); errorsInfoStub.calledOnce.should.be.true(); @@ -960,10 +1037,6 @@ describe('Migrations', function () { }); it('should output a warn message prefixed with "Skipped Migrations"', function () { - var logger = migration.__get__('logger'), - errorsInfoStub = sandbox.stub(errors, 'logComponentInfo'), - errorsWarnStub = sandbox.stub(errors, 'logComponentWarn'); - logger.warn('Stuff'); errorsWarnStub.calledOnce.should.be.true();