From 91495dd8d567f238b5e453ad5f30dec5716a0a11 Mon Sep 17 00:00:00 2001 From: Nico Jansen Date: Mon, 23 Jul 2018 22:55:21 -0700 Subject: [PATCH] refactor(Task): Refactor task to use `Promise.race` Don't reinvent the wheel --- .../src/StrykerMochaReporter.ts | 12 +++-- .../src/child-proxy/ChildProcessProxy.ts | 6 +-- packages/stryker/src/utils/Task.ts | 45 +++++++------------ packages/stryker/src/utils/objectUtils.ts | 2 +- .../child-proxy/ChildProcessProxy.it.ts | 2 +- packages/stryker/test/unit/SandboxPoolSpec.ts | 2 +- .../ChildProcessTestRunnerDecoratorSpec.ts | 2 +- 7 files changed, 32 insertions(+), 39 deletions(-) diff --git a/packages/stryker-mocha-runner/src/StrykerMochaReporter.ts b/packages/stryker-mocha-runner/src/StrykerMochaReporter.ts index b80d524570..07fd557877 100644 --- a/packages/stryker-mocha-runner/src/StrykerMochaReporter.ts +++ b/packages/stryker-mocha-runner/src/StrykerMochaReporter.ts @@ -3,7 +3,7 @@ import { getLogger } from 'stryker-api/logging'; import Timer from './Timer'; export default class StrykerMochaReporter { - + private readonly log = getLogger(StrykerMochaReporter.name); public runResult: RunResult; private timer = new Timer(); @@ -36,7 +36,9 @@ export default class StrykerMochaReporter { }); this.passedCount++; this.timer.reset(); - this.log.debug(`Test passed: ${test.fullTitle()}`); + if (this.log.isTraceEnabled()) { + this.log.trace(`Test passed: ${test.fullTitle()}`); + } }); this.runner.on('fail', (test: any, err: any) => { @@ -50,12 +52,14 @@ export default class StrykerMochaReporter { this.runResult.errorMessages = []; } this.runResult.errorMessages.push(err.message); - this.log.debug(`Test failed: ${test.fullTitle()}. Error: ${err.message}`); + if (this.log.isTraceEnabled()) { + this.log.trace(`Test failed: ${test.fullTitle()}. Error: ${err.message}`); + } }); this.runner.on('end', () => { this.runResult.status = RunStatus.Complete; - this.log.debug(`Mocha test run completed: ${this.passedCount}/${this.runResult.tests.length} passed`); + this.log.debug('Mocha test run completed: %s/%s passed', this.passedCount, this.runResult.tests.length); }); } } \ No newline at end of file diff --git a/packages/stryker/src/child-proxy/ChildProcessProxy.ts b/packages/stryker/src/child-proxy/ChildProcessProxy.ts index 82f531397d..5622777f07 100644 --- a/packages/stryker/src/child-proxy/ChildProcessProxy.ts +++ b/packages/stryker/src/child-proxy/ChildProcessProxy.ts @@ -4,7 +4,7 @@ import { File } from 'stryker-api/core'; import { getLogger } from 'stryker-api/logging'; import { WorkerMessage, WorkerMessageKind, ParentMessage, autoStart, ParentMessageKind } from './messageProtocol'; import { serialize, deserialize, kill, isErrnoException } from '../utils/objectUtils'; -import Task from '../utils/Task'; +import { Task, ExpirableTask } from '../utils/Task'; import LoggingClientContext from '../logging/LoggingClientContext'; import StrykerError from '../utils/StrykerError'; import ChildProcessCrashedError from './ChildProcessCrashedError'; @@ -24,7 +24,7 @@ export default class ChildProcessProxy { private worker: ChildProcess; private initTask: Task; - private disposeTask: Task | undefined; + private disposeTask: ExpirableTask | undefined; private currentError: StrykerError | undefined; private workerTasks: Task[] = []; private log = getLogger(ChildProcessProxy.name); @@ -203,7 +203,7 @@ export default class ChildProcessProxy { kill(this.worker.pid); this.isDisposed = true; }; - this.disposeTask = new Task(TIMEOUT_FOR_DISPOSE); + this.disposeTask = new ExpirableTask(TIMEOUT_FOR_DISPOSE); this.send({ kind: WorkerMessageKind.Dispose }); return this.disposeTask.promise .then(killWorker) diff --git a/packages/stryker/src/utils/Task.ts b/packages/stryker/src/utils/Task.ts index 4e7cae239d..9580864f7a 100644 --- a/packages/stryker/src/utils/Task.ts +++ b/packages/stryker/src/utils/Task.ts @@ -1,59 +1,48 @@ +import { sleep } from './objectUtils'; /** * Wraps a promise in a Task api for convenience. */ -export default class Task { +export class Task { - private _promise: Promise; + protected _promise: Promise; private resolveFn: (value?: T | PromiseLike) => void; private rejectFn: (reason: any) => void; private _isResolved = false; - private timeout: NodeJS.Timer; - constructor(timeoutMs?: number, private timeoutHandler?: () => PromiseLike) { + constructor() { this._promise = new Promise((resolve, reject) => { this.resolveFn = resolve; this.rejectFn = reject; }); - if (timeoutMs) { - this.timeout = setTimeout(() => this.handleTimeout(), timeoutMs); - } - } - - get isResolved() { - return this._isResolved; } get promise() { return this._promise; } - handleTimeout() { - if (this.timeoutHandler) { - this.timeoutHandler().then(val => this.resolve(val)); - } else { - this.resolve(undefined); - } - } - - chainTo(promise: Promise) { - promise.then(value => this.resolve(value), reason => this.reject(reason)); + get isResolved() { + return this._isResolved; } resolve(result: undefined | T | PromiseLike) { - this.resolveTimeout(); + this._isResolved = true; this.resolveFn(result); } reject(reason: any) { - this.resolveTimeout(); + this._isResolved = true; this.rejectFn(reason); } +} - private resolveTimeout() { - if (this.timeout) { - clearTimeout(this.timeout); - } - this._isResolved = true; +/** + * A task that can expire after the given time. + * If that happens, the inner promise is resolved + */ +export class ExpirableTask extends Task { + constructor(timeoutMS: number) { + super(); + this._promise = Promise.race([this._promise, sleep(timeoutMS)]); } } \ No newline at end of file diff --git a/packages/stryker/src/utils/objectUtils.ts b/packages/stryker/src/utils/objectUtils.ts index 40567b8f65..5beb42cf64 100644 --- a/packages/stryker/src/utils/objectUtils.ts +++ b/packages/stryker/src/utils/objectUtils.ts @@ -102,7 +102,7 @@ export function kill(pid: number): Promise { }); } -export function sleep(ms: number) { +export function sleep(ms: number): Promise { return new Promise(res => { setTimeout(res, ms); }); diff --git a/packages/stryker/test/integration/child-proxy/ChildProcessProxy.it.ts b/packages/stryker/test/integration/child-proxy/ChildProcessProxy.it.ts index 6cbc01fb06..6751c269c8 100644 --- a/packages/stryker/test/integration/child-proxy/ChildProcessProxy.it.ts +++ b/packages/stryker/test/integration/child-proxy/ChildProcessProxy.it.ts @@ -6,7 +6,7 @@ import { File, LogLevel } from 'stryker-api/core'; import { Logger } from 'stryker-api/logging'; import Echo from './Echo'; import ChildProcessProxy from '../../../src/child-proxy/ChildProcessProxy'; -import Task from '../../../src/utils/Task'; +import { Task } from '../../../src/utils/Task'; import LoggingServer from '../../helpers/LoggingServer'; import { filter } from 'rxjs/operators'; import { Mock } from '../../helpers/producers'; diff --git a/packages/stryker/test/unit/SandboxPoolSpec.ts b/packages/stryker/test/unit/SandboxPoolSpec.ts index 694fe0ce68..01321e84c4 100644 --- a/packages/stryker/test/unit/SandboxPoolSpec.ts +++ b/packages/stryker/test/unit/SandboxPoolSpec.ts @@ -6,7 +6,7 @@ import { File, LogLevel } from 'stryker-api/core'; import { TestFramework } from 'stryker-api/test_framework'; import Sandbox from '../../src/Sandbox'; import SandboxPool from '../../src/SandboxPool'; -import Task from '../../src/utils/Task'; +import { Task } from '../../src/utils/Task'; import '../helpers/globals'; import { Mock, config, file, mock, testFramework } from '../helpers/producers'; import LoggingClientContext from '../../src/logging/LoggingClientContext'; diff --git a/packages/stryker/test/unit/test-runner/ChildProcessTestRunnerDecoratorSpec.ts b/packages/stryker/test/unit/test-runner/ChildProcessTestRunnerDecoratorSpec.ts index 26b350c9e5..465af335b8 100644 --- a/packages/stryker/test/unit/test-runner/ChildProcessTestRunnerDecoratorSpec.ts +++ b/packages/stryker/test/unit/test-runner/ChildProcessTestRunnerDecoratorSpec.ts @@ -8,7 +8,7 @@ import ChildProcessProxy from '../../../src/child-proxy/ChildProcessProxy'; import LoggingClientContext from '../../../src/logging/LoggingClientContext'; import ChildProcessTestRunnerWorker from '../../../src/test-runner/ChildProcessTestRunnerWorker'; import TestRunnerDecorator from '../../../src/test-runner/TestRunnerDecorator'; -import Task from '../../../src/utils/Task'; +import { Task } from '../../../src/utils/Task'; import ChildProcessCrashedError from '../../../src/child-proxy/ChildProcessCrashedError'; describe(ChildProcessTestRunnerDecorator.name, () => {