Skip to content

Commit

Permalink
fix(doctor): add install checks for various nvm edge cases
Browse files Browse the repository at this point in the history 
closes #281
- throw error if npm bin directory is not the same as the one used to install ghost-cli
  • Loading branch information
@acburdine
acburdine committed Aug 5, 2017
1 parent d0225f8 commit 2a8de70
Show file tree
Hide file tree
Showing 2 changed files with 167 additions and 27 deletions.
26 changes: 20 additions & 6 deletions lib/commands/doctor/checks/install.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const eol = os.EOL;

const tasks = {
// While it's not an actual task, we put it here to make it easier to test
checkDirectoryAndAbove: function checkDirectoryAndAbove(dir) {
checkDirectoryAndAbove: function checkDirectoryAndAbove(dir, extra) {
if (isRoot(dir)) {
return Promise.resolve();
}
Expand All @@ -29,14 +29,24 @@ const tasks = {
return Promise.reject(new errors.SystemError(
`The path ${dir} is not readable by other users on the system.${eol}` +
'This can cause issues with the CLI, please either make this directory ' +
'readable by others or install in another location.'
`readable by others or ${extra} in another location.`
));
}

return checkDirectoryAndAbove(path.join(dir, '../'));
return tasks.checkDirectoryAndAbove(path.join(dir, '../'), extra);
});
},
nodeVersion: function nodeVersion() {
nodeVersion: function nodeVersion(ctx) {
let globalBin = execa.shellSync('npm bin -g').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 All @@ -48,7 +58,11 @@ const tasks = {
));
}

return Promise.resolve();
if (ctx.local || os.platform() !== 'linux' || (ctx.argv && ctx.argv['setup-linux-user'] === false)) {
return Promise.resolve();
}

return tasks.checkDirectoryAndAbove(process.argv[0], 'install node and Ghost-CLI');
},
folderPermissions: function folderPermissions(ctx) {
return fs.access(process.cwd(), constants.R_OK | constants.W_OK).catch(() => {
Expand All @@ -61,7 +75,7 @@ const tasks = {
return Promise.resolve();
}

return tasks.checkDirectoryAndAbove(process.cwd());
return tasks.checkDirectoryAndAbove(process.cwd(), 'run `ghost install`');
});
},
systemStack: function systemStack(ctx) {
Expand Down
168 changes: 147 additions & 21 deletions test/unit/commands/doctor/install-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const expect = require('chai').expect;
const sinon = require('sinon');
const stripAnsi = require('strip-ansi');
const proxyquire = require('proxyquire').noCallThru();
const path = require('path');

const modulePath = '../../../../lib/commands/doctor/checks/install';
const errors = require('../../../../lib/errors');
Expand Down Expand Up @@ -34,57 +35,182 @@ describe('Unit: Doctor Checks > Install', function () {
});

describe('node version check', function () {
it('doesn\'t do anything if GHOST_NODE_VERSION_CHECK is false', function () {
let originalEnv = process.env;
it('rejects if global bin is different than the one ghost is running from', function () {
let execaStub = sinon.stub().returns({stdout: '/usr/local/bin'});
let 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')).to.be.true;
});
});

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

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

return task().then(() => {
process.env = originalEnv;
expect(false, 'error should be thrown').to.be.true;
}).catch((error) => {
expect(error).to.be.an.instanceof(errors.SystemError);
let message = stripAnsi(error.message);

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

it('doesn\'t do anything if node version is in range', function () {
it('doesn\'t reject if bin is the local ghost bin file from the install (and local is true)', function () {
let cliPackage = {
engines: {
node: process.versions.node // this future-proofs the test
}
};
let execaStub = sinon.stub().returns({stdout: '/usr/local/bin'});
let originalArgv = process.argv;
process.argv = ['node', path.join(__dirname, '../../../../bin/ghost')];

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

return task();
return tasks.nodeVersion({local: true}).then(() => {
process.argv = originalArgv;
expect(execaStub.calledOnce).to.be.true;
expect(checkDirectoryStub.called).to.be.false;
});
});

it('throws error if node version is not in range', function () {
it('doesn\'t do anything if GHOST_NODE_VERSION_CHECK is false and local is true', function () {
let originalEnv = process.env;
let cliPackage = {
engines: {
node: '0.10.0'
}
};
process.env = { GHOST_NODE_VERSION_CHECK: 'false' };
let execaStub = sinon.stub().returns({stdout: process.argv[1]});

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

return task().then(() => {
expect(false, 'error should be thrown').to.be.true;
}).catch((error) => {
expect(error).to.be.an.instanceof(errors.SystemError);
let message = stripAnsi(error.message);
return tasks.nodeVersion({local: true}).then(() => {
process.env = originalEnv;
expect(execaStub.calledOnce).to.be.true;
expect(checkDirectoryStub.called).to.be.false;
});
});

expect(message).to.match(/Supported: 0.10.0/);
expect(message).to.match(new RegExp(`Installed: ${process.versions.node}`));
it('doesn\'t do anything if node version is in range and local is true', function () {
let cliPackage = {
engines: {
node: process.versions.node // this future-proofs the test
}
};
let execaStub = sinon.stub().returns({stdout: process.argv[1]});

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

return tasks.nodeVersion({local: true}).then(() => {
expect(execaStub.calledOnce).to.be.true;
expect(checkDirectoryStub.called).to.be.false;
});
});

it('doesn\'t call checkDirectoryAndAbove if os is not linux', function () {
let cliPackage = {
engines: {
node: process.versions.node // this future-proofs the test
}
};
let execaStub = sinon.stub().returns({stdout: process.argv[1]});
let platformStub = sinon.stub().returns('darwin');

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

return tasks.nodeVersion({local: false}).then(() => {
expect(execaStub.calledOnce).to.be.true;
expect(checkDirectoryStub.called).to.be.false;
});
});

it('doesn\'t call checkDirectoryAndAbove if no-setup-linux-user is passed', function () {
let cliPackage = {
engines: {
node: process.versions.node // this future-proofs the test
}
};
let execaStub = sinon.stub().returns({stdout: process.argv[1]});
let platformStub = sinon.stub().returns('linux');

const tasks = proxyquire(modulePath, {
'../../../../package': cliPackage,
execa: {shellSync: execaStub},
os: {platform: platformStub}
}).tasks;
let 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;
});
});

it('calls checkDirectoryAndAbove if none of the three conditions are true', function () {
let cliPackage = {
engines: {
node: process.versions.node // this future-proofs the test
}
};
let execaStub = sinon.stub().returns({stdout: process.argv[1]});
let platformStub = sinon.stub().returns('linux');

const tasks = proxyquire(modulePath, {
'../../../../package': cliPackage,
execa: {shellSync: execaStub},
os: {platform: platformStub}
}).tasks;
let 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

0 comments on commit 2a8de70

Please sign in to comment.