From 84e3681b7948fdfe0c21e31ff06c4147c381a9df Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Mon, 8 Jan 2018 12:28:04 -0800 Subject: [PATCH] Support timeouts in the parallel runner (#20631) * Support timeouts in the parallel runner * Apply PR feedback: unify code paths, use string as sentinel --- Gulpfile.ts | 21 +++++++++---- Jakefile.js | 13 ++++---- src/harness/parallel/host.ts | 37 +++++++++++++++++++++- src/harness/parallel/shared.ts | 3 +- src/harness/parallel/worker.ts | 56 +++++++++++++++++++++++++--------- src/harness/runner.ts | 5 +++ 6 files changed, 106 insertions(+), 29 deletions(-) diff --git a/Gulpfile.ts b/Gulpfile.ts index 996fa1fc140..510c0d87631 100644 --- a/Gulpfile.ts +++ b/Gulpfile.ts @@ -680,14 +680,14 @@ function runConsoleTests(defaultReporter: string, runInParallel: boolean, done: workerCount = cmdLineOptions.workers; } - if (tests || runners || light || taskConfigsFolder) { - writeTestConfigFile(tests, runners, light, taskConfigsFolder, workerCount, stackTraceLimit); - } - if (tests && tests.toLocaleLowerCase() === "rwc") { testTimeout = 400000; } + if (tests || runners || light || testTimeout || taskConfigsFolder) { + writeTestConfigFile(tests, runners, light, taskConfigsFolder, workerCount, stackTraceLimit, testTimeout); + } + const colors = cmdLineOptions.colors; const reporter = cmdLineOptions.reporter || defaultReporter; @@ -872,8 +872,17 @@ function cleanTestDirs(done: (e?: any) => void) { } // used to pass data from jake command line directly to run.js -function writeTestConfigFile(tests: string, runners: string, light: boolean, taskConfigsFolder?: string, workerCount?: number, stackTraceLimit?: string) { - const testConfigContents = JSON.stringify({ test: tests ? [tests] : undefined, runner: runners ? runners.split(",") : undefined, light, workerCount, stackTraceLimit, taskConfigsFolder, noColor: !cmdLineOptions.colors }); +function writeTestConfigFile(tests: string, runners: string, light: boolean, taskConfigsFolder?: string, workerCount?: number, stackTraceLimit?: string, timeout?: number) { + const testConfigContents = JSON.stringify({ + test: tests ? [tests] : undefined, + runner: runners ? runners.split(",") : undefined, + light, + workerCount, + stackTraceLimit, + taskConfigsFolder, + noColor: !cmdLineOptions.colors, + timeout, + }); console.log("Running tests with config: " + testConfigContents); fs.writeFileSync("test.config", testConfigContents); } diff --git a/Jakefile.js b/Jakefile.js index f2a15aa4e19..09155261434 100644 --- a/Jakefile.js +++ b/Jakefile.js @@ -858,7 +858,7 @@ function cleanTestDirs() { } // used to pass data from jake command line directly to run.js -function writeTestConfigFile(tests, runners, light, taskConfigsFolder, workerCount, stackTraceLimit, colors) { +function writeTestConfigFile(tests, runners, light, taskConfigsFolder, workerCount, stackTraceLimit, colors, testTimeout) { var testConfigContents = JSON.stringify({ runners: runners ? runners.split(",") : undefined, test: tests ? [tests] : undefined, @@ -866,7 +866,8 @@ function writeTestConfigFile(tests, runners, light, taskConfigsFolder, workerCou workerCount: workerCount, taskConfigsFolder: taskConfigsFolder, stackTraceLimit: stackTraceLimit, - noColor: !colors + noColor: !colors, + timeout: testTimeout }); fs.writeFileSync('test.config', testConfigContents); } @@ -908,14 +909,14 @@ function runConsoleTests(defaultReporter, runInParallel) { workerCount = process.env.workerCount || process.env.p || os.cpus().length; } - if (tests || runners || light || taskConfigsFolder) { - writeTestConfigFile(tests, runners, light, taskConfigsFolder, workerCount, stackTraceLimit, colors); - } - if (tests && tests.toLocaleLowerCase() === "rwc") { testTimeout = 800000; } + if (tests || runners || light || testTimeout || taskConfigsFolder) { + writeTestConfigFile(tests, runners, light, taskConfigsFolder, workerCount, stackTraceLimit, colors, testTimeout); + } + var colorsFlag = process.env.color || process.env.colors; var colors = colorsFlag !== "false" && colorsFlag !== "0"; var reporter = process.env.reporter || process.env.r || defaultReporter; diff --git a/src/harness/parallel/host.ts b/src/harness/parallel/host.ts index eca719628c7..56b8df5ecfe 100644 --- a/src/harness/parallel/host.ts +++ b/src/harness/parallel/host.ts @@ -9,6 +9,8 @@ namespace Harness.Parallel.Host { on(event: "error", listener: (err: Error) => void): this; on(event: "exit", listener: (code: number, signal: string) => void): this; on(event: "message", listener: (message: ParallelClientMessage) => void): this; + kill(signal?: string): void; + currentTasks?: {file: string}[]; // Custom monkeypatch onto child process handle } interface ProgressBarsOptions { @@ -134,6 +136,11 @@ namespace Harness.Parallel.Host { const newPerfData: {[testHash: string]: number} = {}; const workers: ChildProcessPartial[] = []; + const defaultTimeout = globalTimeout !== undefined + ? globalTimeout + : mocha && mocha.suite && mocha.suite._timeout + ? mocha.suite._timeout + : 20000; // 20 seconds let closedWorkers = 0; for (let i = 0; i < workerCount; i++) { // TODO: Just send the config over the IPC channel or in the command line arguments @@ -141,6 +148,14 @@ namespace Harness.Parallel.Host { const configPath = ts.combinePaths(taskConfigsFolder, `task-config${i}.json`); Harness.IO.writeFile(configPath, JSON.stringify(config)); const child = fork(__filename, [`--config="${configPath}"`]); + let currentTimeout = defaultTimeout; + const killChild = () => { + child.kill(); + console.error(`Worker exceeded timeout ${child.currentTasks && child.currentTasks.length ? `while running test '${child.currentTasks[0].file}'.` : `during test setup.`}`); + return process.exit(2); + }; + let timer = setTimeout(killChild, currentTimeout); + const timeoutStack: number[] = []; child.on("error", err => { console.error("Unexpected error in child process:"); console.error(err); @@ -160,8 +175,23 @@ namespace Harness.Parallel.Host { Stack: ${data.payload.stack}`); return process.exit(2); } + case "timeout": { + if (data.payload.duration === "reset") { + currentTimeout = timeoutStack.pop() || defaultTimeout; + } + else { + timeoutStack.push(currentTimeout); + currentTimeout = data.payload.duration; + } + break; + } case "progress": case "result": { + clearTimeout(timer); + timer = setTimeout(killChild, currentTimeout); + if (child.currentTasks) { + child.currentTasks.shift(); + } totalPassing += data.payload.passing; if (data.payload.errors.length) { errorResults = errorResults.concat(data.payload.errors); @@ -195,6 +225,7 @@ namespace Harness.Parallel.Host { while (tasks.length && taskList.reduce((p, c) => p + c.size, 0) < chunkSize) { taskList.push(tasks.pop()); } + child.currentTasks = taskList; if (taskList.length === 1) { child.send({ type: "test", payload: taskList[0] }); } @@ -252,18 +283,22 @@ namespace Harness.Parallel.Host { for (const worker of workers) { const payload = batches.pop(); if (payload) { + worker.currentTasks = payload; worker.send({ type: "batch", payload }); } else { // Out of batches, send off just one test const payload = tasks.pop(); ts.Debug.assert(!!payload); // The reserve kept above should ensure there is always an initial task available, even in suboptimal scenarios + worker.currentTasks = [payload]; worker.send({ type: "test", payload }); } } } else { for (let i = 0; i < workerCount; i++) { - workers[i].send({ type: "test", payload: tasks.pop() }); + const task = tasks.pop(); + workers[i].currentTasks = [task]; + workers[i].send({ type: "test", payload: task }); } } diff --git a/src/harness/parallel/shared.ts b/src/harness/parallel/shared.ts index 2eb7777f828..26b03ac6234 100644 --- a/src/harness/parallel/shared.ts +++ b/src/harness/parallel/shared.ts @@ -10,5 +10,6 @@ namespace Harness.Parallel { export type ErrorInfo = ParallelErrorMessage["payload"] & { name: string[] }; export type ParallelResultMessage = { type: "result", payload: { passing: number, errors: ErrorInfo[], duration: number, runner: TestRunnerKind | "unittest", file: string } } | never; export type ParallelBatchProgressMessage = { type: "progress", payload: ParallelResultMessage["payload"] } | never; - export type ParallelClientMessage = ParallelErrorMessage | ParallelResultMessage | ParallelBatchProgressMessage; + export type ParallelTimeoutChangeMessage = { type: "timeout", payload: { duration: number | "reset" } } | never; + export type ParallelClientMessage = ParallelErrorMessage | ParallelResultMessage | ParallelBatchProgressMessage | ParallelTimeoutChangeMessage; } \ No newline at end of file diff --git a/src/harness/parallel/worker.ts b/src/harness/parallel/worker.ts index 013992a8e35..1cfc20cbf9b 100644 --- a/src/harness/parallel/worker.ts +++ b/src/harness/parallel/worker.ts @@ -36,11 +36,28 @@ namespace Harness.Parallel.Worker { }) as Mocha.ITestDefinition; } + function setTimeoutAndExecute(timeout: number | undefined, f: () => void) { + if (timeout !== undefined) { + const timeoutMsg: ParallelTimeoutChangeMessage = { type: "timeout", payload: { duration: timeout } }; + process.send(timeoutMsg); + } + f(); + if (timeout !== undefined) { + // Reset timeout + const timeoutMsg: ParallelTimeoutChangeMessage = { type: "timeout", payload: { duration: "reset" } }; + process.send(timeoutMsg); + } + } + function executeSuiteCallback(name: string, callback: MochaCallback) { + let timeout: number; const fakeContext: Mocha.ISuiteCallbackContext = { retries() { return this; }, slow() { return this; }, - timeout() { return this; }, + timeout(n) { + timeout = n; + return this; + }, }; namestack.push(name); let beforeFunc: Callable; @@ -71,7 +88,10 @@ namespace Harness.Parallel.Worker { finally { beforeFunc = undefined; } - testList.forEach(({ name, callback, kind }) => executeCallback(name, callback, kind)); + + setTimeoutAndExecute(timeout, () => { + testList.forEach(({ name, callback, kind }) => executeCallback(name, callback, kind)); + }); try { if (afterFunc) { @@ -103,9 +123,13 @@ namespace Harness.Parallel.Worker { } function executeTestCallback(name: string, callback: MochaCallback) { + let timeout: number; const fakeContext: Mocha.ITestCallbackContext = { skip() { return this; }, - timeout() { return this; }, + timeout(n) { + timeout = n; + return this; + }, retries() { return this; }, slow() { return this; }, }; @@ -121,18 +145,20 @@ namespace Harness.Parallel.Worker { } } if (callback.length === 0) { - try { - // TODO: If we ever start using async test completions, polyfill promise return handling - callback.call(fakeContext); - } - catch (error) { - errors.push({ error: error.message, stack: error.stack, name: [...namestack] }); - return; - } - finally { - namestack.pop(); - } - passing++; + setTimeoutAndExecute(timeout, () => { + try { + // TODO: If we ever start using async test completions, polyfill promise return handling + callback.call(fakeContext); + } + catch (error) { + errors.push({ error: error.message, stack: error.stack, name: [...namestack] }); + return; + } + finally { + namestack.pop(); + } + passing++; + }); } else { // Uses `done` callback diff --git a/src/harness/runner.ts b/src/harness/runner.ts index 2bd098b742d..76d65090f02 100644 --- a/src/harness/runner.ts +++ b/src/harness/runner.ts @@ -100,6 +100,7 @@ interface TestConfig { runners?: string[]; runUnitTests?: boolean; noColors?: boolean; + timeout?: number; } interface TaskSet { @@ -108,12 +109,16 @@ interface TaskSet { } let configOption: string; +let globalTimeout: number; function handleTestConfig() { if (testConfigContent !== "") { const testConfig = JSON.parse(testConfigContent); if (testConfig.light) { Harness.lightMode = true; } + if (testConfig.timeout) { + globalTimeout = testConfig.timeout; + } runUnitTests = testConfig.runUnitTests; if (testConfig.workerCount) { workerCount = +testConfig.workerCount;