Skip to content

Support timeouts in the parallel runner#20631

Merged
weswigham merged 2 commits intomicrosoft:masterfrom
weswigham:implement-timeout
Jan 8, 2018
Merged

Support timeouts in the parallel runner#20631
weswigham merged 2 commits intomicrosoft:masterfrom
weswigham:implement-timeout

Conversation

@weswigham
Copy link
Copy Markdown
Member

This allows the parallel runner to timeout tests (and thereby fail) if they exceed the specified timeout (the default 20s for now). @sandersn mentioned wanting this earlier today. On a somewhat related note, our RWC test timeout is probably waaaay too long for how long we expect RWC to take nowadays.

This implements handling of both timeout=n in jake, --timeout=n in gulp, and this.timeout(n) in a unittest.

Comment thread Gulpfile.ts Outdated
stackTraceLimit,
taskConfigsFolder,
noColor: !cmdLineOptions.colors,
timeout: testTimeout,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I would just name these the same.

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);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why return?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to punctuate that no code should execute while the process is exiting.

Comment thread src/harness/parallel/worker.ts Outdated
return;
}
finally {
if (timeout !== undefined) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks familar. Re-use code?

Comment thread src/harness/parallel/host.ts Outdated
return process.exit(2);
}
case "timeout": {
if (data.payload.duration === -1) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use a union number | "default"

@weswigham weswigham merged commit 84e3681 into microsoft:master Jan 8, 2018
@weswigham weswigham deleted the implement-timeout branch January 8, 2018 20:28
@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants