-
Notifications
You must be signed in to change notification settings - Fork 3.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
feat(launcher): gracefully close browser on sigint #650
Conversation
Updated, PTAL. |
|
||
let gracefullyClosing = false; | ||
async function gracefullyClose(): Promise<void> { | ||
// We keep listeners until we are done, to handle 'exit' and 'SIGINT' while |
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.
Are we calling gracefullyClose on exit? I think this comment is lying to me.
const {describe, xdescribe, fdescribe} = testRunner; | ||
const {it, fit, xit, dit} = testRunner; | ||
const {beforeAll, beforeEach, afterAll, afterEach} = testRunner; | ||
|
||
async function testSignal(action) { | ||
const options = Object.assign({}, defaultBrowserOptions, { |
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.
nit: If you want to be fancy, you can do
{...defaultBrowserOptions, dumpio: false, webSocket: true}
instead of Object.assign
. Up to you :)
res.stdout.on('data', data => { | ||
output += data; | ||
// Uncomment to debug these tests. | ||
// console.log(data.toString()); |
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.
For your consideration:
// console.log(data.toString()); | |
if (defaultBrowserOptions.dumpio) | |
console.log(data.toString()); |
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.
Let's wait to test on windows before landing this, but it lgtm.
Tested on my computer, seems to work fine. |
…icrosoft#650) This reverts commit aadf7d0.
No description provided.