fix(dispose): clean up child processes in alternative flows (#1520)

Make sure Stryker doesn't _hang indefinitely_ when it supposed to _exit prematurely_.
Before, there was a code path that didn't dispose all child processes.
Now using typed-injects [dispose functionality](https://github.com/nicojs/typed-inject/#-disposing-provided-stuff).

e2e/test/exit-prematurely/.babelrc

@@ -0,0 +1,5 @@
+{
+    "presets": [
+        "@babel/env"
+    ]
+}

e2e/test/exit-prematurely/package.json

@@ -0,0 +1,15 @@
+{
+  "name": "exit-prematurely",
+  "version": "1.0.0",
+  "private": true,
+  "description": "A module to test the alternative flow when Stryker should exit prematurely, see https://github.com/stryker-mutator/stryker/issues/1519",
+  "main": "index.js",
+  "scripts": {
+    "pretest": "rimraf \"reports\" stryker.log .stryker-tmp ",
+    "test": "stryker run",
+    "posttest": "mocha --require ts-node/register verify/*.ts"
+  },
+  "keywords": [],
+  "author": "",
+  "license": "ISC"
+}

e2e/test/exit-prematurely/stryker.conf.js

@@ -0,0 +1,19 @@
+module.exports = function (config) {
+  config.set({
+    mutate: [
+      'src/*.js'
+    ],
+    testFramework: 'mocha',
+    testRunner: 'mocha',
+    coverageAnalysis: 'off',
+    mutator: 'javascript',
+    transpilers: [
+      'babel'
+    ],
+    timeoutMS: 60000,
+    reporters: ['clear-text', 'html', 'event-recorder'],
+    maxConcurrentTestRunners: 2,
+    logLevel: 'info',
+    fileLogLevel: 'info'
+  });
+};

e2e/test/exit-prematurely/test/test.js

@@ -0,0 +1 @@
+// Idle

e2e/test/exit-prematurely/tsconfig.json

@@ -0,0 +1,9 @@
+{
+  "extends": "../../tsconfig.json",
+  "compilerOptions": {
+    "declaration": false
+  },
+  "files": [
+    "verify/verify.ts"
+  ]
+}

e2e/test/exit-prematurely/verify/verify.ts

@@ -0,0 +1,19 @@
+import * as fs from 'fs';
+import { expect } from 'chai';
+
+describe('Verify stryker has ran correctly', () => {
+
+  const strykerLog = fs.readFileSync('./stryker.log', 'utf8');
+
+  it('exit prematurely', async () => {
+    expect(strykerLog).contains('No tests were executed. Stryker will exit prematurely.');
+  });
+
+  it('should log about a mutant free world', async () => {
+    expect(strykerLog).contains('It\'s a mutant-free world, nothing to test');
+  });
+
+  it('should warn about the globbing expression resulting in no files', () => {
+    expect(strykerLog).contains('Globbing expression "src/*.js" did not result in any files.');
+  });
+});

packages/core/src/SandboxPool.ts

@@ -11,10 +11,11 @@ import { InitialTestRunResult } from './process/InitialTestExecutor';
 import { Logger } from '@stryker-mutator/api/logging';
 import TranspiledMutant from './TranspiledMutant';
 import { MutantResult } from '@stryker-mutator/api/report';
+import { Disposable } from 'typed-inject';
 
 const MAX_CONCURRENT_INITIALIZING_SANDBOXES = 2;
 
-export class SandboxPool {
+export class SandboxPool implements Disposable {
 
   private readonly allSandboxes: Promise<Sandbox>[] = [];
   private readonly overheadTimeMS: number;
@@ -84,11 +85,11 @@ export class SandboxPool {
   }
 
   private readonly registerSandbox = async (promisedSandbox: Promise<Sandbox>): Promise<Sandbox> => {
-      this.allSandboxes.push(promisedSandbox);
-      return promisedSandbox;
+    this.allSandboxes.push(promisedSandbox);
+    return promisedSandbox;
   }
 
-  public async disposeAll() {
+  public async dispose() {
     const sandboxes = await Promise.all(this.allSandboxes);
     return Promise.all(sandboxes.map(sandbox => sandbox.dispose()));
   }

packages/core/src/Stryker.ts

@@ -79,16 +79,23 @@ export default class Stryker {
       const testableMutants = await mutationTestProcessInjector
         .injectClass(MutantTestMatcher)
         .matchWithMutants(mutator.mutate(inputFiles.filesToMutate));
-      if (initialRunResult.runResult.tests.length && testableMutants.length) {
-        const mutationTestExecutor = mutationTestProcessInjector.injectClass(MutationTestExecutor);
-        const mutantResults = await mutationTestExecutor.run(testableMutants);
-        await this.reportScore(mutantResults, inputFileInjector);
-        await TempFolder.instance().clean();
-        await this.logDone();
+      try {
+        if (initialRunResult.runResult.tests.length && testableMutants.length) {
+          const mutationTestExecutor = mutationTestProcessInjector
+            .injectClass(MutationTestExecutor);
+          const mutantResults = await mutationTestExecutor.run(testableMutants);
+          await this.reportScore(mutantResults, inputFileInjector);
+          await TempFolder.instance().clean();
+          await this.logDone();
+          return mutantResults;
+        } else {
+          this.logRemark();
+        }
+      } finally {
+        // `injector.dispose` calls `dispose` on all created instances
+        // Namely the `SandboxPool` and the `ChildProcessProxy` instances
+        mutationTestProcessInjector.dispose();
         await LogConfigurator.shutdown();
-        return mutantResults;
-      } else {
-        this.logRemark();
       }
     }
     return Promise.resolve([]);

packages/core/src/process/MutationTestExecutor.ts

@@ -28,9 +28,6 @@ export class MutationTestExecutor {
       tap(this.reportAll)
     ).toPromise();
 
-    // TODO: Let typed inject dispose of sandbox pool
-    await this.sandboxPool.disposeAll();
-    await this.mutantTranspileScheduler.dispose();
     return results;
   }
 

packages/core/src/transpiler/MutantTranspileScheduler.ts

@@ -15,7 +15,7 @@ import { BehaviorSubject } from 'rxjs';
 
 const INITIAL_CONCURRENCY = 100;
 
-export class MutantTranspileScheduler {
+export class MutantTranspileScheduler implements Disposable {
 
   private currentMutatedFile: SourceFile;
   private readonly concurrencyTicket$ = new BehaviorSubject<number>(INITIAL_CONCURRENCY);
@@ -44,12 +44,10 @@ export class MutantTranspileScheduler {
   }
 
   /**
-   * Dispose all
+   * Dispose
    */
   public dispose() {
     this.concurrencyTicket$.complete();
-    // TODO: Let typed-inject dispose this one
-    this.transpiler.dispose();
   }
 
   private createTranspiledMutant(mutant: TestableMutant, transpileResult: TranspileResult) {

packages/core/test/unit/SandboxPool.spec.ts

@@ -178,18 +178,18 @@ describe(SandboxPool.name, () => {
       await expect(actRunMutants()).rejectedWith(expectedError);
     });
   });
-  describe('disposeAll', () => {
+  describe('dispose', () => {
     it('should have disposed all sandboxes', async () => {
       sut = createSut();
       await actRunMutants();
-      await sut.disposeAll();
+      await sut.dispose();
       expect(firstSandbox.dispose).called;
       expect(secondSandbox.dispose).called;
     });
 
     it('should not do anything if no sandboxes were created', async () => {
       sut = createSut();
-      await sut.disposeAll();
+      await sut.dispose();
       expect(firstSandbox.dispose).not.called;
       expect(secondSandbox.dispose).not.called;
     });
@@ -211,7 +211,7 @@ describe(SandboxPool.name, () => {
       const runPromise = sut.runMutants(from(inputMutants)).toPromise();
       task.resolve(firstSandbox as unknown as Sandbox);
       await runPromise;
-      const disposePromise = sut.disposeAll();
+      const disposePromise = sut.dispose();
       task2.resolve(secondSandbox as unknown as Sandbox);
       await disposePromise;
       expect(secondSandbox.dispose).called;

packages/core/test/unit/Stryker.spec.ts

@@ -191,6 +191,11 @@ describe(Stryker.name, () => {
         await sut.runMutationTest();
         expect(logMock.info).to.have.been.calledWith('Trouble figuring out what went wrong? Try `npx stryker run --fileLogLevel trace --logLevel debug` to get some more info.');
       });
+
+      it('should dispose the injector', async () => {
+        await sut.runMutationTest();
+        expect(injectorMock.dispose).called;
+      });
     });
 
     describe('happy flow', () => {
@@ -263,10 +268,17 @@ describe(Stryker.name, () => {
         expect(reporterMock.wrapUp).to.have.been.called;
       });
 
+      it('should dispose the injector', async () => {
+        sut = new Stryker({});
+        await sut.runMutationTest();
+        expect(injectorMock.dispose).called;
+      });
+
       it('should shutdown the log4js server', async () => {
         sut = new Stryker({});
         await sut.runMutationTest();
         expect(shutdownLoggingStub).called;
+        expect(shutdownLoggingStub).calledAfter(injectorMock.dispose);
       });
 
       it('should create the transpiler with produceSourceMaps = true when coverage analysis is enabled', async () => {

packages/core/test/unit/process/MutationTestExecutor.spec.ts

@@ -31,7 +31,7 @@ describe(MutationTestExecutor.name, () => {
     sandboxPoolMock = mock(SandboxPool);
     mutantTranspileSchedulerMock = mock(MutantTranspileScheduler);
     mutantTranspileSchedulerMock.scheduleNext = sinon.stub();
-    sandboxPoolMock.disposeAll.resolves();
+    sandboxPoolMock.dispose.resolves();
     reporter = mock(BroadcastReporter);
     inputFiles = new InputFileCollection([new File('input.ts', '')], []);
     mutants = [testableMutant()];
@@ -56,16 +56,6 @@ describe(MutationTestExecutor.name, () => {
 
   describe('run', () => {
 
-    it('should dispose all sandboxes afterwards', async () => {
-      await sut.run(mutants);
-      expect(sandboxPoolMock.disposeAll).called;
-    });
-
-    it('should dispose the mutantTranspiler', async () => {
-      await sut.run(mutants);
-      expect(mutantTranspileSchedulerMock.dispose).called;
-    });
-
     it('should have ran the mutants in the sandbox pool', async () => {
       await sut.run(mutants);
       expect(mutantTranspileSchedulerMock.scheduleTranspileMutants).calledWith(mutants);