Skip to content

Commit

Permalink
feat(db): remove knex-migrator dep, use one installed with Ghost (#505)
Browse files Browse the repository at this point in the history
refs #503

- use knex-migrator version installed with Ghost itself
- remove global knex-migrator dependency
- remove "same node version" check
  • Loading branch information
acburdine authored and kirrg001 committed Nov 10, 2017
1 parent c897ae9 commit 9b78843
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 489 deletions.
10 changes: 0 additions & 10 deletions lib/commands/doctor/checks/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,6 @@ const tasks = {
});
},
nodeVersion: function nodeVersion(ctx) {
const globalBin = execa.shellSync('npm bin -g', {preferLocal: false}).stdout;

if (process.argv[1] !== path.join(__dirname, '../../../../bin/ghost') && !process.argv[1].startsWith(globalBin)) {
return Promise.reject(new errors.SystemError(
`The version of Ghost-CLI you are running was not installed with this version of Node.${eol}` +
`This means there are likely two versions of Node running on your system, please ensure${eol}` +
'that you are only running one global version of Node before continuing.'
));
}

if (process.env.GHOST_NODE_VERSION_CHECK !== 'false' &&
!semver.satisfies(process.versions.node, cliPackage.engines.node)) {
return Promise.reject(new errors.SystemError(
Expand Down
4 changes: 2 additions & 2 deletions lib/tasks/migrate.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ module.exports = function runMigrations(context) {
// If we're using sqlite and the ghost user owns the content folder, then
// we should run sudo, otherwise run normally
if (shouldUseGhostUser(contentDir)) {
const knexMigratorPath = path.resolve(__dirname, '../../node_modules/.bin/knex-migrator-migrate');
const knexMigratorPath = path.resolve(context.instance.dir, 'current/node_modules/.bin/knex-migrator-migrate');
knexMigratorPromise = context.ui.sudo(`${knexMigratorPath} ${args.join(' ')}`, {sudoArgs: '-E -u ghost'});
} else {
knexMigratorPromise = execa('knex-migrator-migrate', args, {
preferLocal: true,
localDir: __dirname
localDir: path.join(context.instance.dir, 'current')
});
}

Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
"ghost-ignition": "2.8.14",
"inquirer": "3.3.0",
"is-running": "2.1.0",
"knex-migrator": "2.1.7",
"listr": "0.12.0",
"lodash": "4.17.4",
"log-symbols": "2.1.0",
Expand Down
50 changes: 4 additions & 46 deletions test/unit/commands/doctor/install-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,38 +35,15 @@ describe('Unit: Doctor Checks > Install', function () {
});

describe('node version check', function () {
it('rejects if global bin is different than the one ghost is running from', function () {
const execaStub = sinon.stub().returns({stdout: '/usr/local/bin'});
const originalArgv = process.argv;
process.argv = ['node', '/home/ghost/.nvm/versions/6.11.1/bin'];

const task = proxyquire(modulePath, {
execa: {shellSync: execaStub}
}).tasks.nodeVersion;

return task().then(() => {
expect(false, 'error should be thrown').to.be.true;
process.argv = originalArgv;
}).catch((error) => {
process.argv = originalArgv;
expect(error).to.be.an.instanceof(errors.SystemError);
expect(error.message).to.match(/version of Ghost-CLI you are running was not installed with this version of Node./);
expect(execaStub.calledOnce).to.be.true;
expect(execaStub.calledWithExactly('npm bin -g', {preferLocal: false})).to.be.true;
});
});

it('rejects if node version is not in range', function () {
const cliPackage = {
engines: {
node: '0.10.0'
}
};
const execaStub = sinon.stub().returns({stdout: process.argv[1]});

const task = proxyquire(modulePath, {
'../../../../package': cliPackage,
execa: {shellSync: execaStub}
'../../../../package': cliPackage
}).tasks.nodeVersion;

return task().then(() => {
Expand All @@ -77,7 +54,6 @@ describe('Unit: Doctor Checks > Install', function () {

expect(message).to.match(/Supported: 0.10.0/);
expect(message).to.match(new RegExp(`Installed: ${process.versions.node}`));
expect(execaStub.calledOnce).to.be.true;
});
});

Expand All @@ -87,19 +63,16 @@ describe('Unit: Doctor Checks > Install', function () {
node: process.versions.node // this future-proofs the test
}
};
const execaStub = sinon.stub().returns({stdout: '/usr/local/bin'});
const originalArgv = process.argv;
process.argv = ['node', path.join(__dirname, '../../../../bin/ghost')];

const tasks = proxyquire(modulePath, {
'../../../../package': cliPackage,
execa: {shellSync: execaStub}
'../../../../package': cliPackage
}).tasks;
const checkDirectoryStub = sinon.stub(tasks, 'checkDirectoryAndAbove').resolves();

return tasks.nodeVersion({local: true}).then(() => {
process.argv = originalArgv;
expect(execaStub.calledOnce).to.be.true;
expect(checkDirectoryStub.called).to.be.false;
});
});
Expand All @@ -112,17 +85,14 @@ describe('Unit: Doctor Checks > Install', function () {
}
};
process.env = {GHOST_NODE_VERSION_CHECK: 'false'};
const execaStub = sinon.stub().returns({stdout: process.argv[1]});

const tasks = proxyquire(modulePath, {
'../../../../package': cliPackage,
execa: {shellSync: execaStub}
'../../../../package': cliPackage
}).tasks;
const checkDirectoryStub = sinon.stub(tasks, 'checkDirectoryAndAbove').resolves();

return tasks.nodeVersion({local: true}).then(() => {
process.env = originalEnv;
expect(execaStub.calledOnce).to.be.true;
expect(checkDirectoryStub.called).to.be.false;
});
});
Expand All @@ -133,16 +103,13 @@ describe('Unit: Doctor Checks > Install', function () {
node: process.versions.node // this future-proofs the test
}
};
const execaStub = sinon.stub().returns({stdout: process.argv[1]});

const tasks = proxyquire(modulePath, {
'../../../../package': cliPackage,
execa: {shellSync: execaStub}
'../../../../package': cliPackage
}).tasks;
const checkDirectoryStub = sinon.stub(tasks, 'checkDirectoryAndAbove').resolves();

return tasks.nodeVersion({local: true}).then(() => {
expect(execaStub.calledOnce).to.be.true;
expect(checkDirectoryStub.called).to.be.false;
});
});
Expand All @@ -153,18 +120,15 @@ describe('Unit: Doctor Checks > Install', function () {
node: process.versions.node // this future-proofs the test
}
};
const execaStub = sinon.stub().returns({stdout: process.argv[1]});
const platformStub = sinon.stub().returns('darwin');

const tasks = proxyquire(modulePath, {
'../../../../package': cliPackage,
execa: {shellSync: execaStub},
os: {platform: platformStub}
}).tasks;
const checkDirectoryStub = sinon.stub(tasks, 'checkDirectoryAndAbove').resolves();

return tasks.nodeVersion({local: false}).then(() => {
expect(execaStub.calledOnce).to.be.true;
expect(checkDirectoryStub.called).to.be.false;
});
});
Expand All @@ -175,18 +139,15 @@ describe('Unit: Doctor Checks > Install', function () {
node: process.versions.node // this future-proofs the test
}
};
const execaStub = sinon.stub().returns({stdout: process.argv[1]});
const platformStub = sinon.stub().returns('linux');

const tasks = proxyquire(modulePath, {
'../../../../package': cliPackage,
execa: {shellSync: execaStub},
os: {platform: platformStub}
}).tasks;
const checkDirectoryStub = sinon.stub(tasks, 'checkDirectoryAndAbove').resolves();

return tasks.nodeVersion({local: false, argv: {'setup-linux-user': false}}).then(() => {
expect(execaStub.calledOnce).to.be.true;
expect(checkDirectoryStub.called).to.be.false;
});
});
Expand All @@ -197,18 +158,15 @@ describe('Unit: Doctor Checks > Install', function () {
node: process.versions.node // this future-proofs the test
}
};
const execaStub = sinon.stub().returns({stdout: process.argv[1]});
const platformStub = sinon.stub().returns('linux');

const tasks = proxyquire(modulePath, {
'../../../../package': cliPackage,
execa: {shellSync: execaStub},
os: {platform: platformStub}
}).tasks;
const checkDirectoryStub = sinon.stub(tasks, 'checkDirectoryAndAbove').resolves();

return tasks.nodeVersion({local: false, argv: {'setup-linux-user': true}}).then(() => {
expect(execaStub.calledOnce).to.be.true;
expect(checkDirectoryStub.calledOnce).to.be.true;
expect(checkDirectoryStub.calledWith(process.argv[0])).to.be.true;
});
Expand Down
Loading

0 comments on commit 9b78843

Please sign in to comment.