diff --git a/packages/core/src/test-runner/child-process-test-runner-decorator.ts b/packages/core/src/test-runner/child-process-test-runner-decorator.ts index 22943b6513..a60a38b563 100644 --- a/packages/core/src/test-runner/child-process-test-runner-decorator.ts +++ b/packages/core/src/test-runner/child-process-test-runner-decorator.ts @@ -1,4 +1,5 @@ import { StrykerOptions } from '@stryker-mutator/api/core'; +import { Logger } from '@stryker-mutator/api/logging'; import { TestRunner, DryRunOptions, MutantRunOptions, MutantRunResult, DryRunResult } from '@stryker-mutator/api/test-runner'; import { ExpirableTask } from '@stryker-mutator/util'; @@ -16,7 +17,7 @@ const MAX_WAIT_FOR_DISPOSE = 2000; export class ChildProcessTestRunnerDecorator implements TestRunner { private readonly worker: ChildProcessProxy; - constructor(options: StrykerOptions, sandboxWorkingDirectory: string, loggingContext: LoggingClientContext) { + constructor(options: StrykerOptions, sandboxWorkingDirectory: string, loggingContext: LoggingClientContext, private readonly log: Logger) { this.worker = ChildProcessProxy.create( require.resolve('./child-process-test-runner-worker'), loggingContext, @@ -45,7 +46,11 @@ export class ChildProcessTestRunnerDecorator implements TestRunner { this.worker.proxy.dispose().catch((error) => { // It's OK if the child process is already down. if (!(error instanceof ChildProcessCrashedError)) { - throw error; + // Handle error by logging it. We still want to kill the child process. + this.log.warn( + 'An unexpected error occurred during test runner disposal. This might be worth looking into. Stryker will ignore this error.', + error + ); } }), diff --git a/packages/core/src/test-runner/index.ts b/packages/core/src/test-runner/index.ts index d849490b66..8c1af89ddc 100644 --- a/packages/core/src/test-runner/index.ts +++ b/packages/core/src/test-runner/index.ts @@ -30,7 +30,16 @@ export function createTestRunnerFactory( new RetryRejectedDecorator( () => new MaxTestRunnerReuseDecorator( - () => new TimeoutDecorator(() => new ChildProcessTestRunnerDecorator(options, sandbox.workingDirectory, loggingContext)), + () => + new TimeoutDecorator( + () => + new ChildProcessTestRunnerDecorator( + options, + sandbox.workingDirectory, + loggingContext, + getLogger(ChildProcessTestRunnerDecorator.name) + ) + ), options ) ), diff --git a/packages/core/test/integration/test-runner/additional-test-runners.ts b/packages/core/test/integration/test-runner/additional-test-runners.ts index c8f39d4509..cf5259687b 100644 --- a/packages/core/test/integration/test-runner/additional-test-runners.ts +++ b/packages/core/test/integration/test-runner/additional-test-runners.ts @@ -112,6 +112,10 @@ class ErroredTestRunner implements TestRunner { public async mutantRun(): Promise { throw new Error('Method not implemented.'); } + + public dispose(): Promise { + throw new Error('Test runner exited with exit code 1'); + } } class RejectInitRunner implements TestRunner { diff --git a/packages/core/test/integration/test-runner/create-test-runner-factory.it.spec.ts b/packages/core/test/integration/test-runner/create-test-runner-factory.it.spec.ts index 9d9a72700f..b418f0dc60 100644 --- a/packages/core/test/integration/test-runner/create-test-runner-factory.it.spec.ts +++ b/packages/core/test/integration/test-runner/create-test-runner-factory.it.spec.ts @@ -24,6 +24,12 @@ describe(`${createTestRunnerFactory.name} integration`, () => { let loggingServer: LoggingServer; let alreadyDisposed: boolean; + function rmSync(fileName: string) { + if (fs.existsSync(fileName)) { + fs.unlinkSync(fileName); + } + } + beforeEach(async () => { // Make sure there is a logging server listening loggingServer = new LoggingServer(); @@ -39,9 +45,8 @@ describe(`${createTestRunnerFactory.name} integration`, () => { .provideValue(coreTokens.loggingContext, loggingContext) .injectFunction(createTestRunnerFactory); - if (fs.existsSync(CounterTestRunner.COUNTER_FILE)) { - await fs.unlinkSync(CounterTestRunner.COUNTER_FILE); - } + rmSync(CounterTestRunner.COUNTER_FILE); + rmSync(SecondTimeIsTheCharm.COUNTER_FILE); }); afterEach(async () => { @@ -49,10 +54,8 @@ describe(`${createTestRunnerFactory.name} integration`, () => { await sut.dispose(); } await loggingServer.dispose(); - - if (fs.existsSync(CounterTestRunner.COUNTER_FILE)) { - fs.unlinkSync(CounterTestRunner.COUNTER_FILE); - } + rmSync(CounterTestRunner.COUNTER_FILE); + rmSync(SecondTimeIsTheCharm.COUNTER_FILE); }); async function arrangeSut(name: string): Promise { @@ -125,6 +128,11 @@ describe(`${createTestRunnerFactory.name} integration`, () => { await expect(arrangeSut('reject-init')).rejectedWith('Init was rejected'); }); + it('should still shutdown the child process, even when test runner dispose rejects', async () => { + arrangeSut('errored'); + await sut.dispose(); + }); + it('should change the current working directory to the sandbox directory', async () => { await arrangeSut('verify-working-folder'); const result = await actDryRun(); diff --git a/packages/core/test/unit/test-runner/child-process-test-runner-decorator.spec.ts b/packages/core/test/unit/test-runner/child-process-test-runner-decorator.spec.ts index a46d8c2b72..0fb8b77bde 100644 --- a/packages/core/test/unit/test-runner/child-process-test-runner-decorator.spec.ts +++ b/packages/core/test/unit/test-runner/child-process-test-runner-decorator.spec.ts @@ -3,7 +3,7 @@ import { expect } from 'chai'; import sinon from 'sinon'; import { Task } from '@stryker-mutator/util'; import { TestRunner } from '@stryker-mutator/api/test-runner'; -import { factory } from '@stryker-mutator/test-helpers'; +import { factory, testInjector } from '@stryker-mutator/test-helpers'; import { ChildProcessCrashedError } from '../../../src/child-proxy/child-process-crashed-error'; import { ChildProcessProxy } from '../../../src/child-proxy/child-process-proxy'; @@ -36,7 +36,7 @@ describe(ChildProcessTestRunnerDecorator.name, () => { }); function createSut(): ChildProcessTestRunnerDecorator { - return new ChildProcessTestRunnerDecorator(options, 'a working directory', loggingContext); + return new ChildProcessTestRunnerDecorator(options, 'a working directory', loggingContext, testInjector.logger); } it('should create the child process proxy', () => { @@ -97,6 +97,19 @@ describe(ChildProcessTestRunnerDecorator.name, () => { childProcessProxyMock.proxy.dispose.rejects(new ChildProcessCrashedError(1, '1')); await sut.dispose(); expect(childProcessProxyMock.dispose).called; + expect(testInjector.logger.warn).not.called; + }); + + it('should log, but not reject, when the child process rejects', async () => { + const sut = createSut(); + const expectedError = new Error('Could not divide by zero 🤷‍♀️'); + childProcessProxyMock.proxy.dispose.rejects(expectedError); + await sut.dispose(); + expect(childProcessProxyMock.dispose).called; + expect(testInjector.logger.warn).calledWithExactly( + 'An unexpected error occurred during test runner disposal. This might be worth looking into. Stryker will ignore this error.', + expectedError + ); }); it('should only wait 2 seconds for the test runner to be disposed', async () => { diff --git a/packages/karma-runner/src/karma-test-runner.ts b/packages/karma-runner/src/karma-test-runner.ts index 69ed3642af..dac6f59e6c 100644 --- a/packages/karma-runner/src/karma-test-runner.ts +++ b/packages/karma-runner/src/karma-test-runner.ts @@ -1,3 +1,5 @@ +import { promisify } from 'util'; + import karma from 'karma'; import { StrykerOptions, MutantCoverage } from '@stryker-mutator/api/core'; import { Logger, LoggerFactoryMethod } from '@stryker-mutator/api/logging'; @@ -11,6 +13,7 @@ import { DryRunResult, MutantRunResult, toMutantRunResult, + MutantRunStatus, } from '@stryker-mutator/api/test-runner'; import { StrykerKarmaSetup } from '../src-generated/karma-runner-options'; @@ -32,6 +35,7 @@ export class KarmaTestRunner implements TestRunner { private currentCoverageReport?: MutantCoverage; private readonly starter: ProjectStarter; public port: number | undefined; + private exitPromise: Promise | undefined; public static inject = tokens(commonTokens.logger, commonTokens.getLogger, commonTokens.options); constructor(private readonly log: Logger, getLogger: LoggerFactoryMethod, options: StrykerOptions) { @@ -47,8 +51,19 @@ export class KarmaTestRunner implements TestRunner { public async init(): Promise { return new Promise((res, rej) => { - StrykerReporter.instance.once('browsers_ready', () => res()); - this.starter.start().catch(rej); + let promiseResolved = false; + StrykerReporter.instance.once('browsers_ready', () => { + promiseResolved = true; + res(); + }); + this.exitPromise = this.starter.start().catch((err) => { + if (promiseResolved) { + throw err; + } else { + rej(err); + return 0; + } + }); }); } @@ -60,6 +75,13 @@ export class KarmaTestRunner implements TestRunner { public async mutantRun(options: MutantRunOptions): Promise { TestHooksMiddleware.instance.configureActiveMutant(options); const dryRunResult = await this.run(); + if ( + dryRunResult.status === DryRunStatus.Error && + /Disconnected reconnect failed before timeout of \d+ms \(ping timeout\)/.exec(dryRunResult.errorMessage) + ) { + // Browser is stuck in an infinite loop, see https://github.com/stryker-mutator/stryker-js/issues/2989#issuecomment-885461482 + return { status: MutantRunStatus.Timeout }; + } return toMutantRunResult(dryRunResult); } @@ -74,6 +96,11 @@ export class KarmaTestRunner implements TestRunner { return runResult; } + public async dispose(): Promise { + await promisify(karma.stopper.stop).bind(karma.stopper)({ port: this.port }); + await this.exitPromise; + } + private loadSetup(options: StrykerOptions): StrykerKarmaSetup { const defaultKarmaConfig: StrykerKarmaSetup = { projectType: 'custom', diff --git a/packages/karma-runner/src/starters/angular-starter.ts b/packages/karma-runner/src/starters/angular-starter.ts index 544bacc538..68c366c54b 100644 --- a/packages/karma-runner/src/starters/angular-starter.ts +++ b/packages/karma-runner/src/starters/angular-starter.ts @@ -10,7 +10,7 @@ import { NgConfigOptions, NgTestArguments } from '../../src-generated/karma-runn const MIN_ANGULAR_CLI_VERSION = '6.1.0'; -export async function start(getLogger: LoggerFactoryMethod, ngConfig?: NgConfigOptions): Promise { +export async function start(getLogger: LoggerFactoryMethod, ngConfig?: NgConfigOptions): Promise { const logger: Logger = getLogger(path.basename(__filename)); verifyAngularCliVersion(); @@ -44,6 +44,7 @@ export async function start(getLogger: LoggerFactoryMethod, ngConfig?: NgConfigO `\`ng test\` command failed with exit code ${exitCode}. Please run with logLevel 'trace' to see the angular-cli console output (actual command was ${actualCommand})` ); } + return exitCode; }); } diff --git a/packages/karma-runner/src/starters/karma-starter.ts b/packages/karma-runner/src/starters/karma-starter.ts index 1cd252db9d..b6c6d115b0 100644 --- a/packages/karma-runner/src/starters/karma-starter.ts +++ b/packages/karma-runner/src/starters/karma-starter.ts @@ -1,9 +1,16 @@ import { requireResolve } from '@stryker-mutator/util'; -export async function start(): Promise { +export function start(): Promise { // Make sure require karma from inside this function, that way it won't break if karma isn't installed and this file is required. - const karma: any = requireResolve('karma'); - await new karma.Server({ - configFile: require.resolve('./stryker-karma.conf'), - }).start(); + const karma = requireResolve('karma') as typeof import('karma'); + return new Promise((res, rej) => { + new karma.Server( + { + configFile: require.resolve('./stryker-karma.conf'), + }, + res + ) + .start() + .catch(rej); + }); } diff --git a/packages/karma-runner/src/starters/project-starter.ts b/packages/karma-runner/src/starters/project-starter.ts index 07a3d53708..4fd38daf7f 100644 --- a/packages/karma-runner/src/starters/project-starter.ts +++ b/packages/karma-runner/src/starters/project-starter.ts @@ -7,7 +7,7 @@ import * as karmaStarter from './karma-starter'; export class ProjectStarter { constructor(private readonly getLogger: LoggerFactoryMethod, private readonly setup: StrykerKarmaSetup) {} - public start(): Promise { + public start(): Promise { if (this.setup.projectType === 'angular-cli') { return angularStarter.start(this.getLogger, this.setup.ngConfig); } else { diff --git a/packages/karma-runner/test/integration/angular.it.spec.ts b/packages/karma-runner/test/integration/angular.it.spec.ts new file mode 100644 index 0000000000..cf37ef1ef8 --- /dev/null +++ b/packages/karma-runner/test/integration/angular.it.spec.ts @@ -0,0 +1,24 @@ +import { testInjector } from '@stryker-mutator/test-helpers'; +import { expect } from 'chai'; + +import { KarmaTestRunner } from '../../src/karma-test-runner'; +import { KarmaRunnerOptionsWithStrykerOptions } from '../../src/karma-runner-options-with-stryker-options'; +import { StrykerReporter } from '../../src/karma-plugins/stryker-reporter'; +import { resolveTestResource } from '../helpers/resolve-test-resource'; + +describe.only(`${KarmaTestRunner.name} on using projectType angular`, () => { + afterEach(() => { + StrykerReporter.instance.removeAllListeners(); + }); + + it('should reject when no angular cli is available', async () => { + (testInjector.options as KarmaRunnerOptionsWithStrykerOptions).karma = { + projectType: 'angular-cli', + configFile: resolveTestResource('sampleProject', 'karma-jasmine.conf.js'), + }; + const sut = testInjector.injector.injectClass(KarmaTestRunner); + await expect(sut.init()).rejectedWith("Cannot find module '@angular/cli"); + }); + + // Other tests are done in e2e testing 🤷‍♀️ +}); diff --git a/packages/karma-runner/test/integration/instrumented.it.spec.ts b/packages/karma-runner/test/integration/instrumented.it.spec.ts index e8b9b8f163..b7a2bc9ad0 100644 --- a/packages/karma-runner/test/integration/instrumented.it.spec.ts +++ b/packages/karma-runner/test/integration/instrumented.it.spec.ts @@ -24,8 +24,9 @@ describe(`${KarmaTestRunner.name} running on instrumented code`, () => { await sut.init(); }); - after(() => { + after(async () => { StrykerReporter.instance.removeAllListeners(); + await sut.dispose(); }); describe('dryRun', () => { @@ -158,8 +159,9 @@ describe(`${KarmaTestRunner.name} running on instrumented code`, () => { sut = createSut(); await sut.init(); }); - after(() => { + after(async () => { StrykerReporter.instance.removeAllListeners(); + await sut.dispose(); }); describe('dryRun', () => { diff --git a/packages/karma-runner/test/integration/karma-test-runner.it.spec.ts b/packages/karma-runner/test/integration/karma-test-runner.it.spec.ts index d1d0f35d46..2675f283ca 100644 --- a/packages/karma-runner/test/integration/karma-test-runner.it.spec.ts +++ b/packages/karma-runner/test/integration/karma-test-runner.it.spec.ts @@ -49,8 +49,9 @@ describe(`${KarmaTestRunner.name} integration`, () => { sut = createSut(); return sut.init(); }); - after(() => { + after(async () => { StrykerReporter.instance.removeAllListeners(); + await sut.dispose(); }); describe('dryRun()', () => { @@ -85,8 +86,9 @@ describe(`${KarmaTestRunner.name} integration`, () => { sut = createSut(); return sut.init(); }); - after(() => { + after(async () => { StrykerReporter.instance.removeAllListeners(); + await sut.dispose(); }); describe('dryRun', () => { it('should report the first failed test (bail)', async () => { @@ -117,8 +119,9 @@ describe(`${KarmaTestRunner.name} integration`, () => { sut = createSut(); return sut.init(); }); - after(() => { + after(async () => { StrykerReporter.instance.removeAllListeners(); + await sut.dispose(); }); describe('dryRun', () => { it('should report Error with the error message', async () => { @@ -143,8 +146,9 @@ describe(`${KarmaTestRunner.name} integration`, () => { sut = createSut(); return sut.init(); }); - after(() => { + after(async () => { StrykerReporter.instance.removeAllListeners(); + await sut.dispose(); }); it('should report Complete without errors', async () => { const runResult = await sut.dryRun(factory.dryRunOptions()); @@ -164,8 +168,9 @@ describe(`${KarmaTestRunner.name} integration`, () => { sut = createSut(); return sut.init(); }); - after(() => { + after(async () => { StrykerReporter.instance.removeAllListeners(); + await sut.dispose(); }); it('should report Complete without errors', async () => { const runResult = await sut.dryRun(factory.dryRunOptions()); @@ -188,6 +193,7 @@ describe(`${KarmaTestRunner.name} integration`, () => { await dummyServer.dispose(); } StrykerReporter.instance.removeAllListeners(); + await sut.dispose(); }); it('should choose different port automatically and report Complete without errors', async () => { diff --git a/packages/karma-runner/test/integration/read-config.it.spec.ts b/packages/karma-runner/test/integration/read-config.it.spec.ts index 672b3fa17d..a239463f4e 100644 --- a/packages/karma-runner/test/integration/read-config.it.spec.ts +++ b/packages/karma-runner/test/integration/read-config.it.spec.ts @@ -7,16 +7,20 @@ import { StrykerReporter } from '../../src/karma-plugins/stryker-reporter'; import { resolveTestResource } from '../helpers/resolve-test-resource'; describe('read config integration', () => { - afterEach(() => { + let sut: KarmaTestRunner | undefined; + + after(async () => { StrykerReporter.instance.removeAllListeners(); + await sut?.dispose(); }); + it('should not override client options in a mocha project', async () => { testInjector.options.karma = { configFile: resolveTestResource('configs', 'mocha-client-options-karma.conf.js'), }; - const runner = testInjector.injector.injectClass(KarmaTestRunner); - await runner.init(); - const dryRunResult = await runner.dryRun(factory.dryRunOptions()); + sut = testInjector.injector.injectClass(KarmaTestRunner); + await sut.init(); + const dryRunResult = await sut.dryRun(factory.dryRunOptions()); assertions.expectCompleted(dryRunResult); expect(dryRunResult.tests).lengthOf(2); const [test1, test2] = dryRunResult.tests; @@ -30,9 +34,9 @@ describe('read config integration', () => { testInjector.options.karma = { configFile: resolveTestResource('configs', 'jasmine-client-options-karma.conf.js'), }; - const runner = testInjector.injector.injectClass(KarmaTestRunner); - await runner.init(); - const dryRunResult = await runner.dryRun(factory.dryRunOptions()); + sut = testInjector.injector.injectClass(KarmaTestRunner); + await sut.init(); + const dryRunResult = await sut.dryRun(factory.dryRunOptions()); assertions.expectCompleted(dryRunResult); expect(dryRunResult.tests).lengthOf(3); const [test1, test2, test3] = dryRunResult.tests; diff --git a/packages/karma-runner/test/integration/sample.it.spec.ts b/packages/karma-runner/test/integration/sample.it.spec.ts index c0a176d63a..63fa8f5571 100644 --- a/packages/karma-runner/test/integration/sample.it.spec.ts +++ b/packages/karma-runner/test/integration/sample.it.spec.ts @@ -8,15 +8,18 @@ import { StrykerReporter } from '../../src/karma-plugins/stryker-reporter'; import { resolveTestResource } from '../helpers/resolve-test-resource'; describe('Sample project', () => { - afterEach(() => { + let sut: KarmaTestRunner | undefined; + + afterEach(async () => { StrykerReporter.instance.removeAllListeners(); + await sut?.dispose(); }); it('should be able to run karma with jasmine', async () => { testInjector.options.karma = { configFile: resolveTestResource('sampleProject', 'karma-jasmine.conf.js') }; - const runner = testInjector.injector.injectClass(KarmaTestRunner); - await runner.init(); - const result = await runner.dryRun(factory.dryRunOptions()); + sut = testInjector.injector.injectClass(KarmaTestRunner); + await sut.init(); + const result = await sut.dryRun(factory.dryRunOptions()); assertions.expectCompleted(result); const expectedTestResults: TimelessTestResult[] = [ { @@ -65,9 +68,9 @@ describe('Sample project', () => { configFile: resolveTestResource('sampleProject', 'karma-mocha.conf.js'), }; - const runner = testInjector.injector.injectClass(KarmaTestRunner); - await runner.init(); - const result = await runner.dryRun(factory.dryRunOptions()); + sut = testInjector.injector.injectClass(KarmaTestRunner); + await sut.init(); + const result = await sut.dryRun(factory.dryRunOptions()); assertions.expectCompleted(result); const expectedTestResults: TimelessTestResult[] = [ { diff --git a/packages/karma-runner/test/integration/timeout-on-infinite-loop.it.spec.ts b/packages/karma-runner/test/integration/timeout-on-infinite-loop.it.spec.ts new file mode 100644 index 0000000000..4d41f07867 --- /dev/null +++ b/packages/karma-runner/test/integration/timeout-on-infinite-loop.it.spec.ts @@ -0,0 +1,34 @@ +'use strict'; +Object.defineProperty(exports, '__esModule', { value: true }); +import { assertions, factory, testInjector } from '@stryker-mutator/test-helpers'; + +import { StrykerReporter } from '../../src/karma-plugins/stryker-reporter'; +import { KarmaTestRunner } from '../../src/karma-test-runner'; +import { resolveTestResource } from '../helpers/resolve-test-resource'; + +describe('Infinite loop', () => { + let sut: KarmaTestRunner; + beforeEach(async () => { + const karmaOptions = { + config: { + framework: ['mocha'], + files: [resolveTestResource('infinite-loop', 'infinite-loop.spec.js')], + }, + }; + testInjector.options.karma = karmaOptions; + sut = testInjector.injector.injectClass(KarmaTestRunner); + await sut.init(); + }); + afterEach(async () => { + StrykerReporter.instance.removeAllListeners(); + await sut.dispose(); + }); + + it('should report a timeout eventually', async () => { + // Act + const result = await sut.mutantRun(factory.mutantRunOptions()); + + // Assert + assertions.expectTimeout(result); + }); +}); diff --git a/packages/karma-runner/testResources/infinite-loop/infinite-loop.spec.js b/packages/karma-runner/testResources/infinite-loop/infinite-loop.spec.js new file mode 100644 index 0000000000..e408342e2e --- /dev/null +++ b/packages/karma-runner/testResources/infinite-loop/infinite-loop.spec.js @@ -0,0 +1,4 @@ +it('should handle an infinite loop as a timeout', () => { + while(true){ + } +}); diff --git a/packages/test-helpers/src/assertions.ts b/packages/test-helpers/src/assertions.ts index a85794a45f..df10c794a8 100644 --- a/packages/test-helpers/src/assertions.ts +++ b/packages/test-helpers/src/assertions.ts @@ -15,6 +15,7 @@ import { TestResult, FailedTestResult, TestStatus, + TimeoutMutantRunResult, } from '@stryker-mutator/api/test-runner'; import { File } from '@stryker-mutator/api/core'; import { CheckResult, FailedCheckResult, CheckStatus } from '@stryker-mutator/api/check'; @@ -23,6 +24,10 @@ export function expectKilled(result: MutantRunResult): asserts result is KilledM assert.strictEqual(result.status, MutantRunStatus.Killed, result.status === MutantRunStatus.Error ? result.errorMessage : ''); } +export function expectTimeout(result: MutantRunResult): asserts result is TimeoutMutantRunResult { + assert.strictEqual(result.status, MutantRunStatus.Timeout); +} + export function expectFailed(result: TestResult): asserts result is FailedTestResult { assert.strictEqual(result.status, TestStatus.Failed); }