refactor: moved isLocal computation to process manager

- update/start/install all run `Doctor` internally to run the associated checks
- before this commit, only `install` set `argv.local`, and we had to manually check the process manager for `update`/`start`
- now all 3 commands set `argv.local`.
- we consider an install to be local if the ProcessManager is `local`

lib/commands/doctor/checks/install-folder-permissions.js

@@ -19,8 +19,7 @@ async function installFolderPermissions(ctx) {
         });
     }
 
-    const isLocal = ctx.local || (ctx.instance && ctx.instance.process.name === 'local');
-    if (isLocal || !ctx.system.platform.linux || (ctx.argv && ctx.argv['setup-linux-user'] === false)) {
+    if (ctx.local || !ctx.system.platform.linux || (ctx.argv && ctx.argv['setup-linux-user'] === false)) {
         return;
     }
 

lib/commands/doctor/checks/node-version.js

@@ -21,8 +21,7 @@ See ${chalk.underline.blue('https://ghost.org/docs/faq/node-versions/')} for mor
         });
     }
 
-    const isLocal = ctx.local || (ctx.instance && ctx.instance.process.name === 'local');
-    if (isLocal || !ctx.system.platform.linux || (ctx.argv && ctx.argv['setup-linux-user'] === false)) {
+    if (ctx.local || !ctx.system.platform.linux || (ctx.argv && ctx.argv['setup-linux-user'] === false)) {
         return;
     }
 

lib/commands/start.js

@@ -33,6 +33,7 @@ class StartCommand extends Command {
             recurse: !argv.dir
         });
 
+        argv.local = instance.isLocal;
         const isRunning = await instance.isRunning();
         if (isRunning) {
             this.ui.log('Ghost is already running! For more information, run', 'ghost ls', 'green', 'cmd', true);

lib/commands/stop.js

@@ -33,6 +33,7 @@ class StopCommand extends Command {
             command: 'stop',
             recurse: !argv.dir
         });
+        argv.local = instance.isLocal;
         const isRunning = await instance.isRunning();
 
         if (!isRunning) {

lib/commands/update.js

@@ -26,6 +26,7 @@ class UpdateCommand extends Command {
         const releaseNotes = require('../tasks/release-notes');
 
         const instance = this.system.getInstance();
+        argv.local = instance.isLocal;
 
         // If installed with a version < 1.0
         if (semver.lt(instance.cliVersion, '1.0.0')) {

lib/instance.js

@@ -155,6 +155,14 @@ class Instance {
         return this.system.hasInstance(this);
     }
 
+    /**
+     * Returns whether or not the instance is a local installation
+     * @readonly
+     */
+    get isLocal() {
+        return this.process.name === 'local';
+    }
+
     /**
      * Constructs the instance
      *

test/unit/commands/doctor/checks/install-folder-permissions-spec.js

@@ -40,19 +40,6 @@ describe('Unit: Doctor Checks > installFolderPermissions', function () {
         });
     });
 
-    it('skips checking parent folder permissions if local process manager is used', function () {
-        const accessStub = sinon.stub(fs, 'access').resolves();
-        const checkDirectoryStub = sinon.stub().resolves();
-        const installFolderPermissions = proxyquire(modulePath, {
-            './check-directory': checkDirectoryStub
-        }).task;
-
-        return installFolderPermissions({instance: {process: {name: 'local'}}}).then(() => {
-            expect(accessStub.calledOnce).to.be.true;
-            expect(checkDirectoryStub.called).to.be.false;
-        });
-    });
-
     it('skips checking parent folder permissions if os is not linux', function () {
         const accessStub = sinon.stub(fs, 'access').resolves();
         const checkDirectoryStub = sinon.stub().resolves();

test/unit/commands/doctor/checks/node-version-spec.js

@@ -160,25 +160,6 @@ describe('Unit: Doctor Checks > nodeVersion', function () {
         });
     });
 
-    it('doesn\'t call checkDirectoryAndAbove for local process managers', function () {
-        const cliPackage = {
-            engines: {
-                node: process.versions.node // this future-proofs the test
-            }
-        };
-        const ctx = {local: false, instance: {process: {name: 'local'}}};
-
-        const checkDirectoryStub = sinon.stub().resolves();
-        const nodeVersion = proxyquire(modulePath, {
-            '../../../../package': cliPackage,
-            './check-directory': checkDirectoryStub
-        }).task;
-
-        return nodeVersion(ctx, {}).then(() => {
-            expect(checkDirectoryStub.called).to.be.false;
-        });
-    });
-
     it('calls checkDirectoryAndAbove if none of the three conditions are true', function () {
         const cliPackage = {
             engines: {

test/unit/commands/start-spec.js

@@ -9,7 +9,7 @@ const UI = require('../../../lib/ui');
 
 const modulePath = '../../../lib/commands/start';
 
-function getStubs(dir, environment = undefined) {
+function getStubs(dir, environment = undefined, isLocal = false) {
     const ui = new UI({});
     const system = new System(ui, []);
     const instance = new Instance(ui, system, dir);
@@ -18,6 +18,11 @@ function getStubs(dir, environment = undefined) {
     instance._cliConfig = createConfigStub();
     instance._cliConfig.get.withArgs('name').returns('testing');
     instance._config.environment = environment;
+    Object.defineProperty(instance, 'isLocal', {
+        get() {
+            return isLocal;
+        }
+    });
     system.environment = environment;
 
     return {
@@ -130,6 +135,7 @@ describe('Unit: Commands > Start', function () {
             expect(runCommand.calledOnce).to.be.true;
             expect(runCommand.args[0][1]).to.deep.equal({
                 categories: ['start'],
+                local: false,
                 quiet: true,
                 checkMem: false,
                 skipInstanceCheck: true
@@ -158,6 +164,7 @@ describe('Unit: Commands > Start', function () {
             expect(runCommand.calledOnce).to.be.true;
             expect(runCommand.args[0][1]).to.deep.equal({
                 categories: ['start'],
+                local: false,
                 quiet: true,
                 checkMem: false,
                 skipInstanceCheck: true
@@ -167,6 +174,40 @@ describe('Unit: Commands > Start', function () {
             expect(log.called).to.be.false;
             expect(instance.config.get.called).to.be.false;
         });
+
+        it('sets argv.local based on the process manager (local)', async function () {
+            const {ui, system, instance} = getStubs('/var/www/ghost', undefined, true);
+            returnedInstance = instance;
+            const stopError = new Error('stopError');
+            sinon.stub(instance, 'isRunning').throws(stopError);
+            const cmd = new StartCommand(ui, system);
+            const argv = {};
+
+            try {
+                await cmd.run(argv);
+            } catch (error) {
+                expect(error).to.equal(stopError);
+                expect(argv.local).to.be.true;
+            }
+        });
+
+        it('sets argv.local based on the process manager (not local)', async function () {
+            const {ui, system, instance} = getStubs('/var/www/ghost', undefined, false);
+            returnedInstance = instance;
+            const stopError = new Error('stopError');
+            sinon.stub(instance, 'isRunning').throws(stopError);
+            const cmd = new StartCommand(ui, system);
+
+            // Example: `ghost update --local` in production
+            const argv = {local: true};
+
+            try {
+                await cmd.run(argv);
+            } catch (error) {
+                expect(error).to.equal(stopError);
+                expect(argv.local).to.be.false;
+            }
+        });
     });
 
     it('configureOptions loops over extensions', function () {

test/unit/commands/stop-spec.js

@@ -84,6 +84,42 @@ describe('Unit: Commands > Stop', function () {
             expect(stop.calledOnce).to.be.true;
             expect(log.called).to.be.false;
         });
+
+        it('sets argv.local based on the process manager (local)', async function () {
+            const error = new Error('stopError');
+            const isRunning = sinon.stub().throws(error);
+
+            const getInstance = sinon.stub().returns({isRunning, isLocal: true});
+            const Command = proxyquire(modulePath, {'../utils/get-instance': getInstance});
+
+            const cmd = new Command();
+            const argv = {};
+
+            try {
+                await cmd.run(argv);
+            } catch (error) {
+                expect(error).to.equal(error);
+                expect(argv.local).to.be.true;
+            }
+        });
+
+        it('sets argv.local based on the process manager (not local)', async function () {
+            const error = new Error('stopError');
+            const isRunning = sinon.stub().throws(error);
+
+            const getInstance = sinon.stub().returns({isRunning, isLocal: false});
+            const Command = proxyquire(modulePath, {'../utils/get-instance': getInstance});
+
+            const cmd = new Command();
+            const argv = {};
+
+            try {
+                await cmd.run(argv);
+            } catch (error) {
+                expect(error).to.equal(error);
+                expect(argv.local).to.be.false;
+            }
+        });
     });
 
     it('stopAll stops all instances', async function () {

test/unit/commands/update-spec.js

@@ -12,7 +12,7 @@ const modulePath = '../../../lib/commands/update';
 const errors = require('../../../lib/errors');
 const Instance = require('../../../lib/instance');
 
-function createTestInstance(version, cliVersion, previousVersion = null, config = {}) {
+function createTestInstance(version, cliVersion, previousVersion = null, config = {}, isLocal = false) {
     return class extends Instance {
         get version() {
             return version;
@@ -26,6 +26,9 @@ function createTestInstance(version, cliVersion, previousVersion = null, config
         get config() {
             return config;
         }
+        get isLocal() {
+            return isLocal;
+        }
     };
 }
 
@@ -737,6 +740,53 @@ describe('Unit: Commands > Update', function () {
 
             expect.fail('run should have thrown an error');
         });
+
+        it('sets argv.local based on the process manager (local)', async function () {
+            const UpdateCommand = require(modulePath);
+            const ui = {
+                log: sinon.stub(),
+                listr: sinon.stub().rejects(new Error('do_nothing')),
+                run: sinon.stub().callsFake(fn => fn())
+            };
+            const system = {getInstance: sinon.stub()};
+            const TestInstance = createTestInstance('1.1.0', '1.0.0', '1.0.0', {}, true);
+            const fakeInstance = sinon.stub(new TestInstance(ui, system, '/var/www/ghost'));
+            system.getInstance.returns(fakeInstance);
+            const expectedError = new Error('do_nothing');
+            fakeInstance.isRunning.throws(expectedError);
+
+            const argv = {};
+            try {
+                await new UpdateCommand(ui, system).run(argv);
+            } catch (error) {
+                expect(error).to.equal(error);
+                expect(argv.local).to.equal(true);
+            }
+        });
+
+        it('sets argv.local based on the process manager (not local)', async function () {
+            const UpdateCommand = require(modulePath);
+            const ui = {
+                log: sinon.stub(),
+                listr: sinon.stub().rejects(new Error('do_nothing')),
+                run: sinon.stub().callsFake(fn => fn())
+            };
+            const system = {getInstance: sinon.stub()};
+            const TestInstance = createTestInstance('1.1.0', '1.0.0', '1.0.0', {}, false);
+            const fakeInstance = sinon.stub(new TestInstance(ui, system, '/var/www/ghost'));
+            system.getInstance.returns(fakeInstance);
+            const expectedError = new Error('do_nothing');
+            fakeInstance.isRunning.throws(expectedError);
+
+            // Example: `ghost update --local` in production
+            const argv = {local: true};
+            try {
+                await new UpdateCommand(ui, system).run(argv);
+            } catch (error) {
+                expect(error).to.equal(error);
+                expect(argv.local).to.equal(false);
+            }
+        });
     });
 
     describe('downloadAndUpdate task', function () {

test/unit/instance-spec.js

@@ -133,6 +133,7 @@ describe('Unit: Instance', function () {
             const proc = testInstance.process;
             expect(proc).to.deep.equal(testProcess);
             expect(getStub.calledOnce).to.be.true;
+            expect(testInstance.isLocal).to.be.true;
         });
 
         it('loads new instance if cached one is available but the name is different', function () {
@@ -153,6 +154,7 @@ describe('Unit: Instance', function () {
             expect(testInstance._process).to.deep.equal(proc);
             expect(getStub.calledOnce).to.be.true;
             expect(procManagerStub.calledOnce).to.be.true;
+            expect(testInstance.isLocal).to.be.true;
         });
     });
 
@@ -280,7 +282,7 @@ describe('Unit: Instance', function () {
             const isRunningStub = sinon.stub().returns(true);
             class TestInstance extends Instance {
                 get process() {
-                    return {isRunning: isRunningStub};
+                    return {isRunning: isRunningStub, name: 'systemd'};
                 }
             }
 
@@ -299,14 +301,15 @@ describe('Unit: Instance', function () {
             expect(configStub.set.calledOnce).to.be.true;
             expect(configStub.set.calledWithExactly('running', 'production')).to.be.true;
             expect(configStub.save.calledOnce).to.be.true;
+            expect(testInstance.isLocal).to.be.false;
         });
 
         it('queries process manager in dev if prod config not exists and dev config does', async function () {
             const configStub = createConfigStub();
             const isRunningStub = sinon.stub().returns(true);
             class TestInstance extends Instance {
                 get process() {
-                    return {isRunning: isRunningStub};
+                    return {isRunning: isRunningStub, name: 'local'};
                 }
             }
 
@@ -329,6 +332,7 @@ describe('Unit: Instance', function () {
             expect(configStub.set.calledOnce).to.be.true;
             expect(configStub.set.calledWithExactly('running', 'development')).to.be.true;
             expect(configStub.save.calledOnce).to.be.true;
+            expect(testInstance.isLocal).to.be.true;
         });
 
         it('queries process manager in dev if not running in prod and dev config exists', async function () {
@@ -338,7 +342,7 @@ describe('Unit: Instance', function () {
             isRunningStub.onSecondCall().returns(true);
             class TestInstance extends Instance {
                 get process() {
-                    return {isRunning: isRunningStub};
+                    return {isRunning: isRunningStub, name: 'local'};
                 }
             }
 
@@ -358,14 +362,15 @@ describe('Unit: Instance', function () {
             expect(configStub.set.calledOnce).to.be.true;
             expect(configStub.set.calledWithExactly('running', 'development')).to.be.true;
             expect(configStub.save.calledOnce).to.be.true;
+            expect(testInstance.isLocal).to.be.true;
         });
 
         it('returns false if ghost isn\'t running in either environment', async function () {
             const configStub = createConfigStub();
             const isRunningStub = sinon.stub().returns(false);
             class TestInstance extends Instance {
                 get process() {
-                    return {isRunning: isRunningStub};
+                    return {isRunning: isRunningStub, name: 'local'};
                 }
             }
 
@@ -386,14 +391,15 @@ describe('Unit: Instance', function () {
             expect(configStub.save.called).to.be.false;
             expect(setEnvironmentStub.calledThrice).to.be.true;
             expect(setEnvironmentStub.args[2][0]).to.be.false;
+            expect(testInstance.isLocal).to.be.true;
         });
 
         it('loads running environment and checks if process manager returns false', async function () {
             const hasStub = sinon.stub().withArgs('running').returns(true);
             const isRunningStub = sinon.stub().returns(true);
             class TestInstance extends Instance {
                 get process() {
-                    return {isRunning: isRunningStub};
+                    return {isRunning: isRunningStub, name: 'local'};
                 }
             }
             const testInstance = new TestInstance({}, {}, '');
@@ -405,6 +411,7 @@ describe('Unit: Instance', function () {
             expect(hasStub.calledOnce).to.be.true;
             expect(isRunningStub.calledOnce).to.be.true;
             expect(loadRunEnvStub.calledOnce).to.be.true;
+            expect(testInstance.isLocal).to.be.true;
         });
 
         it('sets running to null in cliConfig if process manager\'s isRunning method returns false', async function () {