feat(process): improve restart flow

no issue
- add optional restart method to process managers that will, by default, just run the stop & start methods of the process manager in sequence. However, process managers can choose to override this with a custom implementation that is more suited to their individual setups
- add tests around restart flow

Author: acburdine

extensions/systemd/systemd.js

@@ -18,6 +18,11 @@ class SystemdProcessManager extends cli.ProcessManager {
             .catch((error) => Promise.reject(new cli.errors.ProcessError(error)));
     }
 
+    restart() {
+        return this.ui.sudo(`systemctl restart ${this.systemdName}`)
+            .catch((error) => Promise.reject(new cli.errors.ProcessError(error)));
+    }
+
     isRunning() {
         try {
             execa.shellSync(`systemctl is-active ${this.systemdName}`);

lib/commands/restart.js

@@ -1,10 +1,8 @@
 'use strict';
 const Command = require('../command');
-const StartCommand = require('./start');
-const StopCommand = require('./stop');
 
 class RestartCommand extends Command {
-    run(argv) {
+    run() {
         let instance = this.system.getInstance();
 
         if (!instance.running) {
@@ -13,7 +11,7 @@ class RestartCommand extends Command {
 
         instance.loadRunningEnvironment(true);
 
-        return this.runCommand(StopCommand, argv).then(() => this.runCommand(StartCommand, argv));
+        return this.ui.run(instance.process.restart(process.cwd(), this.system.environment), 'Restarting Ghost');
     }
 }
 

lib/process-manager.js

@@ -28,10 +28,16 @@ class ProcessManager {
      */
     start() {
         // Base implementation - noop
+        return Promise.resolve();
     }
 
     stop() {
         // Base implementation - noop
+        return Promise.resolve();
+    }
+
+    restart(cwd, env) {
+        return this.stop(cwd, env).then(() => this.start(cwd, env));
     }
 
     success() {
@@ -60,11 +66,11 @@ class ProcessManager {
 }
 
 function isValid(SubClass) {
-    if (!SubClass.prototype instanceof ProcessManager) {
+    if (!(SubClass.prototype instanceof ProcessManager)) {
         return false;
     }
 
-    let missing = requiredMethods.filter((method) => !SubClass.prototype[method]);
+    let missing = requiredMethods.filter((method) => !SubClass.prototype.hasOwnProperty(method));
 
     if (!missing.length) {
         return true;
@@ -74,6 +80,4 @@ function isValid(SubClass) {
 }
 
 module.exports = ProcessManager
-// Rather than make it a static method,
-// we export it here so subclasses can't override it
 module.exports.isValid = isValid;

test/unit/commands/restart.js

@@ -0,0 +1,51 @@
+'use strict';
+const expect = require('chai').expect;
+const sinon = require('sinon');
+
+const RestartCommand = require('../../../lib/commands/restart');
+const Instance = require('../../../lib/instance');
+
+describe('Unit: Command > Restart', function () {
+    it('throws error if instance is not running', function () {
+        class TestInstance extends Instance {
+            get running() { return false; }
+        }
+        let testInstance = new TestInstance();
+
+        let command = new RestartCommand({}, {
+            getInstance: () => testInstance
+        });
+
+        return command.run().then(() => {
+            throw new Error('Run method should have thrown');
+        }).catch((error) => {
+            expect(error.message).to.match(/instance is not currently running/);
+        });
+    });
+
+    it('calls process restart method if instance is running', function () {
+        let restartStub = sinon.stub().resolves();
+        class TestInstance extends Instance {
+            get running() { return true; }
+            get process() { return { restart: restartStub }; }
+        }
+        let testInstance = new TestInstance();
+        let loadRunEnvStub = sinon.stub(testInstance, 'loadRunningEnvironment');
+        let runStub = sinon.stub().resolves();
+
+        let command = new RestartCommand({
+            run: runStub
+        }, {
+            environment: 'testing',
+            getInstance: () => testInstance
+        });
+
+        return command.run().then(() => {
+            expect(loadRunEnvStub.calledOnce).to.be.true;
+            expect(loadRunEnvStub.args[0][0]).to.be.true;
+            expect(restartStub.calledOnce).to.be.true;
+            expect(restartStub.args[0]).to.deep.equal([process.cwd(), 'testing']);
+            expect(runStub.calledOnce).to.be.true;
+        });
+    });
+});

test/unit/process-manager.js

@@ -0,0 +1,43 @@
+'use strict';
+const expect = require('chai').expect;
+const sinon = require('sinon');
+
+const ProcessManager = require('../../lib/process-manager');
+
+describe('Unit: Process Manager', function () {
+    describe('isValid', function () {
+        it('returns false if passed class is not a subclass of ProcessManager', function () {
+            let result = ProcessManager.isValid({});
+            expect(result).to.be.false;
+        });
+
+        it('returns array of missing methods if class is missing required methods', function () {
+            class TestProcess extends ProcessManager {}
+            let result = ProcessManager.isValid(TestProcess);
+
+            expect(result).to.deep.equal(['start', 'stop', 'isRunning']);
+        });
+
+        it('returns true if class exists and implements all the right methods', function () {
+            class TestProcess extends ProcessManager {
+                start() { return Promise.resolve(); }
+                stop() { return Promise.resolve(); }
+                isRunning() { return true; }
+            }
+            let result = ProcessManager.isValid(TestProcess);
+            expect(result).to.be.true;
+        });
+    });
+
+    it('restart base implementation calls start and stop methods', function () {
+        let instance = new ProcessManager({}, {}, {});
+        let startStub = sinon.stub(instance, 'start').resolves();
+        let stopStub = sinon.stub(instance, 'stop').resolves();
+
+        return instance.restart().then(() => {
+            expect(stopStub.calledOnce).to.be.true;
+            expect(startStub.calledOnce).to.be.true;
+            expect(stopStub.calledBefore(startStub)).to.be.true;
+        })
+    });
+});