-
Notifications
You must be signed in to change notification settings - Fork 29.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test: terminate WPT workers after test completion #37627
Conversation
@@ -496,6 +498,9 @@ class WPTRunner { | |||
this.resultCallback(filename, { status: 2, name: 'Unknown' }); | |||
} | |||
this.inProgress.delete(filename); | |||
// Always force termination of the worker. Some tests allocate resources | |||
// that would otherwise keep it alive. | |||
this.workers.get(filename).terminate(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's safe to ignore the Promise status, maybe add an empty catch handler?
this.workers.get(filename).terminate(); | |
this.workers.get(filename).terminate().catch(() => {}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This promise can never reject
Lines 344 to 354 in f3d3769
if (this[kHandle] === null) return PromiseResolve(); | |
this[kHandle].stopThread(); | |
// Do not use events.once() here, because the 'exit' event will always be | |
// emitted regardless of any errors, and the point is to only resolve | |
// once the thread has actually stopped. | |
return new Promise((resolve) => { | |
this.once('exit', resolve); | |
}); | |
} |
PR-URL: #37627 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Landed in bba5aac |
PR-URL: #37627 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
No description provided.