fix(stryker): kill entire test process tree (#927)

We cannot trust that a test runner process didn't spawn additional processes. For example, angular cli will spawn an additional process to host the type checker (typescript) in. We should kill the entire process tree.

This fixes an edge case where Stryker doesn't shut down and keeps "hanging" after it is reported "done"

packages/stryker/package.json

@@ -70,6 +70,7 @@
     "rxjs": "^6.0.0",
     "source-map": "^0.6.1",
     "surrial": "^0.1.3",
+    "tree-kill": "^1.2.0",
     "tslib": "^1.5.0",
     "typed-rest-client": "^1.0.7"
   },

packages/stryker/src/isolated-runner/IsolatedTestRunnerAdapter.ts

@@ -3,10 +3,11 @@ import { getLogger } from 'log4js';
 import * as _ from 'lodash';
 import { fork, ChildProcess } from 'child_process';
 import { TestRunner, RunResult, RunOptions } from 'stryker-api/test_runner';
-import { serialize } from '../utils/objectUtils';
+import { serialize, kill } from '../utils/objectUtils';
 import { AdapterMessage, WorkerMessage } from './MessageProtocol';
 import IsolatedRunnerOptions from './IsolatedRunnerOptions';
 import Task from '../utils/Task';
+
 const MAX_WAIT_FOR_DISPOSE = 2000;
 
 class InitTask extends Task<void> {
@@ -20,6 +21,7 @@ class RunTask extends Task<RunResult> {
 }
 type WorkerTask = InitTask | DisposeTask | RunTask;
 
+
 /**
  * Runs the given test runner in a child process and forwards reports about test results
  * Also implements timeout-mechanism (on timeout, restart the child runner and report timeout) 
@@ -137,7 +139,7 @@ export default class TestRunnerChildProcessAdapter extends EventEmitter implemen
     this.currentTask = new DisposeTask(MAX_WAIT_FOR_DISPOSE);
     this.sendDisposeCommand();
     return this.currentTask.promise
-      .then(() => this.workerProcess.kill());
+      .then(() => kill(this.workerProcess.pid));
   }
 
   private sendRunCommand(options: RunOptions) {

packages/stryker/src/utils/objectUtils.ts

@@ -1,4 +1,5 @@
 import * as _ from 'lodash';
+import treeKill = require('tree-kill');
 export { serialize, deserialize } from 'surrial';
 
 export function freezeRecursively<T extends { [prop: string]: any }>(target: T): T {
@@ -81,4 +82,16 @@ export function base64Decode(base64EncodedString: string) {
  */
 export function normalizeWhiteSpaces(str: string) {
   return str.replace(/\s+/g, ' ').trim();
+}
+
+export function kill(pid: number): Promise<void> {
+  return new Promise((res, rej) => {
+    treeKill(pid, 'SIGKILL', err => {
+      if (err) {
+        rej(err);
+      } else {
+        res();
+      }
+    });
+  });
 }

packages/stryker/test/unit/isolated-runner/IsolatedTestRunnerAdapterSpec.ts

@@ -8,16 +8,19 @@ import { RunResult, RunStatus } from 'stryker-api/test_runner';
 import IsolatedTestRunnerAdapter from '../../../src/isolated-runner/IsolatedTestRunnerAdapter';
 import IsolatedRunnerOptions from '../../../src/isolated-runner/IsolatedRunnerOptions';
 import { WorkerMessage, RunMessage, ResultMessage } from '../../../src/isolated-runner/MessageProtocol';
-import { serialize } from '../../../src/utils/objectUtils';
+import * as objectUtils from '../../../src/utils/objectUtils';
+
 
 describe('IsolatedTestRunnerAdapter', () => {
   let sut: IsolatedTestRunnerAdapter;
   let sinonSandbox: sinon.SinonSandbox;
   let clock: sinon.SinonFakeTimers;
+  let killStub: sinon.SinonStub;
   let fakeChildProcess: {
     kill: sinon.SinonStub;
     send: sinon.SinonStub;
     on: sinon.SinonStub;
+    pid: number;
   };
   let runnerOptions: IsolatedRunnerOptions;
 
@@ -32,8 +35,10 @@ describe('IsolatedTestRunnerAdapter', () => {
     fakeChildProcess = {
       kill: sinon.stub(),
       send: sinon.stub(),
-      on: sinon.stub()
+      on: sinon.stub(),
+      pid: 42
     };
+    killStub = sinonSandbox.stub(objectUtils, 'kill');
     sinonSandbox.stub(child_process, 'fork').returns(fakeChildProcess);
     clock = sinon.useFakeTimers();
   });
@@ -65,7 +70,7 @@ describe('IsolatedTestRunnerAdapter', () => {
       it('should call "init" on child process', () => {
         arrangeAct();
         const expectedMessage: EmptyAdapterMessage = { kind: 'init' };
-        expect(fakeChildProcess.send).to.have.been.calledWith(serialize(expectedMessage));
+        expect(fakeChildProcess.send).to.have.been.calledWith(objectUtils.serialize(expectedMessage));
       });
 
 
@@ -96,7 +101,7 @@ describe('IsolatedTestRunnerAdapter', () => {
           kind: 'run',
           runOptions
         };
-        expect(fakeChildProcess.send).to.have.been.calledWith(serialize(expectedMessage));
+        expect(fakeChildProcess.send).to.have.been.calledWith(objectUtils.serialize(expectedMessage));
       });
 
       it('should proxy run response', () => {
@@ -121,7 +126,7 @@ describe('IsolatedTestRunnerAdapter', () => {
 
       it('should send `dispose` to worker process', () => {
         sut.dispose();
-        return expect(fakeChildProcess.send).to.have.been.calledWith(serialize({ kind: 'dispose' }));
+        return expect(fakeChildProcess.send).to.have.been.calledWith(objectUtils.serialize({ kind: 'dispose' }));
       });
 
       describe('and child process responds to dispose', () => {
@@ -132,8 +137,7 @@ describe('IsolatedTestRunnerAdapter', () => {
         });
 
         it('should kill the child process', () => {
-          expect(fakeChildProcess.kill).to.not.have.been.calledWith('SIGKILL');
-          expect(fakeChildProcess.kill).to.have.been.called;
+          expect(killStub).to.have.been.calledWith(42);
         });
       });
 
@@ -147,8 +151,7 @@ describe('IsolatedTestRunnerAdapter', () => {
         });
 
         it('should kill the child process', () => {
-          expect(fakeChildProcess.kill).to.not.have.been.calledWith('SIGKILL');
-          expect(fakeChildProcess.kill).to.have.been.called;
+          expect(killStub).to.have.been.calledWith(42);
         });
       });
     });